recursive remounts: add fallback for old kernels, some improvements

* vanilla kernels before 3.15 don't expose a mount id in /proc/pid/fdinfo files.
  This is still relevant on Ubuntu 14.04 with 3.13 kernel, CentOS 7 doesn't
  have this problem. In this case fall back to simple a remount and print
  a warning.
* drop euid switching as it doesn't really serve a purpose here (paths are not opened
  in reading or writing mode, and we are not doing anything with it) and potentially
  causes problems when suid programs are sandboxed
* more rigorous error handling

2 files changed, 57 insertions, 28 deletions

diff --git a/src/firejail/fs.c b/src/firejail/fs.c
index eda46d127..38af1fccf 100644
--- a/src/firejail/fs.c
+++ b/src/firejail/fs.c
@@ -34,7 +34,7 @@
 //#define TEST_NO_BLACKLIST_MATCHING
 
 
-
+static int mount_warning = 0; // remember if warning was printed already
 static void fs_rdwr(const char *dir);
 static void fs_rdwr_rec(const char *dir);
 
@@ -482,18 +482,23 @@ void fs_rdonly(const char *dir) {
 // remount directory read-only recursively
 void fs_rdonly_rec(const char *dir) {
         assert(dir);
-        EUID_USER();
         // get mount point of the directory
         int mountid = get_mount_id(dir);
-        if (mountid == -1) {
-                EUID_ROOT();
+        if (mountid == -1)
+                return;
+        if (mountid == -2) {
+                // falling back to a simple remount on old kernels
+                if (!mount_warning) {
+                        fwarning("read-only, read-write and noexec options are not applied recursively\n");
+                        mount_warning = 1;
+                }
+                fs_rdonly(dir);
                 return;
         }
         // build array with all mount points that need to get remounted
         char **arr = build_mount_array(mountid, dir);
         assert(arr);
         // remount
-        EUID_ROOT();
         char **tmp = arr;
         while (*tmp) {
                 fs_rdonly(*tmp);
@@ -540,18 +545,23 @@ static void fs_rdwr(const char *dir) {
 // remount directory read-write recursively
 static void fs_rdwr_rec(const char *dir) {
         assert(dir);
-        EUID_USER();
         // get mount point of the directory
         int mountid = get_mount_id(dir);
-        if (mountid == -1) {
-                EUID_ROOT();
+        if (mountid == -1)
+                return;
+        if (mountid == -2) {
+                // falling back to a simple remount on old kernels
+                if (!mount_warning) {
+                        fwarning("read-only, read-write and noexec options are not applied recursively\n");
+                        mount_warning = 1;
+                }
+                fs_rdwr(dir);
                 return;
         }
         // build array with all mount points that need to get remounted
         char **arr = build_mount_array(mountid, dir);
         assert(arr);
         // remount
-        EUID_ROOT();
         char **tmp = arr;
         while (*tmp) {
                 fs_rdwr(*tmp);
@@ -586,18 +596,23 @@ void fs_noexec(const char *dir) {
 // remount directory noexec, nodev, nosuid recursively
 void fs_noexec_rec(const char *dir) {
         assert(dir);
-        EUID_USER();
         // get mount point of the directory
         int mountid = get_mount_id(dir);
-        if (mountid == -1) {
-                EUID_ROOT();
+        if (mountid == -1)
+                return;
+        if (mountid == -2) {
+                // falling back to a simple remount on old kernels
+                if (!mount_warning) {
+                        fwarning("read-only, read-write and noexec options are not applied recursively\n");
+                        mount_warning = 1;
+                }
+                fs_noexec(dir);
                 return;
         }
         // build array with all mount points that need to get remounted
         char **arr = build_mount_array(mountid, dir);
         assert(arr);
         // remount
-        EUID_ROOT();
         char **tmp = arr;
         while (*tmp) {
                 fs_noexec(*tmp);
diff --git a/src/firejail/mountinfo.c b/src/firejail/mountinfo.c
index b7e6c6fdd..ab1e501a7 100644
--- a/src/firejail/mountinfo.c
+++ b/src/firejail/mountinfo.c
@@ -153,7 +153,6 @@ MountData *get_last_mount(void) {
 
 // Extract the mount id from /proc/self/fdinfo and return it.
 int get_mount_id(const char *path) {
-        EUID_ASSERT();
         int fd = open(path, O_PATH|O_CLOEXEC);
         if (fd == -1)
                 return -1;
@@ -161,32 +160,41 @@ int get_mount_id(const char *path) {
         char *fdinfo;
         if (asprintf(&fdinfo, "/proc/self/fdinfo/%d", fd) == -1)
                 errExit("asprintf");
-        EUID_ROOT();
         FILE *fp = fopen(fdinfo, "re");
-        EUID_USER();
-        if (!fp)
-                goto errexit;
+        if (!fp) {
+                perror("fopen");
+                fprintf(stderr, "Error: cannot open %s\n", fdinfo);
+                exit(1);
+        }
 
         // read the file
         char buf[MAX_BUF];
-        while (fgets(buf, MAX_BUF, fp)) {
+        if (fgets(buf, MAX_BUF, fp) == NULL) {
+                fprintf(stderr, "Error: cannot read %s\n", fdinfo);
+                exit(1);
+        }
+        do {
                 if (strncmp(buf, "mnt_id:", 7) == 0) {
                         char *ptr = buf + 7;
                         while (*ptr != '\0' && (*ptr == ' ' || *ptr == '\t')) {
                                 ptr++;
                         }
-                        if (*ptr == '\0')
-                                goto errexit;
+                        if (*ptr == '\0') {
+                                fprintf(stderr, "Error: cannot read %s\n", fdinfo);
+                                exit(1);
+                        }
                         fclose(fp);
                         close(fd);
                         free(fdinfo);
                         return atoi(ptr);
                 }
-        }
+        } while (fgets(buf, MAX_BUF, fp));
 
-errexit:
-        fprintf(stderr, "Error: cannot read %s\n", fdinfo);
-        exit(1);
+        // fallback, kernels older than 3.15 don't expose the mount id in this place
+        fclose(fp);
+        close(fd);
+        free(fdinfo);
+        return -2;
 }
 
 // Return array with all paths that might need a remount.
@@ -208,7 +216,11 @@ char **build_mount_array(const int mountid, const char *path) {
         // read /proc/self/mountinfo
         size_t pathlen = strlen(path);
         int found = 0;
-        while (fgets(mbuf, MAX_BUF, fp)) {
+        if (fgets(mbuf, MAX_BUF, fp) == NULL) {
+                fprintf(stderr, "Error: cannot read /proc/self/mountinfo\n");
+                exit(1);
+        }
+        do {
                 // find mount point with mount id
                 if (!found) {
                         parse_line(mbuf, &mdata);
@@ -230,7 +242,8 @@ char **build_mount_array(const int mountid, const char *path) {
                         else
                                 continue;
                 }
-                // from here on add all mount points below path
+                // from here on add all mount points below path,
+                // don't remount blacklisted paths
                 parse_line(mbuf, &mdata);
                 if (strncmp(mdata.dir, path, pathlen) == 0 &&
                     mdata.dir[pathlen] == '/' &&
@@ -248,7 +261,8 @@ char **build_mount_array(const int mountid, const char *path) {
                                 errExit("strdup");
                         cnt++;
                 }
-        }
+        } while (fgets(mbuf, MAX_BUF, fp));
+
         if (cnt == size) {
                 size++;
                 rv = realloc(rv, size * sizeof(*rv));