From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from goalie.tycho.ncsc.mil (goalie [144.51.242.250]) by tarius.tycho.ncsc.mil (8.14.4/8.14.4) with ESMTP id v3PKZUBP008767 for ; Tue, 25 Apr 2017 16:35:30 -0400 Message-ID: <1493152517.17316.0.camel@trentalancia.net> Subject: [PATCH v3] libsemanage: remove lock files From: Guido Trentalancia To: selinux@tycho.nsa.gov Date: Tue, 25 Apr 2017 22:35:17 +0200 In-Reply-To: <1493150773.12050.2.camel@trentalancia.net> References: <58517705.198270.1492699110308@pim.register.it> <100DD2AB-228E-47B5-8058-C9F030AEA665@trentalancia.net> <201704251630.03844.russell@coker.com.au> <1493150773.12050.2.camel@trentalancia.net> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 List-Id: "Security-Enhanced Linux \(SELinux\) mailing list" List-Post: List-Help: Do not use flock() for file locking, but instead use generic text files that keep track of the process ID (PID) of the locking process. Remove semanage read and transaction lock files upon releasing them. This third version fixes a bug in the previous version and also applies cleanly to the latest git tree. Signed-off-by: Guido Trentalancia --- src/Makefile | 2 src/semanage_store.c | 214 +++++++++++++++++++++++++++++++++++++-------------- 2 files changed, 160 insertions(+), 56 deletions(-) --- a/src/Makefile 2017-04-25 22:27:38.105555427 +0200 +++ b/src/Makefile 2017-04-25 22:28:58.512555098 +0200 @@ -91,7 +91,7 @@ $(LIBA): $(OBJS) $(RANLIB) $@ $(LIBSO): $(LOBJS) - $(CC) $(CFLAGS) $(LDFLAGS) -shared -o $@ $^ -lsepol -laudit -lselinux -lbz2 -Wl,-soname,$(LIBSO),--version-script=libsemanage.map,-z,defs + $(CC) $(CFLAGS) $(LDFLAGS) -shared -o $@ $^ -lm -lsepol -laudit -lselinux -lbz2 -Wl,-soname,$(LIBSO),--version-script=libsemanage.map,-z,defs ln -sf $@ $(TARGET) $(LIBPC): $(LIBPC).in ../VERSION --- a/src/semanage_store.c 2017-04-20 16:30:21.218209972 +0200 +++ b/src/semanage_store.c 2017-04-25 22:24:35.883556172 +0200 @@ -45,6 +45,8 @@ typedef struct dbase_policydb dbase_t; #include #include #include +#include +#include #include #include #include @@ -52,11 +54,24 @@ typedef struct dbase_policydb dbase_t; #include #include #include +#include #include #include #include #include +#include + +#ifndef CONFIG_BASE_SMALL +#define CONFIG_BASE_SMALL 0 +#endif + +#include + +#ifndef PID_MAX_DEFAULT +#define PID_MAX_DEFAULT 32768 +#endif + #include "debug.h" #include "utilities.h" @@ -76,6 +91,8 @@ enum semanage_file_defs { static char *semanage_paths[SEMANAGE_NUM_STORES][SEMANAGE_STORE_NUM_PATHS]; static char *semanage_files[SEMANAGE_NUM_FILES] = { NULL }; static int semanage_paths_initialized = 0; +static int pid_max; +static ssize_t pid_max_length; /* These are paths relative to the bottom of the module store */ static const char *semanage_relative_files[SEMANAGE_NUM_FILES] = { @@ -427,8 +442,23 @@ cleanup: int semanage_check_init(semanage_handle_t *sh, const char *prefix) { int rc; + int fd; + char root[PATH_MAX]; + ssize_t amount_read; + if (semanage_paths_initialized == 0) { - char root[PATH_MAX]; + pid_max = PID_MAX_DEFAULT; + pid_max_length = ceil(log10(PID_MAX_DEFAULT + 1)); + + fd = open("/proc/sys/kernel/pid_max", O_RDONLY); + if (fd > 0) { + char sysctlstring[pid_max_length]; + amount_read = read(fd, sysctlstring, pid_max_length); + if (amount_read > 0) { + pid_max = atoi(sysctlstring); + pid_max_length = ceil(log10(pid_max + 1)); + } + } rc = snprintf(root, sizeof(root), @@ -528,16 +558,23 @@ char *semanage_conf_path(void) /**************** functions that create module store ***************/ -/* Check that the semanage store exists. If 'create' is non-zero then - * create the directories. Returns 0 if module store exists (either - * already or just created), -1 if does not exist or could not be - * read, or -2 if it could not create the store. */ +/* Check that the semanage store exists and that the read lock can be + * taken. If 'create' is non-zero then it creates the directories + * and the lock file. Returns 0 if the module store exists (either + * already or just created) and the read lock can be taken, -1 if it + * does not exist or it is not possible to read from it, or -2 if it + * could not create the store or it could not take the lock file. */ int semanage_create_store(semanage_handle_t * sh, int create) { struct stat sb; int mode_mask = R_OK | W_OK | X_OK; const char *path = semanage_files[SEMANAGE_ROOT]; int fd; + pid_t pid, lock_pid; + char *pid_string, *lock_pid_string; + size_t pid_length; + ssize_t pid_bytes; + int invalid_lock = 0; if (stat(path, &sb) == -1) { if (errno == ENOENT && create) { @@ -607,24 +644,81 @@ int semanage_create_store(semanage_handl return -1; } } + pid = getpid(); + pid_string = malloc(pid_max_length * sizeof(char)); + sprintf(pid_string, "%d", pid); + pid_length = strlen(pid_string); path = semanage_files[SEMANAGE_READ_LOCK]; if (stat(path, &sb) == -1) { if (errno == ENOENT && create) { if ((fd = creat(path, S_IRUSR | S_IWUSR)) == -1) { ERR(sh, "Could not create lock file at %s.", path); + free(pid_string); return -2; } + pid_bytes = write(fd, pid_string, pid_length); close(fd); + free(pid_string); + if (pid_bytes == (ssize_t) pid_length) + return 0; + else { + ERR(sh, "Could not create lock file at %s.", path); + return -2; + } } else { ERR(sh, "Could not read lock file at %s.", path); + free(pid_string); return -1; } } else { if (!S_ISREG(sb.st_mode) || access(path, R_OK | W_OK) == -1) { ERR(sh, "Could not access lock file at %s.", path); + free(pid_string); return -1; - } + } else { + fd = open(path, O_RDWR); + if (fd > 0) { + lock_pid_string = malloc(pid_max_length * sizeof(char)); + pid_bytes = read(fd, lock_pid_string, pid_max_length); + if (pid_bytes > 0) { + lock_pid = (pid_t) atoi(lock_pid_string); + if (lock_pid > pid_max) + invalid_lock = 1; + else if (kill(lock_pid, 0) != 0) + if (errno == ESRCH) + invalid_lock = 1; + + if (!invalid_lock) { + ERR(sh, "Could not create lock file at %s.", path); + close(fd); + free(pid_string); + free(lock_pid_string); + return -2; + } + } else + invalid_lock = 1; + + if (invalid_lock) { + pid_bytes = pwrite(fd, pid_string, pid_length, 0); + close(fd); + free(pid_string); + free(lock_pid_string); + if (pid_bytes == (ssize_t) pid_length) + return 0; + else { + ERR(sh, "Could not create lock file at %s.", path); + return -2; + } + } else { + ERR(sh, "Could not create lock file at %s.", path); + close(fd); + free(pid_string); + free(lock_pid_string); + return -2; + } + } + } } return 0; } @@ -1795,64 +1884,74 @@ int semanage_install_sandbox(semanage_ha static int semanage_get_lock(semanage_handle_t * sh, const char *lock_name, const char *lock_file) { + pid_t pid, lock_pid; + char *pid_string, *lock_pid_string; int fd; - struct timeval origtime, curtime; + size_t pid_length; + ssize_t pid_bytes; int got_lock = 0; + int overwritten_lock = 0; - if ((fd = open(lock_file, O_RDONLY)) == -1) { - if ((fd = - open(lock_file, O_RDWR | O_CREAT | O_TRUNC, - S_IRUSR | S_IWUSR)) == -1) { + pid = getpid(); + pid_string = malloc(pid_max_length * sizeof(char)); + sprintf(pid_string, "%d", pid); + pid_length = strlen(pid_string); + if ((fd = open(lock_file, O_RDWR | O_CREAT, S_IRUSR | S_IWUSR)) == -1) { ERR(sh, "Could not open direct %s at %s.", lock_name, lock_file); + free(pid_string); return -1; + } else { + lock_pid_string = malloc(pid_max_length * sizeof(char)); + pid_bytes = pread(fd, lock_pid_string, pid_max_length, 0); + if (pid_bytes == 0) { + overwritten_lock = 1; + pid_bytes = write(fd, pid_string, pid_length); + if (pid_bytes == (ssize_t) pid_length) + got_lock = 1; + } else { + lock_pid = (pid_t) atoi(lock_pid_string); + if (lock_pid > pid_max) { + overwritten_lock = 1; + pid_bytes = pwrite(fd, pid_string, pid_length, 0); + if (pid_bytes == (ssize_t) pid_length) + got_lock = 1; + } else { + if (lock_pid == pid) + got_lock = 1; + else if (kill(lock_pid, 0) != 0) + if (errno == ESRCH) { + overwritten_lock = 1; + pid_bytes = pwrite(fd, pid_string, pid_length, 0); + if (pid_bytes == (ssize_t) pid_length) + got_lock = 1; + } + } } } - if (fcntl(fd, F_SETFD, FD_CLOEXEC) < 0) { - ERR(sh, "Could not set close-on-exec for %s at %s.", lock_name, - lock_file); + + if (!got_lock) { close(fd); + free(pid_string); + free(lock_pid_string); + if (overwritten_lock) + unlink(lock_file); + ERR(sh, "Could not get direct %s at %s.", lock_name, lock_file); return -1; } - if (sh->timeout == 0) { - /* return immediately */ - origtime.tv_sec = 0; - } else { - origtime.tv_sec = sh->timeout; - } - origtime.tv_usec = 0; - do { - curtime.tv_sec = 1; - curtime.tv_usec = 0; - if (flock(fd, LOCK_EX | LOCK_NB) == 0) { - got_lock = 1; - break; - } else if (errno != EAGAIN) { - ERR(sh, "Error obtaining direct %s at %s.", lock_name, - lock_file); - close(fd); - return -1; - } - if (origtime.tv_sec > 0 || sh->timeout == -1) { - if (select(0, NULL, NULL, NULL, &curtime) == -1) { - if (errno == EINTR) { - continue; - } - ERR(sh, - "Error while waiting to get direct %s at %s.", - lock_name, lock_file); - close(fd); - return -1; - } - origtime.tv_sec--; - } - } while (origtime.tv_sec > 0 || sh->timeout == -1); - if (!got_lock) { - ERR(sh, "Could not get direct %s at %s.", lock_name, lock_file); + if (fcntl(fd, F_SETFD, FD_CLOEXEC) < 0) { + ERR(sh, "Could not set close-on-exec for %s at %s.", lock_name, + lock_file); close(fd); + free(pid_string); + free(lock_pid_string); + unlink(lock_file); return -1; } + + free(pid_string); + free(lock_pid_string); return fd; } @@ -1901,29 +2000,27 @@ int semanage_get_active_lock(semanage_ha } } -/* Releases the transaction lock. Does nothing if there was not one already - * there. */ +/* Releases the transaction lock: the lock file is removed */ void semanage_release_trans_lock(semanage_handle_t * sh) { int errsv = errno; if (sh->u.direct.translock_file_fd >= 0) { - flock(sh->u.direct.translock_file_fd, LOCK_UN); close(sh->u.direct.translock_file_fd); sh->u.direct.translock_file_fd = -1; } + unlink(semanage_files[SEMANAGE_TRANS_LOCK]); errno = errsv; } -/* Releases the read lock. Does nothing if there was not one already - * there. */ +/* Releases the read lock: the lock file is removed */ void semanage_release_active_lock(semanage_handle_t * sh) { int errsv = errno; if (sh->u.direct.activelock_file_fd >= 0) { - flock(sh->u.direct.activelock_file_fd, LOCK_UN); close(sh->u.direct.activelock_file_fd); sh->u.direct.activelock_file_fd = -1; } + unlink(semanage_files[SEMANAGE_READ_LOCK]); errno = errsv; }