Bug 189353 - POSIX sem_unlink does not actually unlink the semaphore in the process context
Summary: POSIX sem_unlink does not actually unlink the semaphore in the process context
Status: In Progress
Alias: None
Product: Base System
Classification: Unclassified
Component: standards (show other bugs)
Version: Unspecified
Hardware: Any Any
: Normal Affects Only Me
Assignee: freebsd-standards (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-05-04 12:20 UTC by joris
Modified: 2018-01-13 09:54 UTC (History)
0 users

See Also:


Attachments
sem_test.c (1.79 KB, text/plain)
2014-05-04 13:46 UTC, joris
no flags Details
sem_test.c (1.79 KB, text/plain)
2014-05-04 13:46 UTC, joris
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description joris 2014-05-04 12:20:00 UTC
When a posix sempahore is open, it is cached in the process address space. A call to sem_unlink does not invalidate the caching, and trying to reopen the semaphore with O_EXCL after calling sem_unlink fails with EEXIST.

How-To-Repeat: Here is a test case :

#include <fcntl.h>
#include <sys/stat.h>
#include <sys/wait.h>
#include <sys/types.h>
#include <semaphore.h>

#include <err.h>


int
main(int argc, char *argv[])
{
	sem_t *sem1, *sem2;
	int error;

	sem1 = sem_open("/test-sem1", O_CREAT | O_EXCL, S_IRWXU, 1);
	if (sem1 == SEM_FAILED)
		err(1,"Cannot create test /test-sem1 semaphore");

	error = sem_unlink("/test-sem1");

	if (error)
		err(1, "Cannot unlink semaphore /test-sem1");


	sem2 = sem_open("/test-sem1", O_CREAT | O_EXCL, S_IRWXU, 2);

	if (sem2 == SEM_FAILED)
		err(1, "Cannot re-create semaphore /test-sem1");

	error = sem_unlink("/test-sem1");

	if (error)
		err(1, "Cannot unlink semaphore /test-sem1");

	return(0);
}
Comment 1 Kostik Belousov 2014-05-04 13:37:43 UTC
Yes, I think you are right.  Please try the following patch, which worked
for me, but I only lightly tested it.

diff --git a/lib/libc/gen/sem_new.c b/lib/libc/gen/sem_new.c
index 9a2ab27..8e4a91d 100644
--- a/lib/libc/gen/sem_new.c
+++ b/lib/libc/gen/sem_new.c
@@ -161,7 +161,7 @@ _sem_open(const char *name, int flags, ...)
 
 	_pthread_mutex_lock(&sem_llock);
 	LIST_FOREACH(ni, &sem_list, next) {
-		if (strcmp(name, ni->name) == 0) {
+		if (ni->name != NULL && strcmp(name, ni->name) == 0) {
 			if ((flags & (O_CREAT|O_EXCL)) == (O_CREAT|O_EXCL)) {
 				_pthread_mutex_unlock(&sem_llock);
 				errno = EEXIST;
@@ -287,20 +287,32 @@ _sem_close(sem_t *sem)
 int
 _sem_unlink(const char *name)
 {
+	struct sem_nameinfo *ni;
 	char path[PATH_MAX];
+	int error;
 
 	if (name[0] != '/') {
 		errno = ENOENT;
 		return -1;
 	}
 	name++;
-
 	strcpy(path, SEM_PREFIX);
 	if (strlcat(path, name, sizeof(path)) >= sizeof(path)) {
 		errno = ENAMETOOLONG;
 		return (-1);
 	}
-	return unlink(path);
+
+	_pthread_once(&once, sem_module_init);
+	_pthread_mutex_lock(&sem_llock);
+	LIST_FOREACH(ni, &sem_list, next) {
+		if (ni->name != NULL && strcmp(name, ni->name) == 0) {
+			ni->name = NULL;
+			break;
+		}
+	}
+	error = unlink(path);
+	_pthread_mutex_unlock(&sem_llock);
+	return (error);
 }
 
 int
Comment 2 joris 2014-05-04 13:46:23 UTC
I've found this issue while implementing semaphores for dragonflyBSD,
and at first i came with the same solution than yours. But it won't
work when you do the same in two processes : unlink will only remove
the caching on the process calling unlink, not all of them. See
attached test case.

It looks like glibc is using inodes numbers and dev number for that,
which, while not being strictly correct (inodes can be reused), seems
to work fine on linux.

For dragonfly, i'm currently trying to keep filedescriptors opened in
the nameinfo structure. This adds quite a bit of overhead, but this
way you can check link count with fstat during a sem_open to see if
the semaphore file still exist, and return the cached mapping only if
st_nlink > 0.

This solution at least should be strictly correct, but the inode
solution could be fine in practice.

Regards,
joris
Comment 3 joris 2014-05-04 13:46:43 UTC
I've found this issue while implementing semaphores for dragonflyBSD,
and at first i came with the same solution than yours. But it won't
work when you do the same in two processes : unlink will only remove
the caching on the process calling unlink, not all of them. See
attached test case.

It looks like glibc is using inodes numbers and dev number for that,
which, while not being strictly correct (inodes can be reused), seems
to work fine on linux.

For dragonfly, i'm currently trying to keep filedescriptors opened in
the nameinfo structure. This adds quite a bit of overhead, but this
way you can check link count with fstat during a sem_open to see if
the semaphore file still exist, and return the cached mapping only if
st_nlink > 0.

This solution at least should be strictly correct, but the inode
solution could be fine in practice.

Regards,
joris
Comment 4 Kostik Belousov 2014-05-04 19:23:56 UTC
On Sun, May 04, 2014 at 02:46:23PM +0200, Joris Giovannangeli wrote:
> work when you do the same in two processes : unlink will only remove
> the caching on the process calling unlink, not all of them. See
> attached test case.

Why didn't you provided both tests in the first report ?

> It looks like glibc is using inodes numbers and dev number for that,
> which, while not being strictly correct (inodes can be reused), seems
> to work fine on linux.

Inode number + generation provides the same practical guarantee as the
file descriptor.  You cannot have two inodes with the same (inum, gen)
simultaneously, generation would need some years to wrap.

> For dragonfly, i'm currently trying to keep filedescriptors opened in
> the nameinfo structure. This adds quite a bit of overhead, but this
> way you can check link count with fstat during a sem_open to see if
> the semaphore file still exist, and return the cached mapping only if
> st_nlink > 0.

Yes, keeping filedescriptors for the semaphores is too heavy-weight.
If keeping fd, we could just use fifos to implement the functionality,
I think.

> 
> This solution at least should be strictly correct, but the inode
> solution could be fine in practice.


Do you have any further tests ?  The patch below passes yours' two
and tools/regression/posixsem2.  I tried to be modest with open(2)
syscalls, reusing the fd in sem_open().  This also should prevent
a race.

diff --git a/lib/libc/gen/sem_new.c b/lib/libc/gen/sem_new.c
index 9a2ab27..ed58de3 100644
--- a/lib/libc/gen/sem_new.c
+++ b/lib/libc/gen/sem_new.c
@@ -66,6 +66,9 @@ __weak_reference(_sem_wait, sem_wait);
 struct sem_nameinfo {
 	int open_count;
 	char *name;
+	dev_t dev;
+	ino_t ino;
+	uint32_t gen;
 	sem_t *sem;
 	LIST_ENTRY(sem_nameinfo) next;
 };
@@ -151,37 +154,50 @@ _sem_open(const char *name, int flags, ...)
 		return (SEM_FAILED);
 	}
 	name++;
-
+	strcpy(path, SEM_PREFIX);
+	if (strlcat(path, name, sizeof(path)) >= sizeof(path)) {
+		errno = ENAMETOOLONG;
+		return (SEM_FAILED);
+	}
 	if (flags & ~(O_CREAT|O_EXCL)) {
 		errno = EINVAL;
 		return (SEM_FAILED);
 	}
-
+	if ((flags & O_CREAT) != 0) {
+		va_start(ap, flags);
+		mode = va_arg(ap, int);
+		value = va_arg(ap, int);
+		va_end(ap);
+	}
+	fd = -1;
 	_pthread_once(&once, sem_module_init);
 
 	_pthread_mutex_lock(&sem_llock);
 	LIST_FOREACH(ni, &sem_list, next) {
-		if (strcmp(name, ni->name) == 0) {
+		if (ni->name != NULL && strcmp(name, ni->name) == 0) {
+			fd = _open(path, flags | O_RDWR | O_CLOEXEC |
+			    O_EXLOCK, mode);
+			if (fd == -1 || _fstat(fd, &sb) == -1)
+				goto error;
+			if (ni->dev != sb.st_dev ||
+			    ni->ino != sb.st_ino ||
+			    ni->gen != sb.st_gen) {
+				ni->name = NULL;
+				ni = NULL;
+				break;
+			}
 			if ((flags & (O_CREAT|O_EXCL)) == (O_CREAT|O_EXCL)) {
-				_pthread_mutex_unlock(&sem_llock);
-				errno = EEXIST;
-				return (SEM_FAILED);
+				goto error;
 			} else {
 				ni->open_count++;
 				sem = ni->sem;
 				_pthread_mutex_unlock(&sem_llock);
+				_close(fd);
 				return (sem);
 			}
 		}
 	}
 
-	if (flags & O_CREAT) {
-		va_start(ap, flags);
-		mode = va_arg(ap, int);
-		value = va_arg(ap, int);
-		va_end(ap);
-	}
-
 	len = sizeof(*ni) + strlen(name) + 1;
 	ni = (struct sem_nameinfo *)malloc(len);
 	if (ni == NULL) {
@@ -192,17 +208,13 @@ _sem_open(const char *name, int flags, ...)
 	ni->name = (char *)(ni+1);
 	strcpy(ni->name, name);
 
-	strcpy(path, SEM_PREFIX);
-	if (strlcat(path, name, sizeof(path)) >= sizeof(path)) {
-		errno = ENAMETOOLONG;
-		goto error;
+	if (fd == -1) {
+		fd = _open(path, flags | O_RDWR | O_CLOEXEC | O_EXLOCK, mode);
+		if (fd == -1)
+			goto error;
+		if (_fstat(fd, &sb) == -1)
+			goto error;
 	}
-
-	fd = _open(path, flags|O_RDWR|O_CLOEXEC|O_EXLOCK, mode);
-	if (fd == -1)
-		goto error;
-	if (_fstat(fd, &sb))
-		goto error;
 	if (sb.st_size < sizeof(sem_t)) {
 		sem_t tmp;
 
@@ -228,6 +240,9 @@ _sem_open(const char *name, int flags, ...)
 	}
 	ni->open_count = 1;
 	ni->sem = sem;
+	ni->dev = sb.st_dev;
+	ni->ino = sb.st_ino;
+	ni->gen = sb.st_gen;
 	LIST_INSERT_HEAD(&sem_list, ni, next);
 	_close(fd);
 	_pthread_mutex_unlock(&sem_llock);
@@ -287,20 +302,32 @@ _sem_close(sem_t *sem)
 int
 _sem_unlink(const char *name)
 {
+	struct sem_nameinfo *ni;
 	char path[PATH_MAX];
+	int error;
 
 	if (name[0] != '/') {
 		errno = ENOENT;
 		return -1;
 	}
 	name++;
-
 	strcpy(path, SEM_PREFIX);
 	if (strlcat(path, name, sizeof(path)) >= sizeof(path)) {
 		errno = ENAMETOOLONG;
 		return (-1);
 	}
-	return unlink(path);
+
+	_pthread_once(&once, sem_module_init);
+	_pthread_mutex_lock(&sem_llock);
+	LIST_FOREACH(ni, &sem_list, next) {
+		if (ni->name != NULL && strcmp(name, ni->name) == 0) {
+			ni->name = NULL;
+			break;
+		}
+	}
+	error = unlink(path);
+	_pthread_mutex_unlock(&sem_llock);
+	return (error);
 }
 
 int
Comment 5 joris 2014-05-04 19:30:33 UTC
> Why didn't you provided both tests in the first report ?

Because i wrote the second one after...

> Do you have any further tests ?  The patch below passes yours' two
> and tools/regression/posixsem2.  I tried to be modest with open(2)
> syscalls, reusing the fd in sem_open().  This also should prevent
> a race.

This looks fine. I don't have any other tests, but you can use the posix
test suite which has coverage for this API.
Comment 6 dfilter service freebsd_committer 2014-05-10 20:08:13 UTC
Author: kib
Date: Sat May 10 19:08:07 2014
New Revision: 265847
URL: http://svnweb.freebsd.org/changeset/base/265847

Log:
  Invalidate the cache for the named posix semaphore when opened and
  actual file storing the semaphore object is different from the file
  created on the first open.  Store the file st_dev and st_ino members
  of the struct stat in the semaphore structure on open, and compare
  them with the attributes of the opened file to detect unlink and
  re-creation.
  
  This fixes an issue of sem_unlink(3) failing to flush the named entry
  in the semaphore list for the current or remote process, making
  sem_unlink(3) not correctly operating if the unlinked semaphore is
  still opened.
  
  Reported by:	Joris Giovannangeli <joris@giovannangeli.fr>
  PR:	standards/189353
  Reviewed by:	jilles (previous version)
  Sponsored by:	The FreeBSD Foundation
  MFC after:	1 week

Modified:
  head/lib/libc/gen/sem_new.c

Modified: head/lib/libc/gen/sem_new.c
==============================================================================
--- head/lib/libc/gen/sem_new.c	Sat May 10 19:06:36 2014	(r265846)
+++ head/lib/libc/gen/sem_new.c	Sat May 10 19:08:07 2014	(r265847)
@@ -66,6 +66,8 @@ __weak_reference(_sem_wait, sem_wait);
 struct sem_nameinfo {
 	int open_count;
 	char *name;
+	dev_t dev;
+	ino_t ino;
 	sem_t *sem;
 	LIST_ENTRY(sem_nameinfo) next;
 };
@@ -151,37 +153,46 @@ _sem_open(const char *name, int flags, .
 		return (SEM_FAILED);
 	}
 	name++;
-
+	strcpy(path, SEM_PREFIX);
+	if (strlcat(path, name, sizeof(path)) >= sizeof(path)) {
+		errno = ENAMETOOLONG;
+		return (SEM_FAILED);
+	}
 	if (flags & ~(O_CREAT|O_EXCL)) {
 		errno = EINVAL;
 		return (SEM_FAILED);
 	}
-
+	if ((flags & O_CREAT) != 0) {
+		va_start(ap, flags);
+		mode = va_arg(ap, int);
+		value = va_arg(ap, int);
+		va_end(ap);
+	}
+	fd = -1;
 	_pthread_once(&once, sem_module_init);
 
 	_pthread_mutex_lock(&sem_llock);
 	LIST_FOREACH(ni, &sem_list, next) {
-		if (strcmp(name, ni->name) == 0) {
-			if ((flags & (O_CREAT|O_EXCL)) == (O_CREAT|O_EXCL)) {
-				_pthread_mutex_unlock(&sem_llock);
-				errno = EEXIST;
-				return (SEM_FAILED);
-			} else {
-				ni->open_count++;
-				sem = ni->sem;
-				_pthread_mutex_unlock(&sem_llock);
-				return (sem);
+		if (ni->name != NULL && strcmp(name, ni->name) == 0) {
+			fd = _open(path, flags | O_RDWR | O_CLOEXEC |
+			    O_EXLOCK, mode);
+			if (fd == -1 || _fstat(fd, &sb) == -1)
+				goto error;
+			if ((flags & (O_CREAT | O_EXCL)) == (O_CREAT |
+			    O_EXCL) || ni->dev != sb.st_dev ||
+			    ni->ino != sb.st_ino) {
+				ni->name = NULL;
+				ni = NULL;
+				break;
 			}
+			ni->open_count++;
+			sem = ni->sem;
+			_pthread_mutex_unlock(&sem_llock);
+			_close(fd);
+			return (sem);
 		}
 	}
 
-	if (flags & O_CREAT) {
-		va_start(ap, flags);
-		mode = va_arg(ap, int);
-		value = va_arg(ap, int);
-		va_end(ap);
-	}
-
 	len = sizeof(*ni) + strlen(name) + 1;
 	ni = (struct sem_nameinfo *)malloc(len);
 	if (ni == NULL) {
@@ -192,17 +203,11 @@ _sem_open(const char *name, int flags, .
 	ni->name = (char *)(ni+1);
 	strcpy(ni->name, name);
 
-	strcpy(path, SEM_PREFIX);
-	if (strlcat(path, name, sizeof(path)) >= sizeof(path)) {
-		errno = ENAMETOOLONG;
-		goto error;
+	if (fd == -1) {
+		fd = _open(path, flags | O_RDWR | O_CLOEXEC | O_EXLOCK, mode);
+		if (fd == -1 || _fstat(fd, &sb) == -1)
+			goto error;
 	}
-
-	fd = _open(path, flags|O_RDWR|O_CLOEXEC|O_EXLOCK, mode);
-	if (fd == -1)
-		goto error;
-	if (_fstat(fd, &sb))
-		goto error;
 	if (sb.st_size < sizeof(sem_t)) {
 		sem_t tmp;
 
@@ -228,6 +233,8 @@ _sem_open(const char *name, int flags, .
 	}
 	ni->open_count = 1;
 	ni->sem = sem;
+	ni->dev = sb.st_dev;
+	ni->ino = sb.st_ino;
 	LIST_INSERT_HEAD(&sem_list, ni, next);
 	_close(fd);
 	_pthread_mutex_unlock(&sem_llock);
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Comment 7 dfilter service freebsd_committer 2014-05-17 12:43:17 UTC
Author: kib
Date: Sat May 17 11:43:14 2014
New Revision: 266306
URL: http://svnweb.freebsd.org/changeset/base/266306

Log:
  MFC r265847:
  Fix sem_unlink(3) to properly invalidate the semaphores name cache.
  
  PR:	standards/189353

Modified:
  stable/10/lib/libc/gen/sem_new.c
Directory Properties:
  stable/10/   (props changed)

Modified: stable/10/lib/libc/gen/sem_new.c
==============================================================================
--- stable/10/lib/libc/gen/sem_new.c	Sat May 17 11:38:26 2014	(r266305)
+++ stable/10/lib/libc/gen/sem_new.c	Sat May 17 11:43:14 2014	(r266306)
@@ -66,6 +66,8 @@ __weak_reference(_sem_wait, sem_wait);
 struct sem_nameinfo {
 	int open_count;
 	char *name;
+	dev_t dev;
+	ino_t ino;
 	sem_t *sem;
 	LIST_ENTRY(sem_nameinfo) next;
 };
@@ -151,37 +153,46 @@ _sem_open(const char *name, int flags, .
 		return (SEM_FAILED);
 	}
 	name++;
-
+	strcpy(path, SEM_PREFIX);
+	if (strlcat(path, name, sizeof(path)) >= sizeof(path)) {
+		errno = ENAMETOOLONG;
+		return (SEM_FAILED);
+	}
 	if (flags & ~(O_CREAT|O_EXCL)) {
 		errno = EINVAL;
 		return (SEM_FAILED);
 	}
-
+	if ((flags & O_CREAT) != 0) {
+		va_start(ap, flags);
+		mode = va_arg(ap, int);
+		value = va_arg(ap, int);
+		va_end(ap);
+	}
+	fd = -1;
 	_pthread_once(&once, sem_module_init);
 
 	_pthread_mutex_lock(&sem_llock);
 	LIST_FOREACH(ni, &sem_list, next) {
-		if (strcmp(name, ni->name) == 0) {
-			if ((flags & (O_CREAT|O_EXCL)) == (O_CREAT|O_EXCL)) {
-				_pthread_mutex_unlock(&sem_llock);
-				errno = EEXIST;
-				return (SEM_FAILED);
-			} else {
-				ni->open_count++;
-				sem = ni->sem;
-				_pthread_mutex_unlock(&sem_llock);
-				return (sem);
+		if (ni->name != NULL && strcmp(name, ni->name) == 0) {
+			fd = _open(path, flags | O_RDWR | O_CLOEXEC |
+			    O_EXLOCK, mode);
+			if (fd == -1 || _fstat(fd, &sb) == -1)
+				goto error;
+			if ((flags & (O_CREAT | O_EXCL)) == (O_CREAT |
+			    O_EXCL) || ni->dev != sb.st_dev ||
+			    ni->ino != sb.st_ino) {
+				ni->name = NULL;
+				ni = NULL;
+				break;
 			}
+			ni->open_count++;
+			sem = ni->sem;
+			_pthread_mutex_unlock(&sem_llock);
+			_close(fd);
+			return (sem);
 		}
 	}
 
-	if (flags & O_CREAT) {
-		va_start(ap, flags);
-		mode = va_arg(ap, int);
-		value = va_arg(ap, int);
-		va_end(ap);
-	}
-
 	len = sizeof(*ni) + strlen(name) + 1;
 	ni = (struct sem_nameinfo *)malloc(len);
 	if (ni == NULL) {
@@ -192,17 +203,11 @@ _sem_open(const char *name, int flags, .
 	ni->name = (char *)(ni+1);
 	strcpy(ni->name, name);
 
-	strcpy(path, SEM_PREFIX);
-	if (strlcat(path, name, sizeof(path)) >= sizeof(path)) {
-		errno = ENAMETOOLONG;
-		goto error;
+	if (fd == -1) {
+		fd = _open(path, flags | O_RDWR | O_CLOEXEC | O_EXLOCK, mode);
+		if (fd == -1 || _fstat(fd, &sb) == -1)
+			goto error;
 	}
-
-	fd = _open(path, flags|O_RDWR|O_CLOEXEC|O_EXLOCK, mode);
-	if (fd == -1)
-		goto error;
-	if (_fstat(fd, &sb))
-		goto error;
 	if (sb.st_size < sizeof(sem_t)) {
 		sem_t tmp;
 
@@ -228,6 +233,8 @@ _sem_open(const char *name, int flags, .
 	}
 	ni->open_count = 1;
 	ni->sem = sem;
+	ni->dev = sb.st_dev;
+	ni->ino = sb.st_ino;
 	LIST_INSERT_HEAD(&sem_list, ni, next);
 	_close(fd);
 	_pthread_mutex_unlock(&sem_llock);
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Comment 8 dfilter service freebsd_committer 2014-05-17 17:29:43 UTC
Author: kib
Date: Sat May 17 16:29:39 2014
New Revision: 266327
URL: http://svnweb.freebsd.org/changeset/base/266327

Log:
  MFC r265847:
  Fix sem_unlink(3) to properly invalidate the semaphores name cache.
  
  PR:	standards/189353

Modified:
  stable/9/lib/libc/gen/sem_new.c
Directory Properties:
  stable/9/lib/libc/   (props changed)

Modified: stable/9/lib/libc/gen/sem_new.c
==============================================================================
--- stable/9/lib/libc/gen/sem_new.c	Sat May 17 16:28:29 2014	(r266326)
+++ stable/9/lib/libc/gen/sem_new.c	Sat May 17 16:29:39 2014	(r266327)
@@ -66,6 +66,8 @@ __weak_reference(_sem_wait, sem_wait);
 struct sem_nameinfo {
 	int open_count;
 	char *name;
+	dev_t dev;
+	ino_t ino;
 	sem_t *sem;
 	LIST_ENTRY(sem_nameinfo) next;
 };
@@ -151,37 +153,46 @@ _sem_open(const char *name, int flags, .
 		return (SEM_FAILED);
 	}
 	name++;
-
+	strcpy(path, SEM_PREFIX);
+	if (strlcat(path, name, sizeof(path)) >= sizeof(path)) {
+		errno = ENAMETOOLONG;
+		return (SEM_FAILED);
+	}
 	if (flags & ~(O_CREAT|O_EXCL)) {
 		errno = EINVAL;
 		return (SEM_FAILED);
 	}
-
+	if ((flags & O_CREAT) != 0) {
+		va_start(ap, flags);
+		mode = va_arg(ap, int);
+		value = va_arg(ap, int);
+		va_end(ap);
+	}
+	fd = -1;
 	_pthread_once(&once, sem_module_init);
 
 	_pthread_mutex_lock(&sem_llock);
 	LIST_FOREACH(ni, &sem_list, next) {
-		if (strcmp(name, ni->name) == 0) {
-			if ((flags & (O_CREAT|O_EXCL)) == (O_CREAT|O_EXCL)) {
-				_pthread_mutex_unlock(&sem_llock);
-				errno = EEXIST;
-				return (SEM_FAILED);
-			} else {
-				ni->open_count++;
-				sem = ni->sem;
-				_pthread_mutex_unlock(&sem_llock);
-				return (sem);
+		if (ni->name != NULL && strcmp(name, ni->name) == 0) {
+			fd = _open(path, flags | O_RDWR | O_CLOEXEC |
+			    O_EXLOCK, mode);
+			if (fd == -1 || _fstat(fd, &sb) == -1)
+				goto error;
+			if ((flags & (O_CREAT | O_EXCL)) == (O_CREAT |
+			    O_EXCL) || ni->dev != sb.st_dev ||
+			    ni->ino != sb.st_ino) {
+				ni->name = NULL;
+				ni = NULL;
+				break;
 			}
+			ni->open_count++;
+			sem = ni->sem;
+			_pthread_mutex_unlock(&sem_llock);
+			_close(fd);
+			return (sem);
 		}
 	}
 
-	if (flags & O_CREAT) {
-		va_start(ap, flags);
-		mode = va_arg(ap, int);
-		value = va_arg(ap, int);
-		va_end(ap);
-	}
-
 	len = sizeof(*ni) + strlen(name) + 1;
 	ni = (struct sem_nameinfo *)malloc(len);
 	if (ni == NULL) {
@@ -192,17 +203,11 @@ _sem_open(const char *name, int flags, .
 	ni->name = (char *)(ni+1);
 	strcpy(ni->name, name);
 
-	strcpy(path, SEM_PREFIX);
-	if (strlcat(path, name, sizeof(path)) >= sizeof(path)) {
-		errno = ENAMETOOLONG;
-		goto error;
+	if (fd == -1) {
+		fd = _open(path, flags | O_RDWR | O_CLOEXEC | O_EXLOCK, mode);
+		if (fd == -1 || _fstat(fd, &sb) == -1)
+			goto error;
 	}
-
-	fd = _open(path, flags|O_RDWR|O_CLOEXEC|O_EXLOCK, mode);
-	if (fd == -1)
-		goto error;
-	if (_fstat(fd, &sb))
-		goto error;
 	if (sb.st_size < sizeof(sem_t)) {
 		sem_t tmp;
 
@@ -228,6 +233,8 @@ _sem_open(const char *name, int flags, .
 	}
 	ni->open_count = 1;
 	ni->sem = sem;
+	ni->dev = sb.st_dev;
+	ni->ino = sb.st_ino;
 	LIST_INSERT_HEAD(&sem_list, ni, next);
 	_close(fd);
 	_pthread_mutex_unlock(&sem_llock);
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Comment 9 Mark Linimon freebsd_committer freebsd_triage 2014-05-17 17:45:09 UTC
State Changed
From-To: open->open

add misfiled PRs. 


Comment 10 Mark Linimon freebsd_committer freebsd_triage 2014-05-17 17:45:09 UTC
Responsible Changed
From-To: freebsd-standards->freebsd-standards
Comment 11 Antoine Brodin freebsd_committer 2017-12-31 22:35:55 UTC
It seems that this bug still exists when /tmp is using tmpfs.

tmpfs generates inode numbers with alloc_unr so they get reused.
Comment 12 commit-hook freebsd_committer 2018-01-13 09:54:05 UTC
A commit references this bug:

Author: antoine
Date: Sat Jan 13 09:53:15 UTC 2018
New revision: 458923
URL: https://svnweb.freebsd.org/changeset/ports/458923

Log:
  Work around sem_unlink bug on FreeBSD when /tmp is using tmpfs

  PR:		189353

Changes:
  head/security/yara/Makefile
  head/security/yara/files/
  head/security/yara/files/patch-threading.c