bind_mount: Return an error code, and provide a way to display it

This gives us better diagnostic messages on failure, particularly for
BIND_MOUNT_ERROR_FIND_DEST_MOUNT where we previously said "Invalid
argument".

Signed-off-by: Simon McVittie <smcv@collabora.com>

3 files changed, 107 insertions, 17 deletions

diff --git a/bind-mount.c b/bind-mount.c
index e692762..9bbd61b 100644
--- a/bind-mount.c
+++ b/bind-mount.c
@@ -373,7 +373,7 @@ parse_mountinfo (int  proc_fd,
   return steal_pointer (&mount_tab);
 }
 
-int
+bind_mount_result
 bind_mount (int           proc_fd,
             const char   *src,
             const char   *dest,
@@ -394,18 +394,18 @@ bind_mount (int           proc_fd,
   if (src)
     {
       if (mount (src, dest, NULL, MS_SILENT | MS_BIND | (recursive ? MS_REC : 0), NULL) != 0)
-        return 1;
+        return BIND_MOUNT_ERROR_MOUNT;
     }
 
   /* The mount operation will resolve any symlinks in the destination
      path, so to find it in the mount table we need to do that too. */
   resolved_dest = realpath (dest, NULL);
   if (resolved_dest == NULL)
-    return 2;
+    return BIND_MOUNT_ERROR_REALPATH_DEST;
 
   dest_fd = open (resolved_dest, O_PATH | O_CLOEXEC);
   if (dest_fd < 0)
-    return 2;
+    return BIND_MOUNT_ERROR_REOPEN_DEST;
 
   /* If we are in a case-insensitive filesystem, mountinfo might contain a
    * different case combination of the path we requested to mount.
@@ -421,13 +421,13 @@ bind_mount (int           proc_fd,
   oldroot_dest_proc = get_oldroot_path (dest_proc);
   kernel_case_combination = readlink_malloc (oldroot_dest_proc);
   if (kernel_case_combination == NULL)
-    die_with_error ("Can't read the link in %s", oldroot_dest_proc);
+    return BIND_MOUNT_ERROR_READLINK_DEST_PROC_FD;
 
   mount_tab = parse_mountinfo (proc_fd, kernel_case_combination);
   if (mount_tab[0].mountpoint == NULL)
     {
       errno = EINVAL;
-      return 2; /* No mountpoint at dest */
+      return BIND_MOUNT_ERROR_FIND_DEST_MOUNT;
     }
 
   assert (path_equal (mount_tab[0].mountpoint, kernel_case_combination));
@@ -436,7 +436,7 @@ bind_mount (int           proc_fd,
   if (new_flags != current_flags &&
       mount ("none", resolved_dest,
              NULL, MS_SILENT | MS_BIND | MS_REMOUNT | new_flags, NULL) != 0)
-    return 3;
+    return BIND_MOUNT_ERROR_REMOUNT_DEST;
 
   /* We need to work around the fact that a bind mount does not apply the flags, so we need to manually
    * apply the flags to all submounts in the recursive case.
@@ -455,10 +455,71 @@ bind_mount (int           proc_fd,
               /* If we can't read the mountpoint we can't remount it, but that should
                  be safe to ignore because its not something the user can access. */
               if (errno != EACCES)
-                return 5;
+                return BIND_MOUNT_ERROR_REMOUNT_SUBMOUNT;
             }
         }
     }
 
-  return 0;
+  return BIND_MOUNT_SUCCESS;
+}
+
+void
+die_with_bind_result (bind_mount_result res,
+                      int               saved_errno,
+                      const char       *format,
+                      ...)
+{
+  va_list args;
+  bool want_errno = TRUE;
+
+  fprintf (stderr, "bwrap: ");
+
+  va_start (args, format);
+  vfprintf (stderr, format, args);
+  va_end (args);
+
+  fprintf (stderr, ": ");
+
+  switch (res)
+    {
+      case BIND_MOUNT_ERROR_MOUNT:
+        fprintf (stderr, "Unable to mount source on destination");
+        break;
+
+      case BIND_MOUNT_ERROR_REALPATH_DEST:
+        fprintf (stderr, "realpath(destination)");
+        break;
+
+      case BIND_MOUNT_ERROR_REOPEN_DEST:
+        fprintf (stderr, "open(destination, O_PATH)");
+        break;
+
+      case BIND_MOUNT_ERROR_READLINK_DEST_PROC_FD:
+        fprintf (stderr, "readlink(/proc/self/fd/<destination>)");
+        break;
+
+      case BIND_MOUNT_ERROR_FIND_DEST_MOUNT:
+        fprintf (stderr, "Unable to find destination in mount table");
+        want_errno = FALSE;
+        break;
+
+      case BIND_MOUNT_ERROR_REMOUNT_DEST:
+        fprintf (stderr, "Unable to remount destination with correct flags");
+        break;
+
+      case BIND_MOUNT_ERROR_REMOUNT_SUBMOUNT:
+        fprintf (stderr, "Unable to remount recursively with correct flags");
+        break;
+
+      case BIND_MOUNT_SUCCESS:
+      default:
+        fprintf (stderr, "(unknown error %d)", res);
+        break;
+    }
+
+  if (want_errno)
+    fprintf (stderr, ": %s", strerror (saved_errno));
+
+  fprintf (stderr, "\n");
+  exit (1);
 }
diff --git a/bind-mount.h b/bind-mount.h
index c763763..6b23ae3 100644
--- a/bind-mount.h
+++ b/bind-mount.h
@@ -24,7 +24,26 @@ typedef enum {
   BIND_RECURSIVE = (1 << 3),
 } bind_option_t;
 
-int bind_mount (int           proc_fd,
-                const char   *src,
-                const char   *dest,
-                bind_option_t options);
+typedef enum
+{
+  BIND_MOUNT_SUCCESS = 0,
+  BIND_MOUNT_ERROR_MOUNT,
+  BIND_MOUNT_ERROR_REALPATH_DEST,
+  BIND_MOUNT_ERROR_REOPEN_DEST,
+  BIND_MOUNT_ERROR_READLINK_DEST_PROC_FD,
+  BIND_MOUNT_ERROR_FIND_DEST_MOUNT,
+  BIND_MOUNT_ERROR_REMOUNT_DEST,
+  BIND_MOUNT_ERROR_REMOUNT_SUBMOUNT,
+} bind_mount_result;
+
+bind_mount_result bind_mount (int           proc_fd,
+                              const char   *src,
+                              const char   *dest,
+                              bind_option_t options);
+
+void die_with_bind_result (bind_mount_result res,
+                           int               saved_errno,
+                           const char       *format,
+                           ...)
+  __attribute__((__noreturn__))
+  __attribute__((format (printf, 3, 4)));
diff --git a/bubblewrap.c b/bubblewrap.c
index 8532152..6225330 100644
--- a/bubblewrap.c
+++ b/bubblewrap.c
@@ -942,6 +942,8 @@ privileged_op (int         privileged_op_socket,
                const char *arg1,
                const char *arg2)
 {
+  bind_mount_result bind_result;
+
   if (privileged_op_socket != -1)
     {
       uint32_t buffer[2048];  /* 8k, but is int32 to guarantee nice alignment */
@@ -1006,15 +1008,23 @@ privileged_op (int         privileged_op_socket,
       break;
 
     case PRIV_SEP_OP_REMOUNT_RO_NO_RECURSIVE:
-      if (bind_mount (proc_fd, NULL, arg2, BIND_READONLY) != 0)
-        die_with_error ("Can't remount readonly on %s", arg2);
+      bind_result = bind_mount (proc_fd, NULL, arg2, BIND_READONLY);
+
+      if (bind_result != BIND_MOUNT_SUCCESS)
+        die_with_bind_result (bind_result, errno,
+                              "Can't remount readonly on %s", arg2);
+
       break;
 
     case PRIV_SEP_OP_BIND_MOUNT:
       /* We always bind directories recursively, otherwise this would let us
          access files that are otherwise covered on the host */
-      if (bind_mount (proc_fd, arg1, arg2, BIND_RECURSIVE | flags) != 0)
-        die_with_error ("Can't bind mount %s on %s", arg1, arg2);
+      bind_result = bind_mount (proc_fd, arg1, arg2, BIND_RECURSIVE | flags);
+
+      if (bind_result != BIND_MOUNT_SUCCESS)
+        die_with_bind_result (bind_result, errno,
+                              "Can't bind mount %s on %s", arg1, arg2);
+
       break;
 
     case PRIV_SEP_OP_PROC_MOUNT: