[glibc] sysdeps: sem_open: Clear O_CREAT when semaphore file is expected to exist [BZ #30789]

[glibc] sysdeps: sem_open: Clear O_CREAT when semaphore file is expected to exist [BZ #30789]

[Adhemerval Zanella]

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=f957f47df75b9fab995754011491edebc6feb147

commit f957f47df75b9fab995754011491edebc6feb147
Author: Sergio Durigan Junior <sergiodj@sergiodj.net>
Date:   Wed Nov 1 18:15:23 2023 -0400

    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)")

Diff:
---
 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)