[PATCH] sysdeps: Clear O_CREAT|O_ACCMODE when trying again on sem_open
Sergio Durigan Junior
sergiodj@sergiodj.net
Wed Aug 23 19:08:50 GMT 2023
On Wednesday, August 23 2023, Adhemerval Zanella Netto wrote:
> On 23/08/23 11:10, Sergio Durigan Junior via Libc-alpha wrote:
>> On Wednesday, August 23 2023, Andreas Schwab wrote:
>>
>>> On Aug 23 2023, Sergio Durigan Junior via Libc-alpha wrote:
>>>
>>>> The fix here is to actually make sure that the O_CREAT|O_ACCMODE flags
>>>> are clear after we enter "try_again".
>>>
>>> That doesn't match what the patch does. It only adds flags to
>>> open_flags, and does not remove any.
>>
>> You're correct. Simon also spotted the same problem. Here's an updated
>> version of the patch.
>>
>
>> From cb1a17590878955bcf6d7c2821ca95da3896608c Mon Sep 17 00:00:00 2001
>> From: Sergio Durigan Junior <sergiodj@sergiodj.net>
>> Date: Wed, 23 Aug 2023 00:10:44 -0400
>> Subject: [PATCH] sysdeps: Clear O_CREAT|O_ACCMODE when trying again on
>> sem_open
>>
>
> Please open a bug report, even without a reproducer; and reference it
> on the subject.
Here it is.
Thanks,
--
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
https://sergiodj.net/
>From 584ac8bf375903cf38d73703d2439b3b74cbcd41 Mon Sep 17 00:00:00 2001
From: Sergio Durigan Junior <sergiodj@sergiodj.net>
Date: Wed, 23 Aug 2023 00:10:44 -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.
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)")
---
sysdeps/pthread/sem_open.c | 10 ++++------
1 file changed, 4 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.39.2
More information about the Libc-alpha
mailing list