minor sandbox lock improvements

3 files changed, 19 insertions, 12 deletions

diff --git a/src/firejail/firejail.h b/src/firejail/firejail.h
index c5004ef8a..aec320c1f 100644
--- a/src/firejail/firejail.h
+++ b/src/firejail/firejail.h
@@ -907,7 +907,8 @@ void delete_bandwidth_run_file(pid_t pid);
 void set_name_run_file(pid_t pid);
 void set_x11_run_file(pid_t pid, int display);
 void set_profile_run_file(pid_t pid, const char *fname);
-int set_sandbox_run_file(pid_t pid, pid_t child);
+void set_sandbox_run_file(pid_t pid, pid_t child);
+void release_sandbox_run_file_lock(void);
 
 // dbus.c
 int dbus_check_name(const char *name);
diff --git a/src/firejail/main.c b/src/firejail/main.c
index 6466be7d4..539760535 100644
--- a/src/firejail/main.c
+++ b/src/firejail/main.c
@@ -190,6 +190,8 @@ static void myexit(int rv) {
 }
 
 static void my_handler(int s) {
+        release_sandbox_run_file_lock();
+
         fmessage("\nParent received signal %d, shutting down the child process...\n", s);
         logsignal(s);
 
@@ -961,7 +963,6 @@ int main(int argc, char **argv, char **envp) {
         int prog_index = -1;                      // index in argv where the program command starts
         int lockfd_network = -1;
         int lockfd_directory = -1;
-        int lockfd_sandboxfile = -1;
         int custom_profile = 0; // custom profile loaded
         int arg_caps_cmdline = 0;       // caps requested on command line (used to break out of --chroot)
         int arg_netlock = 0;
@@ -2997,7 +2998,7 @@ int main(int argc, char **argv, char **envp) {
         EUID_USER();
 
         // sandbox pidfile
-        lockfd_sandboxfile = set_sandbox_run_file(getpid(), child);
+        set_sandbox_run_file(getpid(), child);
 
         if (!arg_command && !arg_quiet) {
                 fmessage("Parent pid %u, child pid %u\n", sandbox_pid, child);
@@ -3222,8 +3223,7 @@ int main(int argc, char **argv, char **envp) {
         // end of signal-safe code
         //*****************************
 
-        // release lock
-        close(lockfd_sandboxfile);
+        release_sandbox_run_file_lock();
 
         if (WIFEXITED(status)){
                 myexit(WEXITSTATUS(status));
diff --git a/src/firejail/run_files.c b/src/firejail/run_files.c
index 8b8bbae12..6724e2cd8 100644
--- a/src/firejail/run_files.c
+++ b/src/firejail/run_files.c
@@ -164,7 +164,8 @@ void set_profile_run_file(pid_t pid, const char *fname) {
         free(runfile);
 }
 
-int set_sandbox_run_file(pid_t pid, pid_t child) {
+static int sandbox_run_file_fd = -1;
+void set_sandbox_run_file(pid_t pid, pid_t child) {
         char *runfile;
         if (asprintf(&runfile, "%s/%d", RUN_FIREJAIL_SANDBOX_DIR, pid) == -1)
                 errExit("asprintf");
@@ -172,8 +173,8 @@ int set_sandbox_run_file(pid_t pid, pid_t child) {
         EUID_ROOT();
         // the file is deleted first
         // this file should be opened with O_CLOEXEC set
-        int fd = open(runfile, O_CREAT | O_WRONLY | O_TRUNC | O_CLOEXEC, S_IRUSR | S_IWUSR);
-        if (fd < 0) {
+        sandbox_run_file_fd = open(runfile, O_CREAT | O_WRONLY | O_TRUNC | O_CLOEXEC, S_IRUSR | S_IWUSR);
+        if (sandbox_run_file_fd < 0) {
                 fprintf(stderr, "Error: cannot create %s\n", runfile);
                 exit(1);
         }
@@ -185,7 +186,7 @@ int set_sandbox_run_file(pid_t pid, pid_t child) {
         size_t len = strlen(buf);
         size_t done = 0;
         while (done != len) {
-                ssize_t rv = write(fd, buf + done, len - done);
+                ssize_t rv = write(sandbox_run_file_fd, buf + done, len - done);
                 if (rv < 0)
                         errExit("write");
                 done += rv;
@@ -193,14 +194,19 @@ int set_sandbox_run_file(pid_t pid, pid_t child) {
 
         // set exclusive lock on the file
         // the lock is never inherited, and is released if this process dies ungracefully
-        struct flock sandboxlock = {
+        struct flock sandbox_lock = {
                 .l_type = F_WRLCK,
                 .l_whence = SEEK_SET,
                 .l_start = 0,
                 .l_len = 0,
         };
-        if (fcntl(fd, F_SETLK, &sandboxlock) < 0)
+        if (fcntl(sandbox_run_file_fd, F_SETLK, &sandbox_lock) < 0)
                 errExit("fcntl");
+}
+
+void release_sandbox_run_file_lock(void) {
+        assert(sandbox_run_file_fd > -1);
 
-        return fd;
+        close(sandbox_run_file_fd);
+        sandbox_run_file_fd = -1;
 }