]>
Commit | Line | Data |
---|---|---|
a61a21ef MT |
1 | From 63dbbc5c52f9823f86270f32fce20d1e91cdf484 Mon Sep 17 00:00:00 2001 |
2 | From: Sergio Durigan Junior <sergiodj@sergiodj.net> | |
3 | Date: Wed, 1 Nov 2023 18:15:23 -0400 | |
4 | Subject: [PATCH 31/44] sysdeps: sem_open: Clear O_CREAT when semaphore file is | |
5 | expected to exist [BZ #30789] | |
6 | ||
7 | When invoking sem_open with O_CREAT as one of its flags, we'll end up | |
8 | in the second part of sem_open's "if ((oflag & O_CREAT) == 0 || (oflag | |
9 | & O_EXCL) == 0)", which means that we don't expect the semaphore file | |
10 | to exist. | |
11 | ||
12 | In that part, open_flags is initialized as "O_RDWR | O_CREAT | O_EXCL | |
13 | | O_CLOEXEC" and there's an attempt to open(2) the file, which will | |
14 | likely fail because it won't exist. After that first (expected) | |
15 | failure, some cleanup is done and we go back to the label "try_again", | |
16 | which lives in the first part of the aforementioned "if". | |
17 | ||
18 | The problem is that, in that part of the code, we expect the semaphore | |
19 | file to exist, and as such O_CREAT (this time the flag we pass to | |
20 | open(2)) needs to be cleaned from open_flags, otherwise we'll see | |
21 | another failure (this time unexpected) when trying to open the file, | |
22 | which will lead the call to sem_open to fail as well. | |
23 | ||
24 | This can cause very strange bugs, especially with OpenMPI, which makes | |
25 | extensive use of semaphores. | |
26 | ||
27 | Fix the bug by simplifying the logic when choosing open(2) flags and | |
28 | making sure O_CREAT is not set when the semaphore file is expected to | |
29 | exist. | |
30 | ||
31 | A regression test for this issue would require a complex and cpu time | |
32 | consuming logic, since to trigger the wrong code path is not | |
33 | straightforward due the racy condition. There is a somewhat reliable | |
34 | reproducer in the bug, but it requires using OpenMPI. | |
35 | ||
36 | This resolves BZ #30789. | |
37 | ||
38 | See also: https://bugs.launchpad.net/ubuntu/+source/h5py/+bug/2031912 | |
39 | ||
40 | Signed-off-by: Sergio Durigan Junior <sergiodj@sergiodj.net> | |
41 | Co-Authored-By: Simon Chopin <simon.chopin@canonical.com> | |
42 | Co-Authored-By: Adhemerval Zanella Netto <adhemerval.zanella@linaro.org> | |
43 | Fixes: 533deafbdf189f5fbb280c28562dd43ace2f4b0f ("Use O_CLOEXEC in more places (BZ #15722)") | |
44 | (cherry picked from commit f957f47df75b9fab995754011491edebc6feb147) | |
45 | --- | |
46 | NEWS | 2 ++ | |
47 | sysdeps/pthread/sem_open.c | 10 ++++------ | |
48 | 2 files changed, 6 insertions(+), 6 deletions(-) | |
49 | ||
50 | diff --git a/NEWS b/NEWS | |
51 | index f117874e34..5ac488bf9b 100644 | |
52 | --- a/NEWS | |
53 | +++ b/NEWS | |
54 | @@ -32,6 +32,8 @@ Security related changes: | |
55 | The following bugs are resolved with this release: | |
56 | ||
57 | [30723] posix_memalign repeatedly scans long bin lists | |
58 | + [30789] sem_open will fail on multithreaded scenarios when semaphore | |
59 | + file doesn't exist (O_CREAT) | |
60 | [30804] F_GETLK, F_SETLK, and F_SETLKW value change for powerpc64 with | |
61 | -D_FILE_OFFSET_BITS=64 | |
62 | [30842] Stack read overflow in getaddrinfo in no-aaaa mode (CVE-2023-4527) | |
63 | diff --git a/sysdeps/pthread/sem_open.c b/sysdeps/pthread/sem_open.c | |
64 | index e5db929d20..0e331a7445 100644 | |
65 | --- a/sysdeps/pthread/sem_open.c | |
66 | +++ b/sysdeps/pthread/sem_open.c | |
67 | @@ -32,11 +32,12 @@ | |
68 | # define __unlink unlink | |
69 | #endif | |
70 | ||
71 | +#define SEM_OPEN_FLAGS (O_RDWR | O_NOFOLLOW | O_CLOEXEC) | |
72 | + | |
73 | sem_t * | |
74 | __sem_open (const char *name, int oflag, ...) | |
75 | { | |
76 | int fd; | |
77 | - int open_flags; | |
78 | sem_t *result; | |
79 | ||
80 | /* Check that shared futexes are supported. */ | |
81 | @@ -65,10 +66,8 @@ __sem_open (const char *name, int oflag, ...) | |
82 | /* If the semaphore object has to exist simply open it. */ | |
83 | if ((oflag & O_CREAT) == 0 || (oflag & O_EXCL) == 0) | |
84 | { | |
85 | - open_flags = O_RDWR | O_NOFOLLOW | O_CLOEXEC; | |
86 | - open_flags |= (oflag & ~(O_CREAT|O_ACCMODE)); | |
87 | try_again: | |
88 | - fd = __open (dirname.name, open_flags); | |
89 | + fd = __open (dirname.name, (oflag & O_EXCL) | SEM_OPEN_FLAGS); | |
90 | ||
91 | if (fd == -1) | |
92 | { | |
93 | @@ -135,8 +134,7 @@ __sem_open (const char *name, int oflag, ...) | |
94 | } | |
95 | ||
96 | /* Open the file. Make sure we do not overwrite anything. */ | |
97 | - open_flags = O_RDWR | O_CREAT | O_EXCL | O_CLOEXEC; | |
98 | - fd = __open (tmpfname, open_flags, mode); | |
99 | + fd = __open (tmpfname, O_CREAT | O_EXCL | SEM_OPEN_FLAGS, mode); | |
100 | if (fd == -1) | |
101 | { | |
102 | if (errno == EEXIST) | |
103 | -- | |
104 | 2.39.2 | |
105 |