summaryrefslogtreecommitdiff
path: root/sysdeps-sem_open-Clear-O_CREAT-when-semaphore-file-i.patch
diff options
context:
space:
mode:
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.patch105
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
+