Use mount_setattr() in bind_mount()

Author: xxyzz

.github/workflows/check.yml

@@ -3,18 +3,18 @@ name: CI checks
 on:
   push:
     branches:
-    - main
+      - main
   pull_request:
     branches:
-    - main
+      - main
 
 jobs:
   meson:
     name: Build with Meson and gcc, and test
     runs-on: ubuntu-latest
     steps:
     - name: Check out
-      uses: actions/checkout@v4
+      uses: actions/checkout@v6
     - name: Install build-dependencies
       run: sudo ./ci/builddeps.sh
     - name: Enable user namespaces
@@ -71,7 +71,7 @@ jobs:
         test ! -e DESTDIR-as-subproject/usr/local/libexec/bwrap
         tests/use-as-subproject/assert-correct-rpath.py DESTDIR-as-subproject/usr/local/libexec/not-flatpak-bwrap
     - name: Upload test logs
-      uses: actions/upload-artifact@v4
+      uses: actions/upload-artifact@v7
       if: failure() || cancelled()
       with:
         name: test logs
@@ -87,11 +87,11 @@ jobs:
           - cpp
     steps:
     - name: Initialize CodeQL
-      uses: github/codeql-action/init@v2
+      uses: github/codeql-action/init@v4
       with:
         languages: ${{ matrix.language }}
     - name: Check out
-      uses: actions/checkout@v4
+      uses: actions/checkout@v6
     - name: Install build-dependencies
       run: sudo ./ci/builddeps.sh --clang
     - run: meson build -Dselinux=enabled
@@ -102,4 +102,4 @@ jobs:
           -Werror=unused-variable
     - run: meson compile -C build
     - name: CodeQL analysis
-      uses: github/codeql-action/analyze@v2
+      uses: github/codeql-action/analyze@v4

bind-mount.c

@@ -414,72 +414,91 @@ bind_mount (int           proc_fd,
       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.
-   * This is due to the fact that the kernel, as of the beginning of 2021,
-   * populates mountinfo with whatever case combination first appeared in the
-   * dcache; kernel developers plan to change this in future so that it
-   * reflects the on-disk encoding instead.
-   * To avoid throwing an error when this happens, we use readlink() result
-   * instead of the provided @root_mount, so that we can compare the mountinfo
-   * entries with the same case combination that the kernel is expected to
-   * use. */
-  dest_proc = xasprintf ("/proc/self/fd/%d", dest_fd);
-  oldroot_dest_proc = get_oldroot_path (dest_proc);
-  kernel_case_combination = readlink_malloc (oldroot_dest_proc);
-  if (kernel_case_combination == NULL)
+  if (has_mount_setattr())
     {
-      if (failing_path != NULL)
-        *failing_path = steal_pointer (&resolved_dest);
-
-      return BIND_MOUNT_ERROR_READLINK_DEST_PROC_FD;
+      struct mount_attr attr = {
+        .attr_clr = 0,
+        .attr_set = MOUNT_ATTR_NOSUID | (devices ? 0 : MOUNT_ATTR_NODEV) |
+                    (readonly ? MOUNT_ATTR_RDONLY : 0),
+      };
+      if (mount_setattr(dest_fd, "",
+                        AT_EMPTY_PATH | (recursive ? AT_RECURSIVE : 0), &attr,
+                        sizeof(attr)) == -1)
+        {
+          if (failing_path != NULL)
+            *failing_path = steal_pointer(&resolved_dest);
+          return BIND_MOUNT_ERROR_MOUNT_SETATTR;
+        }
     }
-
-  mount_tab = parse_mountinfo (proc_fd, kernel_case_combination);
-  if (mount_tab[0].mountpoint == NULL)
+  else
     {
-      if (failing_path != NULL)
-        *failing_path = steal_pointer (&kernel_case_combination);
+      /* If we are in a case-insensitive filesystem, mountinfo might contain a
+       * different case combination of the path we requested to mount.
+       * This is due to the fact that the kernel, as of the beginning of 2021,
+       * populates mountinfo with whatever case combination first appeared in the
+       * dcache; kernel developers plan to change this in future so that it
+       * reflects the on-disk encoding instead.
+       * To avoid throwing an error when this happens, we use readlink() result
+       * instead of the provided @root_mount, so that we can compare the mountinfo
+       * entries with the same case combination that the kernel is expected to
+       * use. */
+      dest_proc = xasprintf ("/proc/self/fd/%d", dest_fd);
+      oldroot_dest_proc = get_oldroot_path (dest_proc);
+      kernel_case_combination = readlink_malloc (oldroot_dest_proc);
+      if (kernel_case_combination == NULL)
+        {
+          if (failing_path != NULL)
+            *failing_path = steal_pointer (&resolved_dest);
 
-      errno = EINVAL;
-      return BIND_MOUNT_ERROR_FIND_DEST_MOUNT;
-    }
+          return BIND_MOUNT_ERROR_READLINK_DEST_PROC_FD;
+        }
 
-  assert (path_equal (mount_tab[0].mountpoint, kernel_case_combination));
-  current_flags = mount_tab[0].options;
-  new_flags = current_flags | (devices ? 0 : MS_NODEV) | MS_NOSUID | (readonly ? MS_RDONLY : 0);
-  if (new_flags != current_flags &&
-      mount ("none", resolved_dest,
-             NULL, MS_SILENT | MS_BIND | MS_REMOUNT | new_flags, NULL) != 0)
-    {
-      if (failing_path != NULL)
-        *failing_path = steal_pointer (&resolved_dest);
+      mount_tab = parse_mountinfo (proc_fd, kernel_case_combination);
+      if (mount_tab[0].mountpoint == NULL)
+        {
+          if (failing_path != NULL)
+            *failing_path = steal_pointer (&kernel_case_combination);
 
-      return BIND_MOUNT_ERROR_REMOUNT_DEST;
-    }
+          errno = EINVAL;
+          return BIND_MOUNT_ERROR_FIND_DEST_MOUNT;
+        }
 
-  /* 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.
-   * Note: This does not apply the flags to mounts which are later propagated into this namespace.
-   */
-  if (recursive)
-    {
-      for (i = 1; mount_tab[i].mountpoint != NULL; i++)
+      assert (path_equal (mount_tab[0].mountpoint, kernel_case_combination));
+      current_flags = mount_tab[0].options;
+      new_flags = current_flags | (devices ? 0 : MS_NODEV) | MS_NOSUID | (readonly ? MS_RDONLY : 0);
+      if (new_flags != current_flags &&
+          mount ("none", resolved_dest,
+                 NULL, MS_SILENT | MS_BIND | MS_REMOUNT | new_flags, NULL) != 0)
+        {
+          if (failing_path != NULL)
+            *failing_path = steal_pointer (&resolved_dest);
+
+          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.
+       * Note: This does not apply the flags to mounts which are later propagated into this namespace.
+       */
+      if (recursive)
         {
-          current_flags = mount_tab[i].options;
-          new_flags = current_flags | (devices ? 0 : MS_NODEV) | MS_NOSUID | (readonly ? MS_RDONLY : 0);
-          if (new_flags != current_flags &&
-              mount ("none", mount_tab[i].mountpoint,
-                     NULL, MS_SILENT | MS_BIND | MS_REMOUNT | new_flags, NULL) != 0)
+          for (i = 1; mount_tab[i].mountpoint != NULL; i++)
             {
-              /* 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)
+              current_flags = mount_tab[i].options;
+              new_flags = current_flags | (devices ? 0 : MS_NODEV) | MS_NOSUID | (readonly ? MS_RDONLY : 0);
+              if (new_flags != current_flags &&
+                  mount ("none", mount_tab[i].mountpoint,
+                         NULL, MS_SILENT | MS_BIND | MS_REMOUNT | new_flags, NULL) != 0)
                 {
-                  if (failing_path != NULL)
-                    *failing_path = xstrdup (mount_tab[i].mountpoint);
-
-                  return BIND_MOUNT_ERROR_REMOUNT_SUBMOUNT;
+                  /* 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)
+                    {
+                      if (failing_path != NULL)
+                        *failing_path = xstrdup (mount_tab[i].mountpoint);
+
+                      return BIND_MOUNT_ERROR_REMOUNT_SUBMOUNT;
+                    }
                 }
             }
         }
@@ -534,6 +553,10 @@ bind_mount_result_to_string (bind_mount_result res,
                             failing_path);
         break;
 
+      case BIND_MOUNT_ERROR_MOUNT_SETATTR:
+        string = xasprintf("mount_setattr() failed at \"%s\"", failing_path);
+        break;
+
       case BIND_MOUNT_SUCCESS:
         string = xstrdup ("Success");
         break;
@@ -587,6 +610,7 @@ die_with_bind_result (bind_mount_result res,
           case BIND_MOUNT_ERROR_REOPEN_DEST:
           case BIND_MOUNT_ERROR_READLINK_DEST_PROC_FD:
           case BIND_MOUNT_ERROR_FIND_DEST_MOUNT:
+          case BIND_MOUNT_ERROR_MOUNT_SETATTR:
           case BIND_MOUNT_SUCCESS:
           default:
             fprintf (stderr, ": %s", strerror (saved_errno));

bind-mount.h

@@ -37,6 +37,7 @@ typedef enum
   BIND_MOUNT_ERROR_FIND_DEST_MOUNT,
   BIND_MOUNT_ERROR_REMOUNT_DEST,
   BIND_MOUNT_ERROR_REMOUNT_SUBMOUNT,
+  BIND_MOUNT_ERROR_MOUNT_SETATTR,
 } bind_mount_result;
 
 bind_mount_result bind_mount (int           proc_fd,

utils.c

@@ -1083,3 +1083,16 @@ strappend_escape_for_mount_options (StringBuilder *dest, const char *src)
       src++;
     }
 }
+
+bool
+has_mount_setattr (void)
+{
+#ifdef __NR_mount_setattr
+  long ret = syscall(__NR_mount_setattr, -1, "", 0, NULL, 0);
+  if (ret == -1 && errno == ENOSYS)
+      return false;
+  return true;
+#else
+  return false;
+#endif
+}

utils.h

@@ -215,3 +215,5 @@ void strappendf (StringBuilder *dest,
                  ...);
 void strappend_escape_for_mount_options (StringBuilder *dest,
                                          const char    *src);
+
+bool has_mount_setattr(void);