From 8da6cd6d27941d7b3b67ea1878e32a0c20206468 Mon Sep 17 00:00:00 2001 From: Mathieu Desnoyers Date: Mon, 29 Aug 2011 10:07:37 -0400 Subject: [PATCH] shm creation: use temporary name with O_CREAT | O_EXCL Tolerate that a rogue process could try inhibiting buffer creation by reserving the shm name. Fix this by using a random suffix. Signed-off-by: Mathieu Desnoyers --- libringbuffer/shm.c | 47 ++++++++++++++++++++++++++++++++------------- 1 file changed, 34 insertions(+), 13 deletions(-) diff --git a/libringbuffer/shm.c b/libringbuffer/shm.c index 5639038..d86abb9 100644 --- a/libringbuffer/shm.c +++ b/libringbuffer/shm.c @@ -13,6 +13,9 @@ #include /* For mode constants */ #include /* For O_* constants */ #include +#include +#include +#include #include struct shm_object_table *shm_object_table_create(size_t max_nb_obj) @@ -31,6 +34,8 @@ struct shm_object *shm_object_table_append(struct shm_object_table *table, int shmfd, waitfd[2], ret, i; struct shm_object *obj; char *memory_map; + char tmp_name[NAME_MAX] = "ust-shm-tmp-XXXXXX"; + sigset_t all_sigs, orig_sigs; if (table->allocated_len >= table->size) return NULL; @@ -59,6 +64,18 @@ struct shm_object *shm_object_table_append(struct shm_object_table *table, /* shm_fd: create shm */ + /* + * Theoretically, we could leak a shm if the application crashes + * between open and unlink. Disable signals on this thread for + * increased safety against this scenario. + */ + sigfillset(&all_sigs); + ret = pthread_sigmask(SIG_BLOCK, &all_sigs, &orig_sigs); + if (ret == -1) { + PERROR("pthread_sigmask"); + goto error_pthread_sigmask; + } + /* * Allocate shm, and immediately unlink its shm oject, keeping * only the file descriptor as a reference to the object. If it @@ -67,30 +84,34 @@ struct shm_object *shm_object_table_append(struct shm_object_table *table, * We specifically do _not_ use the / at the beginning of the * pathname so that some OS implementations can keep it local to * the process (POSIX leaves this implementation-defined). - * Ignore the shm_unlink errors, because we handle leaks that - * could occur by applications crashing between shm_open and - * shm_unlink by unlinking the shm before every open. Therefore, - * we can only leak one single shm (and only if the application - * crashes between shm_open and the following shm_unlink). */ do { - ret = shm_unlink("ust-shm-tmp"); - if (ret < 0 && errno != ENOENT) { - PERROR("shm_unlink"); - goto error_shm_unlink; + /* + * Using mktemp filename with O_CREAT | O_EXCL open + * flags. + */ + mktemp(tmp_name); + if (tmp_name[0] == '\0') { + PERROR("mktemp"); + goto error_shm_open; } - shmfd = shm_open("ust-shm-tmp", + shmfd = shm_open(tmp_name, O_CREAT | O_EXCL | O_RDWR, 0700); - } while (shmfd < 0 && errno == EEXIST); + } while (shmfd < 0 && (errno == EEXIST || errno == EACCES)); if (shmfd < 0) { PERROR("shm_open"); goto error_shm_open; } - ret = shm_unlink("ust-shm-tmp"); + ret = shm_unlink(tmp_name); if (ret < 0 && errno != ENOENT) { PERROR("shm_unlink"); goto error_shm_release; } + ret = pthread_sigmask(SIG_SETMASK, &orig_sigs, NULL); + if (ret == -1) { + PERROR("pthread_sigmask"); + goto error_shm_release; + } ret = ftruncate(shmfd, memory_map_size); if (ret) { PERROR("ftruncate"); @@ -120,8 +141,8 @@ error_shm_release: PERROR("close"); assert(0); } -error_shm_unlink: error_shm_open: +error_pthread_sigmask: error_fcntl: for (i = 0; i < 2; i++) { ret = close(waitfd[i]); -- 2.34.1