From fddfebbd1064dec313891ad91dd9ca6e92e622ec Mon Sep 17 00:00:00 2001 From: Ulrich Drepper Date: Sat, 17 May 2003 20:53:32 +0000 Subject: [PATCH] Update. 2003-05-17 Ulrich Drepper * sem_open.c: Fix one endless loop. Implement correct semantics wrt opening the same semaphore more then once. * sem_close.c: Adjust for sem_open change. * semaphoreP.h: Include . Define struct inuse_sem. Declare __sem_mappings, __sem_mappings_lock, __sem_search. * Makefile (tests): Add tst-sem7. * tst-sem7.c: New file. --- nptl/ChangeLog | 10 +++ nptl/Makefile | 2 +- nptl/sem_open.c | 175 +++++++++++++++++++++++++++++++++++++++++----- nptl/semaphoreP.h | 23 +++++- nptl/tst-sem7.c | 109 +++++++++++++++++++++++++++++ 5 files changed, 298 insertions(+), 21 deletions(-) create mode 100644 nptl/tst-sem7.c diff --git a/nptl/ChangeLog b/nptl/ChangeLog index 0813aaf1c4..0f4d0855a2 100644 --- a/nptl/ChangeLog +++ b/nptl/ChangeLog @@ -1,3 +1,13 @@ +2003-05-17 Ulrich Drepper + + * sem_open.c: Fix one endless loop. Implement correct semantics + wrt opening the same semaphore more then once. + * sem_close.c: Adjust for sem_open change. + * semaphoreP.h: Include . Define struct inuse_sem. + Declare __sem_mappings, __sem_mappings_lock, __sem_search. + * Makefile (tests): Add tst-sem7. + * tst-sem7.c: New file. + 2003-05-16 Roland McGrath * sysdeps/unix/sysv/linux/register-atfork.c (libc_freeres_fn): Fix diff --git a/nptl/Makefile b/nptl/Makefile index a166ddab43..31faa7c4a8 100644 --- a/nptl/Makefile +++ b/nptl/Makefile @@ -149,7 +149,7 @@ tests = tst-attr1 tst-attr2 \ tst-rwlock11 \ tst-once1 tst-once2 tst-once3 tst-once4 \ tst-key1 tst-key2 tst-key3 tst-key4 \ - tst-sem1 tst-sem2 tst-sem3 tst-sem4 tst-sem5 tst-sem6 \ + tst-sem1 tst-sem2 tst-sem3 tst-sem4 tst-sem5 tst-sem6 tst-sem7 \ tst-barrier1 tst-barrier2 tst-barrier3 \ tst-align \ tst-basic1 tst-basic2 tst-basic3 tst-basic4 tst-basic5 tst-basic6 \ diff --git a/nptl/sem_open.c b/nptl/sem_open.c index 2bfea63a80..374c7d84e9 100644 --- a/nptl/sem_open.c +++ b/nptl/sem_open.c @@ -22,6 +22,7 @@ #include #include #include +#include #include #include #include @@ -122,11 +123,105 @@ __where_is_shmfs (void) } +/* 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. */ +lll_lock_t __sem_mappings_lock = 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) +{ + sem_t *result = SEM_FAILED; + + /* Get the information about the file. */ + struct stat64 st; + if (__fxstat64 (_STAT_VER, fd, &st) == 0) + { + /* Get the lock. */ + lll_lock (__sem_mappings_lock); + + /* 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 if (existing != SEM_FAILED) + { + /* We haven't found a mapping but the caller has a mapping. + Install it. */ + struct inuse_sem *newp; + + newp = (struct inuse_sem *) malloc (sizeof (*newp) + namelen); + if (newp != NULL) + { + 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 (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); + } + + if (result != existing && existing != SEM_FAILED) + { + /* Do not disturb errno. */ + INTERNAL_SYSCALL_DECL (err); + INTERNAL_SYSCALL (munmap, err, 2, existing, sizeof (sem_t)); + } + + return result; +} + + sem_t * sem_open (const char *name, int oflag, ...) { char *finalname; - size_t namelen; sem_t *result = SEM_FAILED; int fd; @@ -150,12 +245,12 @@ sem_open (const char *name, int oflag, ...) __set_errno (EINVAL); return SEM_FAILED; } - namelen = strlen (name); + size_t namelen = strlen (name) + 1; /* Create the name of the final file. */ - finalname = (char *) alloca (mountpoint.dirlen + namelen + 1); + finalname = (char *) alloca (mountpoint.dirlen + namelen); __mempcpy (__mempcpy (finalname, mountpoint.dir, mountpoint.dirlen), - name, namelen + 1); + name, namelen); /* If the semaphore object has to exist simply open it. */ if ((oflag & O_CREAT) == 0 || (oflag & O_EXCL) == 0) @@ -167,15 +262,22 @@ sem_open (const char *name, int oflag, ...) if (fd == -1) { /* If we are supposed to create the file try this next. */ - if ((oflag & O_CREAT) != 0) + if ((oflag & O_CREAT) != 0 && errno == ENOENT) goto try_create; /* Return. errno is already set. */ } else - /* Map the sem_t structure from the file. */ - result = (sem_t *) mmap (NULL, sizeof (sem_t), PROT_READ | PROT_WRITE, - MAP_SHARED, fd, 0); + { + /* Check whether we already have this semaphore mapped. */ + result = check_add_mapping (name, namelen, fd, SEM_FAILED); + + /* Map the sem_t structure from the file. */ + if (result == SEM_FAILED) + result = (sem_t *) mmap (NULL, sizeof (sem_t), + PROT_READ | PROT_WRITE, MAP_SHARED, + fd, 0); + } } else { @@ -200,14 +302,6 @@ sem_open (const char *name, int oflag, ...) return SEM_FAILED; } - tmpfname = (char *) alloca (mountpoint.dirlen + 6 + 1); - strcpy (__mempcpy (tmpfname, mountpoint.dir, mountpoint.dirlen), - "XXXXXX"); - - fd = mkstemp (tmpfname); - if (fd == -1) - return SEM_FAILED; - /* Create the initial file content. */ sem_t initsem; @@ -218,10 +312,44 @@ sem_open (const char *name, int oflag, ...) memset ((char *) &initsem + sizeof (struct sem), '\0', sizeof (sem_t) - sizeof (struct sem)); + tmpfname = (char *) alloca (mountpoint.dirlen + 6 + 1); + char *xxxxxx = __mempcpy (tmpfname, mountpoint.dir, mountpoint.dirlen); + + 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 + file create mask. */ + if (mktemp (tmpfname) == NULL) + return SEM_FAILED; + + /* Open the file. Make sure we do not overwrite anything. */ + fd = __libc_open (tmpfname, O_RDWR | O_CREAT | O_EXCL, mode); + if (fd == -1) + { + if (errno == EEXIST) + { + if (++retries < NRETRIES) + continue; + + __set_errno (EAGAIN); + } + + return SEM_FAILED; + } + + /* We got a file. */ + break; + } + if (TEMP_FAILURE_RETRY (__libc_write (fd, &initsem, sizeof (sem_t))) == sizeof (sem_t) - /* Adjust the permission. */ - && fchmod (fd, mode) == 0 /* Map the sem_t structure from the file. */ && (result = (sem_t *) mmap (NULL, sizeof (sem_t), PROT_READ | PROT_WRITE, MAP_SHARED, @@ -249,6 +377,11 @@ sem_open (const char *name, int oflag, ...) goto try_again; } } + else + /* 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); } /* Now remove the temporary name. This should never fail. If @@ -262,7 +395,11 @@ sem_open (const char *name, int oflag, ...) /* We don't need the file descriptor anymore. */ if (fd != -1) - (void) __libc_close (fd); + { + /* Do not disturb errno. */ + INTERNAL_SYSCALL_DECL (err); + INTERNAL_SYSCALL (close, err, 1, fd); + } return result; } diff --git a/nptl/semaphoreP.h b/nptl/semaphoreP.h index 0d3f6e53a0..d14ea92c74 100644 --- a/nptl/semaphoreP.h +++ b/nptl/semaphoreP.h @@ -1,4 +1,4 @@ -/* Copyright (C) 2002 Free Software Foundation, Inc. +/* Copyright (C) 2002, 2003 Free Software Foundation, Inc. This file is part of the GNU C Library. Contributed by Ulrich Drepper , 2002. @@ -17,8 +17,10 @@ Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA. */ +#include #include "pthreadP.h" + /* Mount point of the shared memory filesystem. */ struct mountpoint_info { @@ -26,16 +28,35 @@ struct mountpoint_info size_t dirlen; }; +/* Keeping track of currently used mappings. */ +struct inuse_sem +{ + dev_t dev; + ino_t ino; + int refcnt; + sem_t *sem; + char name[0]; +}; + /* Variables used in multiple interfaces. */ extern struct mountpoint_info mountpoint attribute_hidden; extern pthread_once_t __namedsem_once attribute_hidden; +/* The search tree for existing mappings. */ +extern void *__sem_mappings attribute_hidden; + +/* Lock to protect the search tree. */ +extern lll_lock_t __sem_mappings_lock; + /* Initializer for mountpoint. */ extern void __where_is_shmfs (void) attribute_hidden; +/* Comparison function for search in tree with existing mappings. */ +extern int __sem_search (const void *a, const void *b) attribute_hidden; + /* Prototypes of functions with multiple interfaces. */ extern int __new_sem_init (sem_t *sem, int pshared, unsigned int value); diff --git a/nptl/tst-sem7.c b/nptl/tst-sem7.c new file mode 100644 index 0000000000..a85c73e715 --- /dev/null +++ b/nptl/tst-sem7.c @@ -0,0 +1,109 @@ +/* Copyright (C) 2003 Free Software Foundation, Inc. + This file is part of the GNU C Library. + Contributed by Ulrich Drepper , 2003. + + 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, write to the Free + Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA + 02111-1307 USA. */ + +#include +#include +#include +#include +#include +#include + + +static void +remove_sem (int status, void *arg) +{ + sem_unlink (arg); +} + + +int +main (void) +{ + sem_t *s; + sem_t *s2; + sem_t *s3; + + s = sem_open ("/glibc-tst-sem7", O_CREAT, 0600, 1); + if (s == SEM_FAILED) + { + if (errno == ENOSYS) + { + puts ("sem_open not supported. Oh well."); + return 0; + } + + /* Maybe the shm filesystem has strict permissions. */ + if (errno == EACCES) + { + puts ("sem_open not allowed. Oh well."); + return 0; + } + + printf ("sem_open: %m\n"); + return 1; + } + + on_exit (remove_sem, (void *) "/glibc-tst-sem7"); + + /* We have the semaphore object. Now try again. We should get the + same address. */ + s2 = sem_open ("/glibc-tst-sem7", O_CREAT, 0600, 1); + if (s2 == SEM_FAILED) + { + puts ("2nd sem_open failed"); + return 1; + } + if (s != s2) + { + puts ("2nd sem_open didn't return the same address"); + return 1; + } + + /* And again, this time without O_CREAT. */ + s3 = sem_open ("/glibc-tst-sem7", 0); + if (s3 == SEM_FAILED) + { + puts ("3rd sem_open failed"); + return 1; + } + if (s != s3) + { + puts ("3rd sem_open didn't return the same address"); + return 1; + } + + /* Now close the handle. Three times. */ + if (sem_close (s2) != 0) + { + puts ("1st sem_close failed"); + return 1; + } + if (sem_close (s) != 0) + { + puts ("2nd sem_close failed"); + return 1; + } + if (sem_close (s3) != 0) + { + puts ("3rd sem_close failed"); + return 1; + } + + return 0; +}