overhaul sem_open

this function was overly complicated and not even obviously correct.
avoid using openat/linkat just like in shm_open, and instead expand
pathname using code shared with shm_open. remove bogus (and dangerous,
with priorities) use of spinlocks.

this commit also heavily streamlines the code and ensures there are no
failure cases that can happen after a new semaphore has been created
in the filesystem, since that case is unreportable.
This commit is contained in:
Rich Felker 2012-09-30 19:35:40 -04:00
commit bf258341b7
2 changed files with 101 additions and 110 deletions

View file

@ -7,7 +7,7 @@
char *__strchrnul(const char *, int); char *__strchrnul(const char *, int);
static const char *mapname(const char *name, char *buf) char *__shm_mapname(const char *name, char *buf)
{ {
char *p; char *p;
while (*name == '/') name++; while (*name == '/') name++;
@ -28,13 +28,13 @@ static const char *mapname(const char *name, char *buf)
int shm_open(const char *name, int flag, mode_t mode) int shm_open(const char *name, int flag, mode_t mode)
{ {
char buf[NAME_MAX+10]; char buf[NAME_MAX+10];
if (!(name = mapname(name, buf))) return -1; if (!(name = __shm_mapname(name, buf))) return -1;
return open(name, flag|O_NOFOLLOW|O_CLOEXEC|O_NONBLOCK, mode); return open(name, flag|O_NOFOLLOW|O_CLOEXEC|O_NONBLOCK, mode);
} }
int shm_unlink(const char *name) int shm_unlink(const char *name)
{ {
char buf[NAME_MAX+10]; char buf[NAME_MAX+10];
if (!(name = mapname(name, buf))) return -1; if (!(name = __shm_mapname(name, buf))) return -1;
return unlink(name); return unlink(name);
} }

View file

@ -11,164 +11,155 @@
#include <sys/stat.h> #include <sys/stat.h>
#include <stdlib.h> #include <stdlib.h>
#include <pthread.h> #include <pthread.h>
#include "libc.h"
char *__shm_mapname(const char *, char *);
static struct { static struct {
ino_t ino; ino_t ino;
sem_t *sem; sem_t *sem;
int refcnt; int refcnt;
} *semtab; } *semtab;
static int lock[2];
static int semcnt; #define FLAGS (O_RDWR|O_NOFOLLOW|O_CLOEXEC|O_NONBLOCK)
static pthread_spinlock_t lock;
static pthread_once_t once;
static void init()
{
semtab = calloc(sizeof *semtab, SEM_NSEMS_MAX);
}
static sem_t *find_map(ino_t ino)
{
int i;
for (i=0; i<SEM_NSEMS_MAX && semtab[i].ino != ino; i++);
if (i==SEM_NSEMS_MAX) return 0;
if (semtab[i].refcnt == INT_MAX) return (sem_t *)-1;
semtab[i].refcnt++;
return semtab[i].sem;
}
sem_t *sem_open(const char *name, int flags, ...) sem_t *sem_open(const char *name, int flags, ...)
{ {
va_list ap; va_list ap;
mode_t mode; mode_t mode;
unsigned value; unsigned value;
int fd, tfd, dir; int fd, i, e, slot, first=1, cnt;
sem_t newsem; sem_t newsem;
void *map; void *map;
char tmp[64]; char tmp[64];
struct timespec ts; struct timespec ts;
struct stat st; struct stat st;
int i; char buf[NAME_MAX+10];
while (*name=='/') name++; if (!(name = __shm_mapname(name, buf)))
if (strchr(name, '/')) { return SEM_FAILED;
errno = EINVAL;
LOCK(lock);
/* Allocate table if we don't have one yet */
if (!semtab && !(semtab = calloc(sizeof *semtab, SEM_NSEMS_MAX))) {
UNLOCK(lock);
return SEM_FAILED; return SEM_FAILED;
} }
pthread_once(&once, init); /* Reserve a slot in case this semaphore is not mapped yet;
if (!semtab) { * this is necessary because there is no way to handle
errno = ENOMEM; * failures after creation of the file. */
slot = -1;
for (cnt=i=0; i<SEM_NSEMS_MAX; i++) {
cnt += semtab[i].refcnt;
if (!semtab[i].sem && slot < 0) slot = i;
}
/* Avoid possibility of overflow later */
if (cnt == INT_MAX || slot < 0) {
errno = EMFILE;
UNLOCK(lock);
return SEM_FAILED; return SEM_FAILED;
} }
/* Dummy pointer to make a reservation */
semtab[slot].sem = (sem_t *)-1;
UNLOCK(lock);
if (flags & O_CREAT) { flags &= (O_CREAT|O_EXCL);
va_start(ap, flags);
mode = va_arg(ap, mode_t) & 0666; /* Early failure check for exclusive open; otherwise the case
value = va_arg(ap, unsigned); * where the semaphore already exists is expensive. */
va_end(ap); if (flags == (O_CREAT|O_EXCL) && access(name, F_OK) == 0) {
if (value > SEM_VALUE_MAX) { errno = EEXIST;
errno = EINVAL; return SEM_FAILED;
return SEM_FAILED;
}
sem_init(&newsem, 1, value);
clock_gettime(CLOCK_REALTIME, &ts);
snprintf(tmp, sizeof(tmp), "/dev/shm/%p-%p-%d-%d",
&name, name, (int)getpid(), (int)ts.tv_nsec);
tfd = open(tmp, O_CREAT|O_EXCL|O_RDWR|O_CLOEXEC, mode);
if (tfd<0) return SEM_FAILED;
dir = open("/dev/shm", O_DIRECTORY|O_RDONLY|O_CLOEXEC);
if (dir<0 || write(tfd,&newsem,sizeof newsem)!=sizeof newsem) {
if (dir >= 0) close(dir);
close(tfd);
unlink(tmp);
return SEM_FAILED;
}
} }
flags &= ~O_ACCMODE;
flags |= O_RDWR;
pthread_spin_lock(&lock);
for (;;) { for (;;) {
if (!(flags & O_EXCL)) { /* If exclusive mode is not requested, try opening an
fd = shm_open(name, flags&~O_CREAT, 0); * existing file first and fall back to creation. */
if (fd >= 0 || errno != ENOENT) { if (flags != (O_CREAT|O_EXCL)) {
if (flags & O_CREAT) { fd = open(name, FLAGS);
close(dir); if (fd >= 0) {
close(tfd); if ((map = mmap(0, sizeof(sem_t), PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0)) == MAP_FAILED ||
unlink(tmp); fstat(fd, &st) < 0) {
}
if (fd >= 0 && fstat(fd, &st) < 0) {
close(fd); close(fd);
fd = -1;
}
if (fd < 0) {
pthread_spin_unlock(&lock);
return SEM_FAILED; return SEM_FAILED;
} }
if ((map = find_map(st.st_ino))) { close(fd);
pthread_spin_unlock(&lock);
close(fd);
if (map == (sem_t *)-1)
return SEM_FAILED;
return map;
}
break; break;
} }
if (errno != ENOENT)
return SEM_FAILED;
} }
if (!(flags & O_CREAT)) { if (!(flags & O_CREAT))
pthread_spin_unlock(&lock); return SEM_FAILED;
if (first) {
first = 0;
va_start(ap, flags);
mode = va_arg(ap, mode_t) & 0666;
value = va_arg(ap, unsigned);
va_end(ap);
if (value > SEM_VALUE_MAX) {
errno = EINVAL;
return SEM_FAILED;
}
sem_init(&newsem, 1, value);
}
/* Create a temp file with the new semaphore contents
* and attempt to atomically link it as the new name */
clock_gettime(CLOCK_REALTIME, &ts);
snprintf(tmp, sizeof(tmp), "/dev/shm/tmp-%d", (int)ts.tv_nsec);
fd = open(tmp, O_CREAT|O_EXCL|FLAGS, mode);
if (fd < 0) {
if (errno == EEXIST) continue;
return SEM_FAILED; return SEM_FAILED;
} }
if (!linkat(AT_FDCWD, tmp, dir, name, 0)) { if (write(fd, &newsem, sizeof newsem) != sizeof newsem || fstat(fd, &st) < 0 ||
fd = tfd; (map = mmap(0, sizeof(sem_t), PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0)) == MAP_FAILED) {
close(dir); close(fd);
unlink(tmp);
break;
}
if ((flags & O_EXCL) || errno != EEXIST) {
close(dir);
close(tfd);
unlink(tmp); unlink(tmp);
return SEM_FAILED; return SEM_FAILED;
} }
}
if (fstat(fd, &st) < 0) {
pthread_spin_unlock(&lock);
close(fd); close(fd);
return SEM_FAILED; if (link(tmp, name) == 0) break;
e = errno;
unlink(tmp);
/* Failure is only fatal when doing an exclusive open;
* otherwise, next iteration will try to open the
* existing file. */
if (e != EEXIST || flags == (O_CREAT|O_EXCL))
return SEM_FAILED;
} }
if (semcnt == SEM_NSEMS_MAX) {
pthread_spin_unlock(&lock); /* See if the newly mapped semaphore is already mapped. If
close(fd); * so, unmap the new mapping and use the existing one. Otherwise,
errno = EMFILE; * add it to the table of mapped semaphores. */
return SEM_FAILED; LOCK(lock);
for (i=0; i<SEM_NSEMS_MAX && semtab[i].ino != st.st_ino; i++);
if (i<SEM_NSEMS_MAX) {
munmap(map, sizeof(sem_t));
semtab[i].refcnt++;
UNLOCK(lock);
return semtab[i].sem;
} }
for (i=0; i<SEM_NSEMS_MAX && semtab[i].sem; i++); semtab[slot].refcnt = 1;
map = mmap(0, sizeof(sem_t), PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0); semtab[slot].sem = map;
close(fd); semtab[slot].ino = st.st_ino;
if (map == MAP_FAILED) { UNLOCK(lock);
pthread_spin_unlock(&lock);
return SEM_FAILED;
}
semtab[i].ino = st.st_ino;
semtab[i].sem = map;
semtab[i].refcnt = 1;
pthread_spin_unlock(&lock);
return map; return map;
} }
int sem_close(sem_t *sem) int sem_close(sem_t *sem)
{ {
int i; int i;
pthread_spin_lock(&lock); LOCK(lock);
for (i=0; i<SEM_NSEMS_MAX && semtab[i].sem != sem; i++); for (i=0; i<SEM_NSEMS_MAX && semtab[i].sem != sem; i++);
if (!--semtab[i].refcnt) { if (!--semtab[i].refcnt) {
semtab[i].sem = 0; semtab[i].sem = 0;
semtab[i].ino = 0; semtab[i].ino = 0;
} }
pthread_spin_unlock(&lock); UNLOCK(lock);
return munmap(sem, sizeof *sem); munmap(sem, sizeof *sem);
return 0;
} }