diff options
Diffstat (limited to 'sysdeps-sem_open-Clear-O_CREAT-when-semaphore-file-i.patch')
-rw-r--r-- | sysdeps-sem_open-Clear-O_CREAT-when-semaphore-file-i.patch | 105 |
1 files changed, 105 insertions, 0 deletions
diff --git a/sysdeps-sem_open-Clear-O_CREAT-when-semaphore-file-i.patch b/sysdeps-sem_open-Clear-O_CREAT-when-semaphore-file-i.patch new file mode 100644 index 0000000..1ca450d --- /dev/null +++ b/sysdeps-sem_open-Clear-O_CREAT-when-semaphore-file-i.patch @@ -0,0 +1,105 @@ +From 63dbbc5c52f9823f86270f32fce20d1e91cdf484 Mon Sep 17 00:00:00 2001 +From: Sergio Durigan Junior <sergiodj@sergiodj.net> +Date: Wed, 1 Nov 2023 18:15:23 -0400 +Subject: [PATCH] sysdeps: sem_open: Clear O_CREAT when semaphore file is + expected to exist [BZ #30789] + +When invoking sem_open with O_CREAT as one of its flags, we'll end up +in the second part of sem_open's "if ((oflag & O_CREAT) == 0 || (oflag +& O_EXCL) == 0)", which means that we don't expect the semaphore file +to exist. + +In that part, open_flags is initialized as "O_RDWR | O_CREAT | O_EXCL +| O_CLOEXEC" and there's an attempt to open(2) the file, which will +likely fail because it won't exist. After that first (expected) +failure, some cleanup is done and we go back to the label "try_again", +which lives in the first part of the aforementioned "if". + +The problem is that, in that part of the code, we expect the semaphore +file to exist, and as such O_CREAT (this time the flag we pass to +open(2)) needs to be cleaned from open_flags, otherwise we'll see +another failure (this time unexpected) when trying to open the file, +which will lead the call to sem_open to fail as well. + +This can cause very strange bugs, especially with OpenMPI, which makes +extensive use of semaphores. + +Fix the bug by simplifying the logic when choosing open(2) flags and +making sure O_CREAT is not set when the semaphore file is expected to +exist. + +A regression test for this issue would require a complex and cpu time +consuming logic, since to trigger the wrong code path is not +straightforward due the racy condition. There is a somewhat reliable +reproducer in the bug, but it requires using OpenMPI. + +This resolves BZ #30789. + +See also: https://bugs.launchpad.net/ubuntu/+source/h5py/+bug/2031912 + +Signed-off-by: Sergio Durigan Junior <sergiodj@sergiodj.net> +Co-Authored-By: Simon Chopin <simon.chopin@canonical.com> +Co-Authored-By: Adhemerval Zanella Netto <adhemerval.zanella@linaro.org> +Fixes: 533deafbdf189f5fbb280c28562dd43ace2f4b0f ("Use O_CLOEXEC in more places (BZ #15722)") +(cherry picked from commit f957f47df75b9fab995754011491edebc6feb147) +--- + NEWS | 2 ++ + sysdeps/pthread/sem_open.c | 10 ++++------ + 2 files changed, 6 insertions(+), 6 deletions(-) + +diff --git a/NEWS b/NEWS +index f117874e34..5ac488bf9b 100644 +--- a/NEWS ++++ b/NEWS +@@ -32,6 +32,8 @@ Security related changes: + The following bugs are resolved with this release: + + [30723] posix_memalign repeatedly scans long bin lists ++ [30789] sem_open will fail on multithreaded scenarios when semaphore ++ file doesn't exist (O_CREAT) + [30804] F_GETLK, F_SETLK, and F_SETLKW value change for powerpc64 with + -D_FILE_OFFSET_BITS=64 + [30842] Stack read overflow in getaddrinfo in no-aaaa mode (CVE-2023-4527) +diff --git a/sysdeps/pthread/sem_open.c b/sysdeps/pthread/sem_open.c +index e5db929d20..0e331a7445 100644 +--- a/sysdeps/pthread/sem_open.c ++++ b/sysdeps/pthread/sem_open.c +@@ -32,11 +32,12 @@ + # define __unlink unlink + #endif + ++#define SEM_OPEN_FLAGS (O_RDWR | O_NOFOLLOW | O_CLOEXEC) ++ + sem_t * + __sem_open (const char *name, int oflag, ...) + { + int fd; +- int open_flags; + sem_t *result; + + /* Check that shared futexes are supported. */ +@@ -65,10 +66,8 @@ __sem_open (const char *name, int oflag, ...) + /* If the semaphore object has to exist simply open it. */ + if ((oflag & O_CREAT) == 0 || (oflag & O_EXCL) == 0) + { +- open_flags = O_RDWR | O_NOFOLLOW | O_CLOEXEC; +- open_flags |= (oflag & ~(O_CREAT|O_ACCMODE)); + try_again: +- fd = __open (dirname.name, open_flags); ++ fd = __open (dirname.name, (oflag & O_EXCL) | SEM_OPEN_FLAGS); + + if (fd == -1) + { +@@ -135,8 +134,7 @@ __sem_open (const char *name, int oflag, ...) + } + + /* Open the file. Make sure we do not overwrite anything. */ +- open_flags = O_RDWR | O_CREAT | O_EXCL | O_CLOEXEC; +- fd = __open (tmpfname, open_flags, mode); ++ fd = __open (tmpfname, O_CREAT | O_EXCL | SEM_OPEN_FLAGS, mode); + if (fd == -1) + { + if (errno == EEXIST) +-- +2.33.0 + |