[PATCH 1/3] linux: Require /dev/shm as the shared memory file system [BZ #25383]

[PATCH 1/3] linux: Require /dev/shm as the shared memory file system [BZ #25383]

[Adhemerval Zanella]

From: Florian Weimer <fweimer@redhat.com>

This is an updated version with the patch included in the bug
report, I rebased against master and add the following changes:

- The shm-directory.h is moved to include instead of posix, since it
  is used on different subdirs.

- The shm-directory build rule is added on posix/Makefile instead of
  sysdeps/posix/Makefile

- There is no need for a libc_hidden_def (__shm_get_name).

---
Previously, glibc would pick an arbitrary tmpfs file system from
/proc/mounts if /dev/shm was not available.  This could lead to
an unsuitable file system being picked for the backing storage for
shm_open, sem_open, and related functions.

This patch introduces a new function, __shm_get_name, which builds
the file name under the appropriate (now hard-coded) directory.  It is
called from the various shm_* and sem_* function.  Unlike the
SHM_GET_NAME macro it replaces, the callers handle the return values
and errno updates.  shm-directory.c is moved directly into the posix
subdirectory because it can be implemented directly using POSIX
functionality.  It resides in libc because it is needed by both
librt and nptl/htl.

In the sem_open implementation, tmpfname is initialized directly
from a string constant.  This happens to remove one alloca call.

Checked on x86_64-linux-gnu.
---
 NEWS                                      |   5 +-
 htl/Makefile                              |   2 -
 htl/Versions                              |   1 -
 {sysdeps/nptl => include}/shm-directory.h |  30 +++--
 nptl/Makefile                             |   1 -
 nptl/Versions                             |   1 -
 nptl/nptlfreeres.c                        |   1 -
 nptl/pthreadP.h                           |   1 -
 posix/Makefile                            |   3 +-
 posix/Versions                            |   1 +
 {sysdeps/posix => posix}/shm-directory.c  |  30 +++--
 sysdeps/htl/shm-directory.h               |  30 -----
 sysdeps/posix/Makefile                    |   6 -
 sysdeps/posix/shm-directory.h             |  66 ----------
 sysdeps/posix/shm_open.c                  |  10 +-
 sysdeps/posix/shm_unlink.c                |  13 +-
 sysdeps/pthread/sem_open.c                |  34 ++---
 sysdeps/pthread/sem_unlink.c              |  10 +-
 sysdeps/unix/sysv/linux/shm-directory.c   | 147 ----------------------
 19 files changed, 89 insertions(+), 303 deletions(-)
 rename {sysdeps/nptl => include}/shm-directory.h (56%)
 rename {sysdeps/posix => posix}/shm-directory.c (60%)
 delete mode 100644 sysdeps/htl/shm-directory.h
 delete mode 100644 sysdeps/posix/shm-directory.h
 delete mode 100644 sysdeps/unix/sysv/linux/shm-directory.c

diff --git a/NEWS b/NEWS
index 1ca12bc1a2..85e84fe536 100644
--- a/NEWS
+++ b/NEWS
@@ -20,7 +20,10 @@ Deprecated and removed features, and other changes affecting compatibility:
 
 Changes to build and runtime requirements:
 
-  [Add changes to build and runtime requirements here]
+* On Linux, the shm_open, sem_open, and related functions now expect the
+  file shared memory file system to be mounted at /dev/shm.  These functions
+  no longer search among the system's mount points for a suitable
+  replacement if /dev/shm is not available.
 
 Security related changes:
 
diff --git a/htl/Makefile b/htl/Makefile
index 38d9f8c590..9adc95e07f 100644
--- a/htl/Makefile
+++ b/htl/Makefile
@@ -132,8 +132,6 @@ libpthread-routines := pt-attr pt-attr-destroy pt-attr-getdetachstate	    \
 	sem-post sem-timedwait sem-trywait sem_unlink			    \
 	sem-wait sem-waitfast						    \
 									    \
-	shm-directory							    \
-									    \
 	cancellation							    \
 	cthreads-compat							    \
 	herrno								    \
diff --git a/htl/Versions b/htl/Versions
index 95496297e3..9506043c62 100644
--- a/htl/Versions
+++ b/htl/Versions
@@ -168,7 +168,6 @@ libpthread {
   GLIBC_PRIVATE {
     __pthread_initialize_minimal;
 
-    __shm_directory;
     __pthread_threads;
 
     __cthread_detach;
diff --git a/sysdeps/nptl/shm-directory.h b/include/shm-directory.h
similarity index 56%
rename from sysdeps/nptl/shm-directory.h
rename to include/shm-directory.h
index 995f59834d..1d3ffb6137 100644
--- a/sysdeps/nptl/shm-directory.h
+++ b/include/shm-directory.h
@@ -1,4 +1,4 @@
-/* Header for directory for shm/sem files.  NPTL version.
+/* Header for directory for shm/sem files.
    Copyright (C) 2014-2021 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
 
@@ -18,14 +18,24 @@
 
 #ifndef _SHM_DIRECTORY_H
 
-#include <sysdeps/posix/shm-directory.h>
-
-/* For NPTL the __shm_directory function lives in libpthread.
-   We don't want PLT calls from there.  But it's also used from
-   librt, so it cannot just be declared hidden.  */
-
-#if IS_IN (libpthread)
-hidden_proto (__shm_directory)
-#endif
+#include <limits.h>
+#include <paths.h>
+#include <stdbool.h>
+
+/* The directory that contains shared POSIX objects.  */
+#define SHMDIR _PATH_DEV "shm/"
+
+struct shmdir_name
+{
+  /* The combined prefix/name.  The sizeof includes the terminating
+     NUL byte.  4 bytes are needed for the optional "sem." prefix.  */
+  char name[sizeof (SHMDIR) + 4 + NAME_MAX];
+};
+
+/* Sets RESULT->name to the constructed name and returns 0 on success,
+   or -1 on failure.  Includes the "sem." prefix in the name if
+   SEM_PREFIX is true.  */
+int __shm_get_name (struct shmdir_name *result, const char *name,
+		    bool sem_prefix);
 
 #endif  /* shm-directory.h */
diff --git a/nptl/Makefile b/nptl/Makefile
index 0282e07390..c49134b936 100644
--- a/nptl/Makefile
+++ b/nptl/Makefile
@@ -137,7 +137,6 @@ libpthread-routines = nptl-init nptlfreeres vars events version pt-interp \
 		      pthread_once \
 		      old_pthread_atfork \
 		      pthread_getcpuclockid \
-		      shm-directory \
 		      sem_init sem_destroy \
 		      sem_open sem_close sem_unlink \
 		      sem_getvalue \
diff --git a/nptl/Versions b/nptl/Versions
index 02650fe91c..5db713d1ae 100644
--- a/nptl/Versions
+++ b/nptl/Versions
@@ -303,7 +303,6 @@ libpthread {
     __pthread_unwind; __pthread_get_minstack;
     __pthread_barrier_init; __pthread_barrier_wait;
     __futex_abstimed_wait64; __futex_abstimed_wait_cancelable64;
-    __shm_directory;
     __libpthread_freeres;
   }
 }
diff --git a/nptl/nptlfreeres.c b/nptl/nptlfreeres.c
index 77ad5b53ff..d295bcb3c3 100644
--- a/nptl/nptlfreeres.c
+++ b/nptl/nptlfreeres.c
@@ -27,6 +27,5 @@ __libpthread_freeres (void)
 {
   call_function_static_weak (__default_pthread_attr_freeres);
   call_function_static_weak (__nptl_stacks_freeres);
-  call_function_static_weak (__shm_directory_freeres);
   call_function_static_weak (__nptl_unwind_freeres);
 }
diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h
index e5efa2e62d..d078128230 100644
--- a/nptl/pthreadP.h
+++ b/nptl/pthreadP.h
@@ -629,7 +629,6 @@ extern void __nptl_set_robust (struct pthread *self);
 #endif
 
 extern void __nptl_stacks_freeres (void) attribute_hidden;
-extern void __shm_directory_freeres (void) attribute_hidden;
 
 extern void __wait_lookup_done (void) attribute_hidden;
 
diff --git a/posix/Makefile b/posix/Makefile
index 956ef7d397..f54015b9a8 100644
--- a/posix/Makefile
+++ b/posix/Makefile
@@ -65,7 +65,8 @@ routines :=								      \
 	spawnattr_setsigmask spawnattr_setschedpolicy spawnattr_setschedparam \
 	posix_madvise							      \
 	get_child_max sched_cpucount sched_cpualloc sched_cpufree \
-	streams-compat
+	streams-compat \
+	shm-directory
 
 aux		:= init-posix environ
 tests		:= test-errno tstgetopt testfnm runtests runptests \
diff --git a/posix/Versions b/posix/Versions
index 7d06a6d0c0..cfd3819966 100644
--- a/posix/Versions
+++ b/posix/Versions
@@ -150,5 +150,6 @@ libc {
   GLIBC_PRIVATE {
     __libc_fork; __libc_pread; __libc_pwrite;
     __nanosleep_nocancel; __pause_nocancel;
+    __shm_get_name;
   }
 }
diff --git a/sysdeps/posix/shm-directory.c b/posix/shm-directory.c
similarity index 60%
rename from sysdeps/posix/shm-directory.c
rename to posix/shm-directory.c
index a0510c1ff5..c06bf96aa7 100644
--- a/sysdeps/posix/shm-directory.c
+++ b/posix/shm-directory.c
@@ -16,23 +16,31 @@
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
-#include <shm-directory.h>
 #include <unistd.h>
 
 #if _POSIX_MAPPED_FILES
 
-# include <paths.h>
-
-# define SHMDIR (_PATH_DEV "shm/")
+#include <alloc_buffer.h>
+#include <shm-directory.h>
+#include <string.h>
 
-const char *
-__shm_directory (size_t *len)
+int
+__shm_get_name (struct shmdir_name *result, const char *name, bool sem_prefix)
 {
-  *len = sizeof SHMDIR - 1;
-  return SHMDIR;
+  while (name[0] == '/')
+    ++name;
+  size_t namelen = strlen (name);
+
+  struct alloc_buffer buffer
+    = alloc_buffer_create (result->name, sizeof (result->name));
+  alloc_buffer_copy_bytes (&buffer, SHMDIR, strlen (SHMDIR));
+  if (sem_prefix)
+    alloc_buffer_copy_bytes (&buffer, "sem.", strlen ("sem."));
+  alloc_buffer_copy_bytes (&buffer, name, namelen + 1);
+  if (namelen == 0 || memchr (name, '/', namelen) != NULL
+      || alloc_buffer_has_failed (&buffer))
+    return -1;
+  return 0;
 }
-# if IS_IN (libpthread)
-hidden_def (__shm_directory)
-# endif
 
 #endif
diff --git a/sysdeps/htl/shm-directory.h b/sysdeps/htl/shm-directory.h
deleted file mode 100644
index 8bfd7287b5..0000000000
--- a/sysdeps/htl/shm-directory.h
+++ /dev/null
@@ -1,30 +0,0 @@
-/* Header for directory for shm/sem files.  libpthread version.
-   Copyright (C) 2014-2021 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-
-   The GNU C Library is free software; you can redistribute it and/or
-   modify it under the terms of the GNU Lesser General Public
-   License as published by the Free Software Foundation; either
-   version 2.1 of the License, or (at your option) any later version.
-
-   The GNU C Library is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-   Lesser General Public License for more details.
-
-   You should have received a copy of the GNU Lesser General Public
-   License along with the GNU C Library; if not, see
-   <https://www.gnu.org/licenses/>.  */
-
-#ifndef _SHM_DIRECTORY_H
-
-#include <sysdeps/posix/shm-directory.h>
-
-/* For libpthread the __shm_directory function lives in libpthread.
-   We don't want PLT calls from there.  But it's also used from
-   librt, so it cannot just be declared hidden.  */
-
-#if IS_IN (libpthread)
-hidden_proto (__shm_directory)
-#endif
-#endif /* shm-directory.h */
diff --git a/sysdeps/posix/Makefile b/sysdeps/posix/Makefile
index 52f20f5d97..b58aa6aadb 100644
--- a/sysdeps/posix/Makefile
+++ b/sysdeps/posix/Makefile
@@ -3,9 +3,3 @@ L_tmpnam  = 20
 TMP_MAX   = 238328
 L_ctermid = 9
 L_cuserid = 9
-
-ifeq ($(subdir)|$(have-thread-library),rt|no)
-# With NPTL, this lives in libpthread so it can be used for sem_open too.
-# Without NPTL, it's just private in librt.
-librt-routines += shm-directory
-endif
diff --git a/sysdeps/posix/shm-directory.h b/sysdeps/posix/shm-directory.h
deleted file mode 100644
index 7254bdabba..0000000000
--- a/sysdeps/posix/shm-directory.h
+++ /dev/null
@@ -1,66 +0,0 @@
-/* Header for directory for shm/sem files.
-   Copyright (C) 2014-2021 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-
-   The GNU C Library is free software; you can redistribute it and/or
-   modify it under the terms of the GNU Lesser General Public
-   License as published by the Free Software Foundation; either
-   version 2.1 of the License, or (at your option) any later version.
-
-   The GNU C Library is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-   Lesser General Public License for more details.
-
-   You should have received a copy of the GNU Lesser General Public
-   License along with the GNU C Library; if not, see
-   <https://www.gnu.org/licenses/>.  */
-
-#ifndef _SHM_DIRECTORY_H
-
-#include <errno.h>
-#include <limits.h>
-#include <stdbool.h>
-#include <stdlib.h>
-#include <string.h>
-
-extern const char *__shm_directory (size_t *len);
-
-/* This defines local variables SHM_DIR and SHM_DIRLEN, giving the
-   directory prefix (with trailing slash) and length (not including '\0'
-   terminator) of the directory used for shm files.  If that cannot be
-   determined, it sets errno to ENOSYS and returns RETVAL_FOR_INVALID.
-
-   This uses the local variable NAME as an lvalue, and increments it past
-   any leading slashes.  It then defines the local variable NAMELEN, giving
-   strlen (NAME) + 1.  If NAME is invalid, it sets errno to
-   ERRNO_FOR_INVALID and returns RETVAL_FOR_INVALID.  Finally, it defines
-   the local variable SHM_NAME, giving the absolute file name of the shm
-   file corresponding to NAME.  PREFIX is a string constant used as a
-   prefix on NAME.  */
-
-#define SHM_GET_NAME(errno_for_invalid, retval_for_invalid, prefix)           \
-  size_t shm_dirlen;							      \
-  const char *shm_dir = __shm_directory (&shm_dirlen);			      \
-  /* If we don't know what directory to use, there is nothing we can do.  */  \
-  if (__glibc_unlikely (shm_dir == NULL))				      \
-    {									      \
-      __set_errno (ENOSYS);						      \
-      return retval_for_invalid;					      \
-    }									      \
-  /* Construct the filename.  */					      \
-  while (name[0] == '/')						      \
-    ++name;								      \
-  size_t namelen = strlen (name) + 1;					      \
-  /* Validate the filename.  */						      \
-  if (namelen == 1 || namelen >= NAME_MAX || strchr (name, '/') != NULL)      \
-    {									      \
-      __set_errno (errno_for_invalid);					      \
-      return retval_for_invalid;					      \
-    }									      \
-  char *shm_name = __alloca (shm_dirlen + sizeof prefix - 1 + namelen);	      \
-  __mempcpy (__mempcpy (__mempcpy (shm_name, shm_dir, shm_dirlen),	      \
-                        prefix, sizeof prefix - 1),			      \
-             name, namelen)
-
-#endif	/* shm-directory.h */
diff --git a/sysdeps/posix/shm_open.c b/sysdeps/posix/shm_open.c
index aabc724b27..1817c52f7f 100644
--- a/sysdeps/posix/shm_open.c
+++ b/sysdeps/posix/shm_open.c
@@ -24,6 +24,7 @@
 
 #else
 
+# include <errno.h>
 # include <fcntl.h>
 # include <pthread.h>
 # include <shm-directory.h>
@@ -33,7 +34,12 @@
 int
 shm_open (const char *name, int oflag, mode_t mode)
 {
-  SHM_GET_NAME (EINVAL, -1, "");
+  struct shmdir_name dirname;
+  if (__shm_get_name (&dirname, name, false) != 0)
+    {
+      __set_errno (EINVAL);
+      return -1;
+    }
 
   oflag |= O_NOFOLLOW | O_CLOEXEC;
 
@@ -41,7 +47,7 @@ shm_open (const char *name, int oflag, mode_t mode)
   int state;
   pthread_setcancelstate (PTHREAD_CANCEL_DISABLE, &state);
 
-  int fd = open (shm_name, oflag, mode);
+  int fd = open (dirname.name, oflag, mode);
   if (fd == -1 && __glibc_unlikely (errno == EISDIR))
     /* It might be better to fold this error with EINVAL since
        directory names are just another example for unsuitable shared
diff --git a/sysdeps/posix/shm_unlink.c b/sysdeps/posix/shm_unlink.c
index f3258a0f5a..c90b854c78 100644
--- a/sysdeps/posix/shm_unlink.c
+++ b/sysdeps/posix/shm_unlink.c
@@ -25,16 +25,21 @@
 
 #include <errno.h>
 #include <string.h>
-#include "shm-directory.h"
+#include <shm-directory.h>
 
 
 /* Remove shared memory object.  */
 int
 shm_unlink (const char *name)
 {
-  SHM_GET_NAME (ENOENT, -1, "");
-
-  int result = unlink (shm_name);
+  struct shmdir_name dirname;
+  if (__shm_get_name (&dirname, name, false) != 0)
+    {
+      __set_errno (ENOENT);
+      return -1;
+    }
+
+  int result = unlink (dirname.name);
   if (result < 0 && errno == EPERM)
     __set_errno (EACCES);
   return result;
diff --git a/sysdeps/pthread/sem_open.c b/sysdeps/pthread/sem_open.c
index b0b722121d..d666414f32 100644
--- a/sysdeps/pthread/sem_open.c
+++ b/sysdeps/pthread/sem_open.c
@@ -62,8 +62,9 @@ int __sem_mappings_lock attribute_hidden = LLL_LOCK_INITIALIZER;
 
 /* Search for existing mapping and if possible add the one provided.  */
 static sem_t *
-check_add_mapping (const char *name, size_t namelen, int fd, sem_t *existing)
+check_add_mapping (const char *name, int fd, sem_t *existing)
 {
+  size_t namelen = strlen (name);
   sem_t *result = SEM_FAILED;
 
   /* Get the information about the file.  */
@@ -150,8 +151,12 @@ sem_open (const char *name, int oflag, ...)
       return SEM_FAILED;
     }
 
-  /* Create the name of the final file in local variable SHM_NAME.  */
-  SHM_GET_NAME (EINVAL, SEM_FAILED, SEM_SHM_PREFIX);
+  struct shmdir_name dirname;
+  if (__shm_get_name (&dirname, name, true) != 0)
+    {
+      __set_errno (EINVAL);
+      return SEM_FAILED;
+    }
 
   /* Disable asynchronous cancellation.  */
 #ifdef __libc_ptf_call
@@ -164,7 +169,7 @@ sem_open (const char *name, int oflag, ...)
   if ((oflag & O_CREAT) == 0 || (oflag & O_EXCL) == 0)
     {
     try_again:
-      fd = __libc_open (shm_name,
+      fd = __libc_open (dirname.name,
 			(oflag & ~(O_CREAT|O_ACCMODE)) | O_NOFOLLOW | O_RDWR);
 
       if (fd == -1)
@@ -178,13 +183,12 @@ sem_open (const char *name, int oflag, ...)
       else
 	/* Check whether we already have this semaphore mapped and
 	   create one if necessary.  */
-	result = check_add_mapping (name, namelen, fd, SEM_FAILED);
+	result = check_add_mapping (name, fd, SEM_FAILED);
     }
   else
     {
       /* We have to open a temporary file first since it must have the
 	 correct form before we can start using it.  */
-      char *tmpfname;
       mode_t mode;
       unsigned int value;
       va_list ap;
@@ -217,16 +221,11 @@ sem_open (const char *name, int oflag, ...)
       memset ((char *) &sem.initsem + sizeof (struct new_sem), '\0',
 	      sizeof (sem_t) - sizeof (struct new_sem));
 
-      tmpfname = __alloca (shm_dirlen + sizeof SEM_SHM_PREFIX + 6);
-      char *xxxxxx = __mempcpy (tmpfname, shm_dir, shm_dirlen);
-
+      char tmpfname[] = SHMDIR "sem.XXXXXX";
       int retries = 0;
 #define NRETRIES 50
       while (1)
 	{
-	  /* Add the suffix for mktemp.  */
-	  strcpy (xxxxxx, "XXXXXX");
-
 	  /* We really want to use mktemp here.  We cannot use mkstemp
 	     since the file must be opened with a specific mode.  The
 	     mode cannot later be set since then we cannot apply the
@@ -244,7 +243,12 @@ sem_open (const char *name, int oflag, ...)
 	      if (errno == EEXIST)
 		{
 		  if (++retries < NRETRIES)
-		    continue;
+		    {
+		      /* Restore the six placeholder bytes before the
+			 null terminator before the next attempt.  */
+		      memcpy (tmpfname + sizeof (tmpfname) - 7, "XXXXXX", 6);
+		      continue;
+		    }
 
 		  __set_errno (EAGAIN);
 		}
@@ -265,7 +269,7 @@ sem_open (const char *name, int oflag, ...)
 				       fd, 0)) != MAP_FAILED)
 	{
 	  /* Create the file.  Don't overwrite an existing file.  */
-	  if (link (tmpfname, shm_name) != 0)
+	  if (link (tmpfname, dirname.name) != 0)
 	    {
 	      /* Undo the mapping.  */
 	      (void) munmap (result, sizeof (sem_t));
@@ -290,7 +294,7 @@ sem_open (const char *name, int oflag, ...)
 	    /* Insert the mapping into the search tree.  This also
 	       determines whether another thread sneaked by and already
 	       added such a mapping despite the fact that we created it.  */
-	    result = check_add_mapping (name, namelen, fd, result);
+	    result = check_add_mapping (name, fd, result);
 	}
 
       /* Now remove the temporary name.  This should never fail.  If
diff --git a/sysdeps/pthread/sem_unlink.c b/sysdeps/pthread/sem_unlink.c
index faafbe4e91..1f06a55b8e 100644
--- a/sysdeps/pthread/sem_unlink.c
+++ b/sysdeps/pthread/sem_unlink.c
@@ -27,11 +27,15 @@
 int
 sem_unlink (const char *name)
 {
-  /* Construct the filename.  */
-  SHM_GET_NAME (ENOENT, -1, SEM_SHM_PREFIX);
+  struct shmdir_name dirname;
+  if (__shm_get_name (&dirname, name, true) != 0)
+    {
+      __set_errno (ENOENT);
+      return -1;
+    }
 
   /* Now try removing it.  */
-  int ret = unlink (shm_name);
+  int ret = unlink (dirname.name);
   if (ret < 0 && errno == EPERM)
     __set_errno (EACCES);
   return ret;
diff --git a/sysdeps/unix/sysv/linux/shm-directory.c b/sysdeps/unix/sysv/linux/shm-directory.c
deleted file mode 100644
index ea6d2a46c1..0000000000
--- a/sysdeps/unix/sysv/linux/shm-directory.c
+++ /dev/null
@@ -1,147 +0,0 @@
-/* Determine directory for shm/sem files.  Linux version.
-   Copyright (C) 2000-2021 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-
-   The GNU C Library is free software; you can redistribute it and/or
-   modify it under the terms of the GNU Lesser General Public
-   License as published by the Free Software Foundation; either
-   version 2.1 of the License, or (at your option) any later version.
-
-   The GNU C Library is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-   Lesser General Public License for more details.
-
-   You should have received a copy of the GNU Lesser General Public
-   License along with the GNU C Library; if not, see
-   <https://www.gnu.org/licenses/>.  */
-
-#include "shm-directory.h"
-
-#include <errno.h>
-#include <mntent.h>
-#include <paths.h>
-#include <stdio.h>
-#include <string.h>
-#include <sys/statfs.h>
-#include <libc-lock.h>
-#include "linux_fsinfo.h"
-
-
-/* Mount point of the shared memory filesystem.  */
-static struct
-{
-  char *dir;
-  size_t dirlen;
-} mountpoint;
-
-/* This is the default directory.  */
-static const char defaultdir[] = "/dev/shm/";
-
-/* Protect the `mountpoint' variable above.  */
-__libc_once_define (static, once);
-
-
-/* Determine where the shmfs is mounted (if at all).  */
-static void
-where_is_shmfs (void)
-{
-  char buf[512];
-  struct statfs f;
-  struct mntent resmem;
-  struct mntent *mp;
-  FILE *fp;
-
-  /* The canonical place is /dev/shm.  This is at least what the
-     documentation tells everybody to do.  */
-  if (__statfs (defaultdir, &f) == 0 && (f.f_type == SHMFS_SUPER_MAGIC
-                                         || f.f_type == RAMFS_MAGIC))
-    {
-      /* It is in the normal place.  */
-      mountpoint.dir = (char *) defaultdir;
-      mountpoint.dirlen = sizeof (defaultdir) - 1;
-
-      return;
-    }
-
-  /* OK, do it the hard way.  Look through the /proc/mounts file and if
-     this does not exist through /etc/fstab to find the mount point.  */
-  fp = __setmntent ("/proc/mounts", "r");
-  if (__glibc_unlikely (fp == NULL))
-    {
-      fp = __setmntent (_PATH_MNTTAB, "r");
-      if (__glibc_unlikely (fp == NULL))
-        /* There is nothing we can do.  Blind guesses are not helpful.  */
-        return;
-    }
-
-  /* Now read the entries.  */
-  while ((mp = __getmntent_r (fp, &resmem, buf, sizeof buf)) != NULL)
-    /* The original name is "shm" but this got changed in early Linux
-       2.4.x to "tmpfs".  */
-    if (strcmp (mp->mnt_type, "tmpfs") == 0
-        || strcmp (mp->mnt_type, "shm") == 0)
-      {
-        /* Found it.  There might be more than one place where the
-           filesystem is mounted but one is enough for us.  */
-        size_t namelen;
-
-        /* First make sure this really is the correct entry.  At least
-           some versions of the kernel give wrong information because
-           of the implicit mount of the shmfs for SysV IPC.  */
-        if (__statfs (mp->mnt_dir, &f) != 0 || (f.f_type != SHMFS_SUPER_MAGIC
-                                                && f.f_type != RAMFS_MAGIC))
-          continue;
-
-        namelen = strlen (mp->mnt_dir);
-
-        if (namelen == 0)
-          /* Hum, maybe some crippled entry.  Keep on searching.  */
-          continue;
-
-        mountpoint.dir = (char *) malloc (namelen + 2);
-        if (mountpoint.dir != NULL)
-          {
-            char *cp = __mempcpy (mountpoint.dir, mp->mnt_dir, namelen);
-            if (cp[-1] != '/')
-              *cp++ = '/';
-            *cp = '\0';
-            mountpoint.dirlen = cp - mountpoint.dir;
-          }
-
-        break;
-      }
-
-  /* Close the stream.  */
-  __endmntent (fp);
-}
-
-
-const char *
-__shm_directory (size_t *len)
-{
-  /* Determine where the shmfs is mounted.  */
-  __libc_once (once, where_is_shmfs);
-
-  /* If we don't know the mount points there is nothing we can do.  Ever.  */
-  if (__glibc_unlikely (mountpoint.dir == NULL))
-    {
-      __set_errno (ENOSYS);
-      return NULL;
-    }
-
-  *len = mountpoint.dirlen;
-  return mountpoint.dir;
-}
-#if IS_IN (libpthread)
-hidden_def (__shm_directory)
-
-/* Make sure the table is freed if we want to free everything before
-   exiting.  */
-void
-__shm_directory_freeres (void)
-{
-  if (mountpoint.dir != defaultdir)
-    free (mountpoint.dir);
-}
-#endif
-- 
2.25.1

[PATCH 2/3] pthread: Refactor semaphore code

[Adhemerval Zanella]

The internal semaphore list code is moved to a specific file,
sem_routine.c, and the internal usage is simplified to only two
functions (one to insert a new semaphore and one to remove it
from the internal list).  There is no need to expose the
internal locking, neither how the semaphore mapping is implemented.

No functional or semantic change is expected, tested on
x86_64-linux-gnu.
---
 htl/Makefile                   |   1 +
 htl/semaphoreP.h               |  21 ----
 nptl/Makefile                  |   2 +-
 nptl/semaphoreP.h              |  21 ----
 sysdeps/pthread/sem_close.c    |  56 +---------
 sysdeps/pthread/sem_open.c     | 116 +-------------------
 sysdeps/pthread/sem_routines.c | 187 +++++++++++++++++++++++++++++++++
 sysdeps/pthread/sem_routines.h |  27 +++++
 8 files changed, 223 insertions(+), 208 deletions(-)
 create mode 100644 sysdeps/pthread/sem_routines.c
 create mode 100644 sysdeps/pthread/sem_routines.h

diff --git a/htl/Makefile b/htl/Makefile
index 9adc95e07f..c15c1b194e 100644
--- a/htl/Makefile
+++ b/htl/Makefile
@@ -131,6 +131,7 @@ libpthread-routines := pt-attr pt-attr-destroy pt-attr-getdetachstate	    \
 	sem_close sem-destroy sem-getvalue sem-init sem_open		    \
 	sem-post sem-timedwait sem-trywait sem_unlink			    \
 	sem-wait sem-waitfast						    \
+	sem_routines							    \
 									    \
 	cancellation							    \
 	cthreads-compat							    \
diff --git a/htl/semaphoreP.h b/htl/semaphoreP.h
index 3d6efcdec7..4b13f7b30d 100644
--- a/htl/semaphoreP.h
+++ b/htl/semaphoreP.h
@@ -21,27 +21,6 @@
 
 #define SEM_SHM_PREFIX  "sem."
 
-/* Keeping track of currently used mappings.  */
-struct inuse_sem
-{
-  dev_t dev;
-  ino_t ino;
-  int refcnt;
-  sem_t *sem;
-  char name[];
-};
-
-
-/* The search tree for existing mappings.  */
-extern void *__sem_mappings attribute_hidden;
-
-/* Lock to protect the search tree.  */
-extern int __sem_mappings_lock attribute_hidden;
-
-
-/* Comparison function for search in tree with existing mappings.  */
-extern int __sem_search (const void *a, const void *b) attribute_hidden;
-
 static inline void __new_sem_open_init (struct new_sem *sem, unsigned value)
 {
   /* This always is a shared semaphore.  */
diff --git a/nptl/Makefile b/nptl/Makefile
index c49134b936..8fb7fee6db 100644
--- a/nptl/Makefile
+++ b/nptl/Makefile
@@ -137,7 +137,7 @@ libpthread-routines = nptl-init nptlfreeres vars events version pt-interp \
 		      pthread_once \
 		      old_pthread_atfork \
 		      pthread_getcpuclockid \
-		      sem_init sem_destroy \
+		      sem_init sem_destroy sem_routines \
 		      sem_open sem_close sem_unlink \
 		      sem_getvalue \
 		      sem_wait sem_timedwait sem_clockwait sem_post \
diff --git a/nptl/semaphoreP.h b/nptl/semaphoreP.h
index 3585af09fc..1b786149f4 100644
--- a/nptl/semaphoreP.h
+++ b/nptl/semaphoreP.h
@@ -22,27 +22,6 @@
 
 #define SEM_SHM_PREFIX  "sem."
 
-/* Keeping track of currently used mappings.  */
-struct inuse_sem
-{
-  dev_t dev;
-  ino_t ino;
-  int refcnt;
-  sem_t *sem;
-  char name[];
-};
-
-
-/* The search tree for existing mappings.  */
-extern void *__sem_mappings attribute_hidden;
-
-/* Lock to protect the search tree.  */
-extern int __sem_mappings_lock attribute_hidden;
-
-
-/* Comparison function for search in tree with existing mappings.  */
-extern int __sem_search (const void *a, const void *b) attribute_hidden;
-
 static inline void __new_sem_open_init (struct new_sem *sem, unsigned value)
 {
 #if __HAVE_64B_ATOMICS
diff --git a/sysdeps/pthread/sem_close.c b/sysdeps/pthread/sem_close.c
index 8b6c9c6dc5..6649196cac 100644
--- a/sysdeps/pthread/sem_close.c
+++ b/sysdeps/pthread/sem_close.c
@@ -17,65 +17,17 @@
    <https://www.gnu.org/licenses/>.  */
 
 #include <errno.h>
-#include <search.h>
-#include <sys/mman.h>
 #include "semaphoreP.h"
-
-struct walk_closure
-{
-  sem_t *the_sem;
-  struct inuse_sem *rec;
-};
-
-static void
-walker (const void *inodep, VISIT which, void *closure0)
-{
-  struct walk_closure *closure = closure0;
-  struct inuse_sem *nodep = *(struct inuse_sem **) inodep;
-
-  if (nodep->sem == closure->the_sem)
-    closure->rec = nodep;
-}
-
+#include <sem_routines.h>
 
 int
 sem_close (sem_t *sem)
 {
-  int result = 0;
-
-  /* Get the lock.  */
-  lll_lock (__sem_mappings_lock, LLL_PRIVATE);
-
-  /* Locate the entry for the mapping the caller provided.  */
-  struct inuse_sem *rec;
-  {
-    struct walk_closure closure = { .the_sem = sem, .rec = NULL };
-    __twalk_r (__sem_mappings, walker, &closure);
-    rec = closure.rec;
-  }
-  if  (rec != NULL)
+  if (!__sem_remove_mapping (sem))
     {
-      /* Check the reference counter.  If it is going to be zero, free
-	 all the resources.  */
-      if (--rec->refcnt == 0)
-	{
-	  /* Remove the record from the tree.  */
-	  (void) __tdelete (rec, &__sem_mappings, __sem_search);
-
-	  result = munmap (rec->sem, sizeof (sem_t));
-
-	  free (rec);
-	}
-    }
-  else
-    {
-      /* This is no valid semaphore.  */
-      result = -1;
       __set_errno (EINVAL);
+      return -1;
     }
 
-  /* Release the lock.  */
-  lll_unlock (__sem_mappings_lock, LLL_PRIVATE);
-
-  return result;
+  return 0;
 }
diff --git a/sysdeps/pthread/sem_open.c b/sysdeps/pthread/sem_open.c
index d666414f32..028d76a685 100644
--- a/sysdeps/pthread/sem_open.c
+++ b/sysdeps/pthread/sem_open.c
@@ -16,127 +16,17 @@
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
-#include <errno.h>
 #include <fcntl.h>
-#include <pthread.h>
-#include <search.h>
 #include <semaphore.h>
 #include <stdarg.h>
-#include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
 #include <unistd.h>
 #include <sys/mman.h>
-#include <sys/stat.h>
 #include "semaphoreP.h"
 #include <shm-directory.h>
+#include <sem_routines.h>
 #include <futex-internal.h>
 #include <libc-lock.h>
 
-/* Comparison function for search of existing mapping.  */
-int
-attribute_hidden
-__sem_search (const void *a, const void *b)
-{
-  const struct inuse_sem *as = (const struct inuse_sem *) a;
-  const struct inuse_sem *bs = (const struct inuse_sem *) b;
-
-  if (as->ino != bs->ino)
-    /* Cannot return the difference the type is larger than int.  */
-    return as->ino < bs->ino ? -1 : (as->ino == bs->ino ? 0 : 1);
-
-  if (as->dev != bs->dev)
-    /* Cannot return the difference the type is larger than int.  */
-    return as->dev < bs->dev ? -1 : (as->dev == bs->dev ? 0 : 1);
-
-  return strcmp (as->name, bs->name);
-}
-
-
-/* The search tree for existing mappings.  */
-void *__sem_mappings attribute_hidden;
-
-/* Lock to protect the search tree.  */
-int __sem_mappings_lock attribute_hidden = LLL_LOCK_INITIALIZER;
-
-
-/* Search for existing mapping and if possible add the one provided.  */
-static sem_t *
-check_add_mapping (const char *name, int fd, sem_t *existing)
-{
-  size_t namelen = strlen (name);
-  sem_t *result = SEM_FAILED;
-
-  /* Get the information about the file.  */
-  struct stat64 st;
-  if (__fstat64 (fd, &st) == 0)
-    {
-      /* Get the lock.  */
-      lll_lock (__sem_mappings_lock, LLL_PRIVATE);
-
-      /* Search for an existing mapping given the information we have.  */
-      struct inuse_sem *fake;
-      fake = (struct inuse_sem *) alloca (sizeof (*fake) + namelen);
-      memcpy (fake->name, name, namelen);
-      fake->dev = st.st_dev;
-      fake->ino = st.st_ino;
-
-      struct inuse_sem **foundp = __tfind (fake, &__sem_mappings,
-					   __sem_search);
-      if (foundp != NULL)
-	{
-	  /* There is already a mapping.  Use it.  */
-	  result = (*foundp)->sem;
-	  ++(*foundp)->refcnt;
-	}
-      else
-	{
-	  /* We haven't found a mapping.  Install ione.  */
-	  struct inuse_sem *newp;
-
-	  newp = (struct inuse_sem *) malloc (sizeof (*newp) + namelen);
-	  if (newp != NULL)
-	    {
-	      /* If the caller hasn't provided any map it now.  */
-	      if (existing == SEM_FAILED)
-		existing = (sem_t *) mmap (NULL, sizeof (sem_t),
-					   PROT_READ | PROT_WRITE, MAP_SHARED,
-					   fd, 0);
-
-	      newp->dev = st.st_dev;
-	      newp->ino = st.st_ino;
-	      newp->refcnt = 1;
-	      newp->sem = existing;
-	      memcpy (newp->name, name, namelen);
-
-	      /* Insert the new value.  */
-	      if (existing != MAP_FAILED
-		  && __tsearch (newp, &__sem_mappings, __sem_search) != NULL)
-		/* Successful.  */
-		result = existing;
-	      else
-		/* Something went wrong while inserting the new
-		   value.  We fail completely.  */
-		free (newp);
-	    }
-	}
-
-      /* Release the lock.  */
-      lll_unlock (__sem_mappings_lock, LLL_PRIVATE);
-    }
-
-  if (result != existing && existing != SEM_FAILED && existing != MAP_FAILED)
-    {
-      /* Do not disturb errno.  */
-      int save = errno;
-      munmap (existing, sizeof (sem_t));
-      errno = save;
-    }
-
-  return result;
-}
-
-
 sem_t *
 sem_open (const char *name, int oflag, ...)
 {
@@ -183,7 +73,7 @@ sem_open (const char *name, int oflag, ...)
       else
 	/* Check whether we already have this semaphore mapped and
 	   create one if necessary.  */
-	result = check_add_mapping (name, fd, SEM_FAILED);
+	result = __sem_check_add_mapping (name, fd, SEM_FAILED);
     }
   else
     {
@@ -294,7 +184,7 @@ sem_open (const char *name, int oflag, ...)
 	    /* Insert the mapping into the search tree.  This also
 	       determines whether another thread sneaked by and already
 	       added such a mapping despite the fact that we created it.  */
-	    result = check_add_mapping (name, fd, result);
+	    result = __sem_check_add_mapping (name, fd, result);
 	}
 
       /* Now remove the temporary name.  This should never fail.  If
diff --git a/sysdeps/pthread/sem_routines.c b/sysdeps/pthread/sem_routines.c
new file mode 100644
index 0000000000..78d9364ebd
--- /dev/null
+++ b/sysdeps/pthread/sem_routines.c
@@ -0,0 +1,187 @@
+/* Helper code for POSIX semaphore implementation.
+   Copyright (C) 2021 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <search.h>
+#include <semaphoreP.h>
+#include <sys/mman.h>
+#include <sem_routines.h>
+
+/* Keeping track of currently used mappings.  */
+struct inuse_sem
+{
+  dev_t dev;
+  ino_t ino;
+  int refcnt;
+  sem_t *sem;
+  char name[];
+};
+
+/* Comparison function for search of existing mapping.  */
+static int
+sem_search (const void *a, const void *b)
+{
+  const struct inuse_sem *as = (const struct inuse_sem *) a;
+  const struct inuse_sem *bs = (const struct inuse_sem *) b;
+
+  if (as->ino != bs->ino)
+    /* Cannot return the difference the type is larger than int.  */
+    return as->ino < bs->ino ? -1 : (as->ino == bs->ino ? 0 : 1);
+
+  if (as->dev != bs->dev)
+    /* Cannot return the difference the type is larger than int.  */
+    return as->dev < bs->dev ? -1 : (as->dev == bs->dev ? 0 : 1);
+
+  return strcmp (as->name, bs->name);
+}
+
+/* The search tree for existing mappings.  */
+static void *sem_mappings;
+
+/* Lock to protect the search tree.  */
+static int sem_mappings_lock = LLL_LOCK_INITIALIZER;
+
+
+/* Search for existing mapping and if possible add the one provided.  */
+sem_t *
+__sem_check_add_mapping (const char *name, int fd, sem_t *existing)
+{
+  size_t namelen = strlen (name);
+  sem_t *result = SEM_FAILED;
+
+  /* Get the information about the file.  */
+  struct stat64 st;
+  if (__fstat64 (fd, &st) == 0)
+    {
+      /* Get the lock.  */
+      lll_lock (sem_mappings_lock, LLL_PRIVATE);
+
+      /* Search for an existing mapping given the information we have.  */
+      struct inuse_sem *fake;
+      fake = (struct inuse_sem *) alloca (sizeof (*fake) + namelen);
+      memcpy (fake->name, name, namelen);
+      fake->dev = st.st_dev;
+      fake->ino = st.st_ino;
+
+      struct inuse_sem **foundp = __tfind (fake, &sem_mappings, sem_search);
+      if (foundp != NULL)
+	{
+	  /* There is already a mapping.  Use it.  */
+	  result = (*foundp)->sem;
+	  ++(*foundp)->refcnt;
+	}
+      else
+	{
+	  /* We haven't found a mapping.  Install ione.  */
+	  struct inuse_sem *newp;
+
+	  newp = (struct inuse_sem *) malloc (sizeof (*newp) + namelen);
+	  if (newp != NULL)
+	    {
+	      /* If the caller hasn't provided any map it now.  */
+	      if (existing == SEM_FAILED)
+		existing = (sem_t *) mmap (NULL, sizeof (sem_t),
+					   PROT_READ | PROT_WRITE, MAP_SHARED,
+					   fd, 0);
+
+	      newp->dev = st.st_dev;
+	      newp->ino = st.st_ino;
+	      newp->refcnt = 1;
+	      newp->sem = existing;
+	      memcpy (newp->name, name, namelen);
+
+	      /* Insert the new value.  */
+	      if (existing != MAP_FAILED
+		  && __tsearch (newp, &sem_mappings, sem_search) != NULL)
+		/* Successful.  */
+		result = existing;
+	      else
+		/* Something went wrong while inserting the new
+		   value.  We fail completely.  */
+		free (newp);
+	    }
+	}
+
+      /* Release the lock.  */
+      lll_unlock (sem_mappings_lock, LLL_PRIVATE);
+    }
+
+  if (result != existing && existing != SEM_FAILED && existing != MAP_FAILED)
+    {
+      /* Do not disturb errno.  */
+      int save = errno;
+      munmap (existing, sizeof (sem_t));
+      errno = save;
+    }
+
+  return result;
+}
+
+struct walk_closure
+{
+  sem_t *the_sem;
+  struct inuse_sem *rec;
+};
+
+static void
+walker (const void *inodep, VISIT which, void *closure0)
+{
+  struct walk_closure *closure = closure0;
+  struct inuse_sem *nodep = *(struct inuse_sem **) inodep;
+
+  if (nodep->sem == closure->the_sem)
+    closure->rec = nodep;
+}
+
+bool
+__sem_remove_mapping (sem_t *sem)
+{
+  bool ret = true;
+
+  /* Get the lock.  */
+  lll_lock (sem_mappings_lock, LLL_PRIVATE);
+
+  /* Locate the entry for the mapping the caller provided.  */
+  struct inuse_sem *rec;
+  {
+    struct walk_closure closure = { .the_sem = sem, .rec = NULL };
+    __twalk_r (sem_mappings, walker, &closure);
+    rec = closure.rec;
+  }
+  if (rec != NULL)
+    {
+      /* Check the reference counter.  If it is going to be zero, free
+	 all the resources.  */
+      if (--rec->refcnt == 0)
+	{
+	  /* Remove the record from the tree.  */
+	  __tdelete (rec, &sem_mappings, sem_search);
+
+	  if (munmap (rec->sem, sizeof (sem_t)) == -1)
+	    ret = false;
+
+	  free (rec);
+	}
+    }
+  else
+    ret = false;
+
+  /* Release the lock.  */
+  lll_unlock (sem_mappings_lock, LLL_PRIVATE);
+
+  return ret;
+}
diff --git a/sysdeps/pthread/sem_routines.h b/sysdeps/pthread/sem_routines.h
new file mode 100644
index 0000000000..25d3e880ea
--- /dev/null
+++ b/sysdeps/pthread/sem_routines.h
@@ -0,0 +1,27 @@
+/* Helper code for POSIX semaphore implementation.
+   Copyright (C) 2021 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#ifndef _SEM_ROUTINES_H
+#define _SEM_ROUTINES_H
+
+sem_t * __sem_check_add_mapping (const char *name, int fd, sem_t *existing)
+  attribute_hidden;
+
+bool __sem_remove_mapping (sem_t *sem) attribute_hidden;
+
+#endif
-- 
2.25.1

[PATCH 3/3] pthread: Remove alloca usage from __sem_check_add_mapping

[Adhemerval Zanella]

sem_open already returns EINVAL for input names larger than NAME_MAX,
so it can assume the largest name length to with tfind.

Checked on x86_64-linux-gnu.
---
 sysdeps/pthread/sem_routines.c | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/sysdeps/pthread/sem_routines.c b/sysdeps/pthread/sem_routines.c
index 78d9364ebd..d5e2f56547 100644
--- a/sysdeps/pthread/sem_routines.c
+++ b/sysdeps/pthread/sem_routines.c
@@ -31,6 +31,15 @@ struct inuse_sem
   char name[];
 };
 
+struct search_sem
+{
+  dev_t dev;
+  ino_t ino;
+  int refcnt;
+  sem_t *sem;
+  char name[NAME_MAX];
+};
+
 /* Comparison function for search of existing mapping.  */
 static int
 sem_search (const void *a, const void *b)
@@ -61,6 +70,9 @@ sem_t *
 __sem_check_add_mapping (const char *name, int fd, sem_t *existing)
 {
   size_t namelen = strlen (name);
+  if (namelen > NAME_MAX)
+    return NULL;
+
   sem_t *result = SEM_FAILED;
 
   /* Get the information about the file.  */
@@ -71,13 +83,12 @@ __sem_check_add_mapping (const char *name, int fd, sem_t *existing)
       lll_lock (sem_mappings_lock, LLL_PRIVATE);
 
       /* Search for an existing mapping given the information we have.  */
-      struct inuse_sem *fake;
-      fake = (struct inuse_sem *) alloca (sizeof (*fake) + namelen);
-      memcpy (fake->name, name, namelen);
-      fake->dev = st.st_dev;
-      fake->ino = st.st_ino;
+      struct search_sem fake;
+      memcpy (fake.name, name, namelen);
+      fake.dev = st.st_dev;
+      fake.ino = st.st_ino;
 
-      struct inuse_sem **foundp = __tfind (fake, &sem_mappings, sem_search);
+      struct inuse_sem **foundp = __tfind (&fake, &sem_mappings, sem_search);
       if (foundp != NULL)
 	{
 	  /* There is already a mapping.  Use it.  */
-- 
2.25.1

Re: [PATCH 1/3] linux: Require /dev/shm as the shared memory file system [BZ #25383]

[Florian Weimer]

* Adhemerval Zanella via Libc-alpha:

> From: Florian Weimer <fweimer@redhat.com>
>
> This is an updated version with the patch included in the bug
> report, I rebased against master and add the following changes:
>
> - The shm-directory.h is moved to include instead of posix, since it
>   is used on different subdirs.
>
> - The shm-directory build rule is added on posix/Makefile instead of
>   sysdeps/posix/Makefile
>
> - There is no need for a libc_hidden_def (__shm_get_name).

The changes look good to me.

I added the libc_hidden_def in anticipation of future symbol moves.

Thanks,
Florian
-- 
Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill

Re: [PATCH 3/3] pthread: Remove alloca usage from __sem_check_add_mapping

[Florian Weimer]

* Adhemerval Zanella via Libc-alpha:

> sem_open already returns EINVAL for input names larger than NAME_MAX,
> so it can assume the largest name length to with tfind.

Typo: “to with”

> @@ -61,6 +70,9 @@ sem_t *
>  __sem_check_add_mapping (const char *name, int fd, sem_t *existing)
>  {
>    size_t namelen = strlen (name);
> +  if (namelen > NAME_MAX)
> +    return NULL;
> +
>    sem_t *result = SEM_FAILED;

Should this return SEM_FAILED?

Thanks,
Florian
-- 
Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill

Re: [PATCH 2/3] pthread: Refactor semaphore code

[Florian Weimer]

* Adhemerval Zanella via Libc-alpha:

> The internal semaphore list code is moved to a specific file,
> sem_routine.c, and the internal usage is simplified to only two
> functions (one to insert a new semaphore and one to remove it
> from the internal list).  There is no need to expose the
> internal locking, neither how the semaphore mapping is implemented.
>
> No functional or semantic change is expected, tested on
> x86_64-linux-gnu.

Looks okay to me.

Thanks,
Florian
-- 
Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill

Re: [PATCH 3/3] pthread: Remove alloca usage from __sem_check_add_mapping

[Adhemerval Zanella]

On 07/02/2021 06:29, Florian Weimer wrote:
> * Adhemerval Zanella via Libc-alpha:
> 
>> sem_open already returns EINVAL for input names larger than NAME_MAX,
>> so it can assume the largest name length to with tfind.
> 
> Typo: “to with”

Ack.

> 
>> @@ -61,6 +70,9 @@ sem_t *
>>  __sem_check_add_mapping (const char *name, int fd, sem_t *existing)
>>  {
>>    size_t namelen = strlen (name);
>> +  if (namelen > NAME_MAX)
>> +    return NULL;
>> +
>>    sem_t *result = SEM_FAILED;
> 
> Should this return SEM_FAILED?

It should, I have fixed it.

> 
> Thanks,
> Florian
> 

[COMMITTED] linux: Fix __sem_check_add_mapping name length

[Adhemerval Zanella]

Take in consideration the trailling NULL since sem_search uses
strcmp to compare entries.

Checked on x86_64-linux-gnu and powerpc-linux-gnu (where it triggered
a nptl/tst-sem7 regression).
---
 sysdeps/pthread/sem_routines.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/sysdeps/pthread/sem_routines.c b/sysdeps/pthread/sem_routines.c
index ea7a445324..c2c8b54cd0 100644
--- a/sysdeps/pthread/sem_routines.c
+++ b/sysdeps/pthread/sem_routines.c
@@ -72,6 +72,7 @@ __sem_check_add_mapping (const char *name, int fd, sem_t *existing)
   size_t namelen = strlen (name);
   if (namelen > NAME_MAX)
     return SEM_FAILED;
+  namelen += 1;
 
   sem_t *result = SEM_FAILED;
 
-- 
2.25.1

Re: [COMMITTED] linux: Fix __sem_check_add_mapping name length

[Andreas Schwab]

On Feb 09 2021, Adhemerval Zanella via Libc-alpha wrote:

> diff --git a/sysdeps/pthread/sem_routines.c b/sysdeps/pthread/sem_routines.c
> index ea7a445324..c2c8b54cd0 100644
> --- a/sysdeps/pthread/sem_routines.c
> +++ b/sysdeps/pthread/sem_routines.c
> @@ -72,6 +72,7 @@ __sem_check_add_mapping (const char *name, int fd, sem_t *existing)
>    size_t namelen = strlen (name);
>    if (namelen > NAME_MAX)
>      return SEM_FAILED;
> +  namelen += 1;

struct search_sem has only room for NAME_MAX.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

[COMMITTED] linux: Fix __sem_check_add_mapping search_sem

[Adhemerval Zanella]

Similar to __sem_check_add_mapping fix, take in consideration the
trailling NULL.

Checked x86_64-linux-gnu.
---
 sysdeps/pthread/sem_routines.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sysdeps/pthread/sem_routines.c b/sysdeps/pthread/sem_routines.c
index c2c8b54cd0..34e6410729 100644
--- a/sysdeps/pthread/sem_routines.c
+++ b/sysdeps/pthread/sem_routines.c
@@ -37,7 +37,7 @@ struct search_sem
   ino_t ino;
   int refcnt;
   sem_t *sem;
-  char name[NAME_MAX];
+  char name[NAME_MAX + 1];
 };
 
 /* Comparison function for search of existing mapping.  */
-- 
2.25.1