From 316a1cd6227e259908287a768953b1171cd9477d Mon Sep 17 00:00:00 2001 From: smitsohu Date: Mon, 18 Oct 2021 18:01:46 +0200 Subject: mountinfo: improve readability Removes the inconsistency that some blacklisted paths could be remounted (files specified explicitly) and some could not. Now all blacklisted paths can be mounted nosuid, nodev, noexec if users specify this. Also fixes the bug that mount id can indeed be 0. Other than that no functional or algorithmic changes, only readability improvements. --- src/firejail/firejail.h | 2 +- src/firejail/fs.c | 11 ++-- src/firejail/mountinfo.c | 149 ++++++++++++++++++++--------------------------- 3 files changed, 71 insertions(+), 91 deletions(-) (limited to 'src') diff --git a/src/firejail/firejail.h b/src/firejail/firejail.h index 13b7b9523..a6924b830 100644 --- a/src/firejail/firejail.h +++ b/src/firejail/firejail.h @@ -567,7 +567,7 @@ typedef struct { // mountinfo.c MountData *get_last_mount(void); int get_mount_id(int fd); -char **build_mount_array(const int mount_id, const char *path); +char **build_mount_array(const int mountid, const char *path); // fs_var.c void fs_var_log(void); // mounting /var/log diff --git a/src/firejail/fs.c b/src/firejail/fs.c index 0b9a38035..9c1b889ed 100644 --- a/src/firejail/fs.c +++ b/src/firejail/fs.c @@ -654,12 +654,13 @@ static void fs_remount_rec(const char *path, OPERATION op) { // build array with all mount points that need to get remounted char **arr = build_mount_array(mountid, path); - assert(arr); + if (!arr) + return; // remount - char **tmp = arr; - while (*tmp) { - fs_remount_simple(*tmp, op); - free(*tmp++); + int i; + for (i = 0; arr[i]; i++) { + fs_remount_simple(arr[i], op); + free(arr[i]); } free(arr); } diff --git a/src/firejail/mountinfo.c b/src/firejail/mountinfo.c index 304f80eee..ee437e10b 100644 --- a/src/firejail/mountinfo.c +++ b/src/firejail/mountinfo.c @@ -33,43 +33,38 @@ static MountData mdata; // Convert octal escape sequence to decimal value -static int read_oct(const char *path) { - int dec = 0; - int digit, i; - // there are always exactly three octal digits - for (i = 1; i < 4; i++) { - digit = *(path + i); - if (digit < '0' || digit > '7') { - fprintf(stderr, "Error: cannot read /proc/self/mountinfo\n"); - exit(1); - } - dec = (dec << 3) + (digit - '0'); - } - return dec; +static unsigned read_oct(char *s) { + assert(s[0] == '\\'); + s++; + + int i; + for (i = 0; i < 3; i++) + assert(s[i] >= '0' && s[i] <= '7'); + + return ((s[0] - '0') << 6 | + (s[1] - '0') << 3 | + (s[2] - '0') << 0); } // Restore empty spaces in pathnames extracted from /proc/self/mountinfo static void unmangle_path(char *path) { - char *p = strchr(path, '\\'); - if (p && read_oct(p) == ' ') { - *p = ' '; - int i = 3; - do { - p++; - if (*(p + i) == '\\' && read_oct(p + i) == ' ') { - *p = ' '; - i += 3; - } - else - *p = *(p + i); - } while (*p); - } + char *r = strchr(path, '\\'); + if (!r) + return; + + char *w = r; + do { + while (*r == '\\') { + *w++ = read_oct(r); + r += 4; + } + *w++ = *r; + } while (*r++); } // Parse a line from /proc/self/mountinfo, // the function does an exit(1) if anything goes wrong. static void parse_line(char *line, MountData *output) { - assert(line && output); memset(output, 0, sizeof(*output)); // extract mount id, filesystem name, directory and filesystem types // examples: @@ -87,8 +82,6 @@ static void parse_line(char *line, MountData *output) { char *ptr = strtok(line, " "); if (!ptr) goto errexit; - if (ptr != line) - goto errexit; output->mountid = atoi(ptr); int cnt = 1; @@ -109,10 +102,9 @@ static void parse_line(char *line, MountData *output) { ptr = strtok(NULL, " "); if (!ptr) goto errexit; - output->fstype = ptr++; + output->fstype = ptr; - - if (output->mountid == 0 || + if (output->mountid < 0 || output->fsname == NULL || output->dir == NULL || output->fstype == NULL) @@ -195,9 +187,9 @@ static int get_mount_id_from_fdinfo(int fd) { char buf[MAX_BUF]; while (fgets(buf, MAX_BUF, fp)) { if (strncmp(buf, "mnt_id:", 7) == 0) { - if (sscanf(buf + 7, "%d", &rv) != 1) - goto errexit; - break; + if (sscanf(buf + 7, "%d", &rv) == 1) + break; + goto errexit; } } @@ -219,62 +211,50 @@ int get_mount_id(int fd) { // Check /proc/self/mountinfo if path contains any mounts points. // Returns an array that can be iterated over for recursive remounting. -char **build_mount_array(const int mount_id, const char *path) { +char **build_mount_array(const int mountid, const char *path) { assert(path); - // open /proc/self/mountinfo FILE *fp = fopen("/proc/self/mountinfo", "re"); if (!fp) { fprintf(stderr, "Error: cannot read /proc/self/mountinfo\n"); exit(1); } - // array to be returned - size_t cnt = 0; + // try to find line with mount id + int found = 0; + MountData mntp; + char line[MAX_BUF]; + while (fgets(line, MAX_BUF, fp)) { + parse_line(line, &mntp); + if (mntp.mountid == mountid) { + found = 1; + break; + } + } + + if (!found) { + fclose(fp); + return NULL; + } + + // allocate array size_t size = 32; char **rv = malloc(size * sizeof(*rv)); if (!rv) errExit("malloc"); - // read /proc/self/mountinfo - size_t pathlen = strlen(path); - char buf[MAX_BUF]; - MountData mntp; - int found = 0; + // add directory itself + size_t cnt = 0; + rv[cnt] = strdup(path); + if (rv[cnt] == NULL) + errExit("strdup"); - if (fgets(buf, MAX_BUF, fp) == NULL) { - fprintf(stderr, "Error: cannot read /proc/self/mountinfo\n"); - exit(1); - } - do { - parse_line(buf, &mntp); - // find mount point with mount id - if (!found) { - if (mntp.mountid == mount_id) { - // give up if mount id has been reassigned, - // don't remount blacklisted path - if (strncmp(mntp.dir, path, strlen(mntp.dir)) || - strstr(mntp.fsname, "firejail.ro.dir") || - strstr(mntp.fsname, "firejail.ro.file")) - break; - - rv[cnt] = strdup(path); - if (rv[cnt] == NULL) - errExit("strdup"); - cnt++; - found = 1; - continue; - } - continue; - } - // from here on add all mount points below path, - // don't remount blacklisted paths - if (strncmp(mntp.dir, path, pathlen) == 0 && - mntp.dir[pathlen] == '/' && - strstr(mntp.fsname, "firejail.ro.dir") == NULL && - strstr(mntp.fsname, "firejail.ro.file") == NULL) { - - if (cnt == size) { + // and add all following mountpoints contained in this directory + size_t pathlen = strlen(path); + while (fgets(line, MAX_BUF, fp)) { + parse_line(line, &mntp); + if (strncmp(mntp.dir, path, pathlen) == 0 && mntp.dir[pathlen] == '/') { + if (++cnt == size) { size *= 2; rv = realloc(rv, size * sizeof(*rv)); if (!rv) @@ -283,18 +263,17 @@ char **build_mount_array(const int mount_id, const char *path) { rv[cnt] = strdup(mntp.dir); if (rv[cnt] == NULL) errExit("strdup"); - cnt++; } - } while (fgets(buf, MAX_BUF, fp)); + } + fclose(fp); - if (cnt == size) { - size++; + // end of array + if (++cnt == size) { + ++size; rv = realloc(rv, size * sizeof(*rv)); if (!rv) errExit("realloc"); } - rv[cnt] = NULL; // end of the array - - fclose(fp); + rv[cnt] = NULL; return rv; } -- cgit v1.2.3-70-g09d2