93 lines
3.4 KiB
Diff
93 lines
3.4 KiB
Diff
|
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/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.43.0
|
||
|
|