cgroup: minor refactor, add v2 support, bugfixes

Adds minimal cgroupv2 support, and fixes
an effective user id assertion in --join
(instead of asserting effective user id
of the user, drop privileges completely
in a child process).

6 files changed, 64 insertions, 47 deletions

diff --git a/src/firejail/cgroup.c b/src/firejail/cgroup.c
index e7ffbca36..38b3c32d3 100644
--- a/src/firejail/cgroup.c
+++ b/src/firejail/cgroup.c
@@ -18,7 +18,8 @@
  * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
 */
 #include "firejail.h"
-#include <sys/stat.h>
+#include <sys/wait.h>
+#include <errno.h>
 
 #define MAXBUF 4096
 
@@ -68,52 +69,60 @@ errout:
                 fclose(fp);
 }
 
+static int is_cgroup_path(const char *fname) {
+        // path starts with /sys/fs/cgroup
+        if (strncmp(fname, "/sys/fs/cgroup", 14) != 0)
+                return 0;
 
-void set_cgroup(const char *path) {
-        EUID_ASSERT();
+        // no .. traversal
+        char *ptr = strstr(fname, "..");
+        if (ptr)
+                return 0;
 
-        invalid_filename(path, 0); // no globbing
+        return 1;
+}
 
-        // path starts with /sys/fs/cgroup
-        if (strncmp(path, "/sys/fs/cgroup", 14) != 0)
-                goto errout;
+void check_cgroup_file(const char *fname) {
+        assert(fname);
+        invalid_filename(fname, 0); // no globbing
 
-        // path ends in tasks
-        char *ptr = strstr(path, "tasks");
-        if (!ptr)
-                goto errout;
-        if (*(ptr + 5) != '\0')
+        if (!is_cgroup_path(fname))
                 goto errout;
 
-        // no .. traversal
-        ptr = strstr(path, "..");
-        if (ptr)
+        const char *base = gnu_basename(fname);
+        if (strcmp(base, "tasks") != 0 &&  // cgroup v1
+            strcmp(base, "cgroup.procs") != 0)
                 goto errout;
 
-        // tasks file exists
-        FILE *fp = fopen(path, "ae");
-        if (!fp)
-                goto errout;
-        // task file belongs to the user running the sandbox
-        int fd = fileno(fp);
-        if (fd == -1)
-                errExit("fileno");
-        struct stat s;
-        if (fstat(fd, &s) == -1)
-                errExit("fstat");
-        if (s.st_uid != getuid() && s.st_gid != getgid())
-                goto errout2;
-        // add the task to cgroup
-        pid_t pid = getpid();
-        int rv = fprintf(fp, "%d\n", pid);
-        (void) rv;
-        fclose(fp);
-        return;
+        if (access(fname, W_OK) == 0)
+                return;
 
 errout:
         fprintf(stderr, "Error: invalid cgroup\n");
         exit(1);
-errout2:
-        fprintf(stderr, "Error: you don't have permissions to use this control group\n");
-        exit(1);
+}
+
+static void do_set_cgroup(const char *fname, pid_t pid) {
+        FILE *fp = fopen(fname, "ae");
+        if (!fp) {
+                fwarning("cannot open %s for writing: %s\n", fname, strerror(errno));
+                return;
+        }
+
+        int rv = fprintf(fp, "%d\n", pid);
+        (void) rv;
+        fclose(fp);
+}
+
+void set_cgroup(const char *fname, pid_t pid) {
+        pid_t child = fork();
+        if (child < 0)
+                errExit("fork");
+        if (child == 0) {
+                drop_privs(0);
+
+                do_set_cgroup(fname, pid);
+                _exit(0);
+        }
+        waitpid(child, NULL, 0);
 }
diff --git a/src/firejail/firejail.h b/src/firejail/firejail.h
index 45075d137..d29c2a976 100644
--- a/src/firejail/firejail.h
+++ b/src/firejail/firejail.h
@@ -647,7 +647,8 @@ void cpu_print_filter(pid_t pid) __attribute__((noreturn));
 // cgroup.c
 void save_cgroup(void);
 void load_cgroup(const char *fname);
-void set_cgroup(const char *path);
+void check_cgroup_file(const char *fname);
+void set_cgroup(const char *fname, pid_t pid);
 
 // output.c
 void check_output(int argc, char **argv);
diff --git a/src/firejail/join.c b/src/firejail/join.c
index a869f6b64..0e76fd944 100644
--- a/src/firejail/join.c
+++ b/src/firejail/join.c
@@ -431,7 +431,7 @@ void join(pid_t pid, int argc, char **argv, int index) {
 
         // set cgroup
         if (cfg.cgroup) // not available for uid 0
-                set_cgroup(cfg.cgroup);
+                set_cgroup(cfg.cgroup, getpid());
 
         // join namespaces
         if (arg_join_network) {
diff --git a/src/firejail/main.c b/src/firejail/main.c
index cc5186204..c5b3d5739 100644
--- a/src/firejail/main.c
+++ b/src/firejail/main.c
@@ -1529,15 +1529,16 @@ int main(int argc, char **argv, char **envp) {
                 else if (strncmp(argv[i], "--cgroup=", 9) == 0) {
                         if (checkcfg(CFG_CGROUP)) {
                                 if (option_cgroup) {
-                                        fprintf(stderr, "Error: only a cgroup can be defined\n");
+                                        fprintf(stderr, "Error: only one cgroup can be defined\n");
                                         exit(1);
                                 }
-
-                                option_cgroup = 1;
                                 cfg.cgroup = strdup(argv[i] + 9);
                                 if (!cfg.cgroup)
                                         errExit("strdup");
-                                set_cgroup(cfg.cgroup);
+
+                                check_cgroup_file(cfg.cgroup);
+                                set_cgroup(cfg.cgroup, getpid());
+                                option_cgroup = 1;
                         }
                         else
                                 exit_err_feature("cgroup");
diff --git a/src/firejail/profile.c b/src/firejail/profile.c
index 5390249ea..9d92b6199 100644
--- a/src/firejail/profile.c
+++ b/src/firejail/profile.c
@@ -1129,8 +1129,14 @@ int profile_check_line(char *ptr, int lineno, const char *fname) {
 
         // cgroup
         if (strncmp(ptr, "cgroup ", 7) == 0) {
-                if (checkcfg(CFG_CGROUP))
-                        set_cgroup(ptr + 7);
+                if (checkcfg(CFG_CGROUP)) {
+                        cfg.cgroup = strdup(ptr + 7);
+                        if (!cfg.cgroup)
+                                errExit("strdup");
+
+                        check_cgroup_file(cfg.cgroup);
+                        set_cgroup(cfg.cgroup, getpid());
+                }
                 else
                         warning_feature_disabled("cgroup");
                 return 0;
diff --git a/src/man/firejail.txt b/src/man/firejail.txt
index 2883ab257..154def585 100644
--- a/src/man/firejail.txt
+++ b/src/man/firejail.txt
@@ -290,8 +290,8 @@ $ firejail \-\-caps.print=3272
 Print content of file from sandbox container, see FILE TRANSFER section for more details.
 #endif
 .TP
-\fB\-\-cgroup=tasks-file
-Place the sandbox in the specified control group. tasks-file is the full path of cgroup tasks file.
+\fB\-\-cgroup=file
+Place the sandbox in the specified control group. file is the full path of a tasks or cgroup.procs file.
 .br
 
 .br