Merge pull request #6307 from spiiroin/serialize_remounts

modif: populate /run/firejail while holding flock

src/firejail/chroot.c

@@ -273,7 +273,10 @@ void fs_chroot(const char *rootdir) {
  errExit("mounting /proc");
 
  // create all other /run/firejail files and directories
- preproc_build_firejail_dir();
+ preproc_build_firejail_dir_unlocked();
+ preproc_lock_firejail_dir();
+ preproc_build_firejail_dir_locked();
+ preproc_unlock_firejail_dir();
 
  // update /var directory in order to support multiple sandboxes running on the same root directory
  // if (!arg_private_dev)

src/firejail/firejail.h

@@ -282,6 +282,8 @@ static inline int any_dhcp(void) {
  return any_ip_dhcp() || any_ip6_dhcp();
 }
 
+extern int lockfd_directory;
+extern int lockfd_network;
 extern int arg_private; // mount private /home
 extern int arg_private_cache; // private home/.cache
 extern int arg_debug; // print debug messages
@@ -429,7 +431,12 @@ int net_get_mac(const char *ifname, unsigned char mac[6]);
 void net_config_interface(const char *dev, uint32_t ip, uint32_t mask, int mtu);
 
 // preproc.c
-void preproc_build_firejail_dir(void);
+void preproc_lock_firejail_dir(void);
+void preproc_unlock_firejail_dir(void);
+void preproc_lock_firejail_network_dir(void);
+void preproc_unlock_firejail_network_dir(void);
+void preproc_build_firejail_dir_unlocked(void);
+void preproc_build_firejail_dir_locked(void);
 void preproc_mount_mnt_dir(void);
 void preproc_clean_run(void);
 

src/firejail/main.c

@@ -63,6 +63,8 @@ gid_t firejail_gid = 0;
 static char child_stack[STACK_SIZE] __attribute__((aligned(STACK_ALIGNMENT))); // space for child's stack
 
 Config cfg; // configuration
+int lockfd_directory = -1;
+int lockfd_network = -1;
 int arg_private = 0; // mount private /home and /tmp directoryu
 int arg_private_cache = 0; // mount private home/.cache
 int arg_debug = 0; // print debug messages
@@ -1056,8 +1058,6 @@ static int check_postexec(const char *list) {
 int main(int argc, char **argv, char **envp) {
  int i;
  int prog_index = -1; // index in argv where the program command starts
- int lockfd_network = -1;
- int lockfd_directory = -1;
  int custom_profile = 0; // custom profile loaded
  int arg_caps_cmdline = 0; // caps requested on command line (used to break out of --chroot)
  char **ptr;
@@ -1166,19 +1166,13 @@ int main(int argc, char **argv, char **envp) {
 #endif
 
  // build /run/firejail directory structure
- preproc_build_firejail_dir();
+ preproc_build_firejail_dir_unlocked();
+ preproc_lock_firejail_dir();
+ preproc_build_firejail_dir_locked();
  const char *container_name = env_get("container");
- if (!container_name || strcmp(container_name, "firejail")) {
- lockfd_directory = open(RUN_DIRECTORY_LOCK_FILE, O_WRONLY | O_CREAT | O_CLOEXEC, S_IRUSR | S_IWUSR);
- if (lockfd_directory != -1) {
- int rv = fchown(lockfd_directory, 0, 0);
- (void) rv;
- flock(lockfd_directory, LOCK_EX);
- }
+ if (!container_name || strcmp(container_name, "firejail"))
  preproc_clean_run();
- flock(lockfd_directory, LOCK_UN);
- close(lockfd_directory);
- }
+ preproc_unlock_firejail_dir();
 
  delete_run_files(getpid());
  atexit(clear_atexit);
@@ -2990,12 +2984,7 @@ int main(int argc, char **argv, char **envp) {
  // check and assign an IP address - for macvlan it will be done again in the sandbox!
  if (any_bridge_configured()) {
  EUID_ROOT();
- lockfd_network = open(RUN_NETWORK_LOCK_FILE, O_WRONLY | O_CREAT | O_CLOEXEC, S_IRUSR | S_IWUSR);
- if (lockfd_network != -1) {
- int rv = fchown(lockfd_network, 0, 0);
- (void) rv;
- flock(lockfd_network, LOCK_EX);
- }
+ preproc_lock_firejail_network_dir();
 
  if (cfg.bridge0.configured && cfg.bridge0.arg_ip_none == 0)
  check_network(&cfg.bridge0);
@@ -3024,21 +3013,13 @@ int main(int argc, char **argv, char **envp) {
 
  // set name and x11 run files
  EUID_ROOT();
- lockfd_directory = open(RUN_DIRECTORY_LOCK_FILE, O_WRONLY | O_CREAT | O_CLOEXEC, S_IRUSR | S_IWUSR);
- if (lockfd_directory != -1) {
- int rv = fchown(lockfd_directory, 0, 0);
- (void) rv;
- flock(lockfd_directory, LOCK_EX);
- }
+ preproc_lock_firejail_dir();
  if (cfg.name)
  set_name_run_file(sandbox_pid);
  int display = x11_display();
  if (display > 0)
  set_x11_run_file(sandbox_pid, display);
- if (lockfd_directory != -1) {
- flock(lockfd_directory, LOCK_UN);
- close(lockfd_directory);
- }
+ preproc_unlock_firejail_dir();
  EUID_USER();
 
 #ifdef HAVE_DBUSPROXY
@@ -3276,10 +3257,7 @@ int main(int argc, char **argv, char **envp) {
  close(parent_to_child_fds[1]);
 
  EUID_ROOT();
- if (lockfd_network != -1) {
- flock(lockfd_network, LOCK_UN);
- close(lockfd_network);
- }
+ preproc_unlock_firejail_network_dir();
  EUID_USER();
 
  // lock netfilter firewall

src/firejail/preproc.c

@@ -18,15 +18,101 @@
  * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
 */
 #include "firejail.h"
+#include <sys/file.h>
 #include <sys/mount.h>
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <dirent.h>
+#include <fcntl.h>
 
 static int tmpfs_mounted = 0;
 
+static void preproc_lock_file(const char *path, int *lockfd_ptr) {
+ assert(path != NULL);
+ assert(lockfd_ptr != NULL);
+
+ long pid = (long)getpid();
+ if (arg_debug)
+ fprintf(stderr, "pid=%ld: locking %s ...\n", pid, path);
+
+ if (*lockfd_ptr != -1) {
+ if (arg_debug)
+ fprintf(stderr, "pid=%ld: already locked %s\n", pid, path);
+ return;
+ }
+
+ int lockfd = open(path, O_WRONLY | O_CREAT | O_CLOEXEC, S_IRUSR | S_IWUSR);
+ if (lockfd == -1) {
+ fprintf(stderr, "Error: cannot create a lockfile at %s\n", path);
+ errExit("open");
+ }
+
+ if (fchown(lockfd, 0, 0) == -1) {
+ fprintf(stderr, "Error: cannot chown root:root %s\n", path);
+ errExit("fchown");
+ }
+
+ if (flock(lockfd, LOCK_EX) == -1) {
+ fprintf(stderr, "Error: cannot lock %s\n", path);
+ errExit("flock");
+ }
+
+ *lockfd_ptr = lockfd;
+ if (arg_debug)
+ fprintf(stderr, "pid=%ld: locked %s\n", pid, path);
+}
+
+static void preproc_unlock_file(const char *path, int *lockfd_ptr) {
+ assert(path != NULL);
+ assert(lockfd_ptr != NULL);
+
+ long pid = (long)getpid();
+ if (arg_debug)
+ fprintf(stderr, "pid=%ld: unlocking %s ...\n", pid, path);
+
+ int lockfd = *lockfd_ptr;
+ if (lockfd == -1) {
+ if (arg_debug)
+ fprintf(stderr, "pid=%ld: already unlocked %s\n", pid, path);
+ return;
+ }
+
+ if (flock(lockfd, LOCK_UN) == -1) {
+ fprintf(stderr, "Error: cannot unlock %s\n", path);
+ errExit("flock");
+ }
+
+ if (close(lockfd) == -1) {
+ fprintf(stderr, "Error: cannot close %s\n", path);
+ errExit("close");
+ }
+
+ *lockfd_ptr = -1;
+ if (arg_debug)
+ fprintf(stderr, "pid=%ld: unlocked %s\n", pid, path);
+}
+
+void preproc_lock_firejail_dir(void) {
+ preproc_lock_file(RUN_DIRECTORY_LOCK_FILE, &lockfd_directory);
+}
+
+void preproc_unlock_firejail_dir(void) {
+ preproc_unlock_file(RUN_DIRECTORY_LOCK_FILE, &lockfd_directory);
+}
+
+void preproc_lock_firejail_network_dir(void) {
+ preproc_lock_file(RUN_NETWORK_LOCK_FILE, &lockfd_network);
+}
+
+void preproc_unlock_firejail_network_dir(void) {
+ preproc_unlock_file(RUN_NETWORK_LOCK_FILE, &lockfd_network);
+}
+
 // build /run/firejail directory
-void preproc_build_firejail_dir(void) {
+//
+// Note: This creates the base directory of the rundir lockfile;
+// it should be called before preproc_lock_firejail_dir().
+void preproc_build_firejail_dir_unlocked(void) {
  struct stat s;
 
  // CentOS 6 doesn't have /run directory
@@ -35,6 +121,14 @@ void preproc_build_firejail_dir(void) {
  }
 
  create_empty_dir_as_root(RUN_FIREJAIL_DIR, 0755);
+}
+
+// build directory hierarchy under /run/firejail
+//
+// Note: Remounts have timing hazards. This function should
+// only be called after acquiring the directory lock via
+// preproc_lock_firejail_dir().
+void preproc_build_firejail_dir_locked(void) {
  create_empty_dir_as_root(RUN_FIREJAIL_NETWORK_DIR, 0755);
  create_empty_dir_as_root(RUN_FIREJAIL_BANDWIDTH_DIR, 0755);
  create_empty_dir_as_root(RUN_FIREJAIL_NAME_DIR, 0755);