PR #1892: several fixes related to -W testing

Author: reidpr

.github/workflows/main.yml

@@ -47,6 +47,15 @@ jobs:
     steps:
       - uses: actions/checkout@v3
 
+      # This allows SSH access to the GitHub Actions VM to debug things that
+      # only happen on CI. Comment out unless needed. WARNING: tmate.io has
+      # access to unencrypted SSH traffic.
+      # See: https://github.com/marketplace/actions/debugging-with-tmate
+      #- name: set up tmate session
+      #  uses: mxschmitt/action-tmate@v3
+      #  with:
+      #    detached: true
+
       - name: early setup & validation
         run: |
           [[ -n $CH_TEST_BUILDER ]]

bin/ch-run.c

@@ -451,6 +451,12 @@ static error_t parse_opt(int key, char *arg, struct argp_state *state)
          exit(0);
 #else
          exit(1);
+#endif
+      } else if (!strcmp(arg, "overlayfs")) {
+#ifdef HAVE_OVERLAYFS
+         exit(0);
+#else
+         exit(1);
 #endif
       } else if (!strcmp(arg, "seccomp")) {
 #ifdef HAVE_SECCOMP
@@ -463,6 +469,12 @@ static error_t parse_opt(int key, char *arg, struct argp_state *state)
          exit(0);
 #else
          exit(1);
+#endif
+      } else if (!strcmp(arg, "tmpfs-xattrs")) {
+#ifdef HAVE_TMPFS_XATTRS
+         exit(0);
+#else
+         exit(1);
 #endif
       }
       else

bin/ch_core.c

@@ -41,10 +41,27 @@
 /* Timeout in seconds for waiting for join semaphore. */
 #define JOIN_TIMEOUT 30
 
-/* Maximum length of paths we're willing to deal with. (Note that
+/* Maximum length of paths we’re willing to deal with. (Note that
    system-defined PATH_MAX isn't reliable.) */
 #define PATH_CHARS 4096
 
+/* Mount point for the tmpfs used by -W. We want this to be (a) always
+   available [1], (b) short, (c) not used by anything else we care about
+   during container setup, and (d) not wildly confusing if users see it in an
+   error message. Must be a string literal because we use C’s literal
+   concatenation feature. Options considered (all of these required by FHS):
+
+      /boot       Not present if host is booted in some strange way?
+      /etc        Likely very reliable but seems risky
+      /mnt        Used for images on GitHub Actions and causes CI failures
+      /opt        Seems very omittable
+      /srv        I’ve never actually seen it used; reliable?
+      /var        Too aggressive?
+      /var/spool  Long; omittable for lightweight hosts?
+
+   [1]: https://www.pathname.com/fhs/pub/fhs-2.3.pdf */
+#define WF_MNT "/srv"
+
 
 /** Constants **/
 
@@ -306,26 +323,30 @@ void enter_udss(struct container *c)
    // https://www.kernel.org/doc/html/v5.11/filesystems/tmpfs.html
    // https://www.kernel.org/doc/html/v5.11/filesystems/overlayfs.html
    if (c->overlay_size != NULL) {
-      VERBOSE("overlaying tmpfs for --write-fake (%s)", c->overlay_size);
       char *options;
+      struct stat st;
+      VERBOSE("overlaying tmpfs for --write-fake (%s)", c->overlay_size);
       T_ (1 <= asprintf(&options, "size=%s", c->overlay_size));
-      Zf (mount(NULL, "/mnt", "tmpfs", 0, options),  // host should have /mnt
+      Zf (mount(NULL, WF_MNT, "tmpfs", 0, options),
           "cannot mount tmpfs for overlay");
       free(options);
-      Z_ (mkdir("/mnt/upper", 0700));
-      Z_ (mkdir("/mnt/work", 0700));
-      Z_ (mkdir("/mnt/merged", 0700));
-      mkdir_scratch = "/mnt/mkdir_overmount";
+      Z_ (mkdir(WF_MNT "/upper", 0700));
+      Z_ (mkdir(WF_MNT "/work", 0700));
+      Z_ (mkdir(WF_MNT "/merged", 0700));
+      mkdir_scratch = WF_MNT "/mkdir_overmount";
       Z_ (mkdir(mkdir_scratch, 0700));
-      T_ (1 <= asprintf(&options, "lowerdir=%s,upperdir=%s,workdir=%s,"
-                                  "index=on,userxattr,volatile",
-                                  c->newroot, "/mnt/upper", "/mnt/work"));
+      T_ (1 <= asprintf(&options, ("lowerdir=%s,upperdir=%s,workdir=%s,"
+                                   "index=on,userxattr,volatile"),
+                        c->newroot, WF_MNT "/upper", WF_MNT "/work"));
       // update newroot
-      c->newroot = "/mnt/merged";
+      Zf (stat(c->newroot, &st),
+          "can't stat new root; overmounted by tmpfs for -W?: %s", c->newroot);
+      c->newroot = WF_MNT "/merged";
       free(nr_parent);
       free(nr_base);
       path_split(c->newroot, &nr_parent, &nr_base);
-      Zf (mount(NULL, c->newroot, "overlay", 0, options), "can't overlay");
+      Zf (mount(NULL, c->newroot, "overlay", 0, options),
+          "can't overlay: %s, %s", c->newroot, options);
       VERBOSE("newroot updated: %s", c->newroot);
       free(options);
    }

configure.ac

@@ -787,6 +787,10 @@ AC_SUBST([CH_RUN_LIBS])
 AC_SUBST([PYTHON_SHEBANG])
 AC_SUBST([SPHINX])
 
+AS_IF([test $have_overlayfs = yes],
+      [AC_DEFINE([HAVE_OVERLAYFS], [1], [unprivileged overlayfs])])
+AS_IF([test $have_tmpfs_xattrs = yes],
+      [AC_DEFINE([HAVE_TMPFS_XATTRS], [1], [tmpfs user xattrs])])
 AS_IF([test $have_fnm_extmatch = yes],
       [AC_DEFINE([HAVE_FNM_EXTMATCH], [1], [extended globs supported])])
 AS_IF([test $have_seccomp = yes],

doc/ch-run.rst

@@ -60,9 +60,16 @@ mounting SquashFS images with FUSE.
     Don’t expand variables when using :code:`--set-env`.
 
   :code:`--feature=FEAT`
-    If feature :code:`FEAT` is enabled, exit with success. Valid values of
-    :code:`FEAT` are :code:`extglob` for extended globs, :code:`seccomp` for
-    :code:`seccomp(2)`, and :code:`squash` for squashfs archives.
+    If feature :code:`FEAT` is enabled, exit successfully (zero); otherwise,
+    exit unsuccessfully (non-zero). Note this just communicates the results of
+    :code:`configure` rather than testing the feature. Valid values of
+    :code:`FEAT` are:
+
+       * :code:`extglob`: extended globs in :code:`--unset-env`
+       * :code:`seccomp`: :code:`--seccomp` available
+       * :code:`squash`: internal SquashFUSE image mounts
+       * :code:`overlayfs`: unprivileged overlayfs support
+       * :code:`tmpfs-xattrs`: :code:`user` xattrs on tmpfs
 
   :code:`-g`, :code:`--gid=GID`
     Run as group :code:`GID` within container.

test/run/ch-run_misc.bats

@@ -12,7 +12,7 @@ setup () {
 
 
 demand-overlayfs () {
-    ch-run -W "$ch_timg" -- true || skip 'no unpriv overlayfs'
+    ch-run --feature=overlayfs || skip 'no unpriv overlayfs'
 }
 
 
@@ -291,10 +291,11 @@ EOF
     [[ $status -eq 0 ]]
 
     # --home
-    run ch-run --home "$img" -- ls -lah /home
+    run ch-run --home "$img" -- ls -lAh /home
     echo "$output"
     [[ $status -eq 0 ]]
-    [[ $(echo "$output" | wc -l) -eq 3 ]]
+    [[ $(echo "$output" | wc -l) -eq 5 ]]  # 4 files plus “total” line
+    [[ $output = *.orig* ]]
     [[ $output = *directory-in-home* ]]
     [[ $output = *file-in-home* ]]
     [[ $output = *"$USER"* ]]