Re: [PATCH v2] linux: return UNSUPPORTED in tst-mount if !support_can_chroot

[Michael Hudson-Doyle <michael.hudson@canonical.com>]

On Sat, 16 Jul 2022, 03:36 Carlos O'Donell via Libc-alpha, <
libc-alpha@sourceware.org> wrote:

> On 7/15/22 03:07, Florian Weimer via Libc-alpha wrote:
> > * Michael Hudson-Doyle via Libc-alpha:
> >
> >> Otherwise the test fails if run in a chroot by a non-root user:
> >>
> >> warning: could not become root outside namespace (Operation not
> permitted)
> >> ../sysdeps/unix/sysv/linux/tst-mount.c:36: numeric comparison failure
> >>    left: 1 (0x1); from: errno
> >>   right: 19 (0x13); from: ENODEV
> >> error: ../sysdeps/unix/sysv/linux/tst-mount.c:39: not true: fd != -1
> >> error: ../sysdeps/unix/sysv/linux/tst-mount.c:46: not true: r != -1
> >> error: ../sysdeps/unix/sysv/linux/tst-mount.c:48: not true: r != -1
> >> ../sysdeps/unix/sysv/linux/tst-mount.c:52: numeric comparison failure
> >>    left: 1 (0x1); from: errno
> >>   right: 9 (0x9); from: EBADF
> >> error: ../sysdeps/unix/sysv/linux/tst-mount.c:55: not true: mfd != -1
> >> ../sysdeps/unix/sysv/linux/tst-mount.c:58: numeric comparison failure
> >>    left: 1 (0x1); from: errno
> >>   right: 2 (0x2); from: ENOENT
> >> error: ../sysdeps/unix/sysv/linux/tst-mount.c:61: not true: r != -1
> >> ../sysdeps/unix/sysv/linux/tst-mount.c:65: numeric comparison failure
> >>    left: 1 (0x1); from: errno
> >>   right: 2 (0x2); from: ENOENT
> >> error: ../sysdeps/unix/sysv/linux/tst-mount.c:68: not true: pfd != -1
> >> error: ../sysdeps/unix/sysv/linux/tst-mount.c:75: not true: fd_tree !=
> -1
> >> ../sysdeps/unix/sysv/linux/tst-mount.c:88: numeric comparison failure
> >>    left: 1 (0x1); from: errno
> >>   right: 38 (0x26); from: ENOSYS
> >> error: 12 test failures
> >> ---
> >> v2: check support_can_chroot() rather than support_become_root return
> >> value
> >> ---
> >>  sysdeps/unix/sysv/linux/tst-mount.c | 3 +++
> >>  1 file changed, 3 insertions(+)
> >>
> >> diff --git a/sysdeps/unix/sysv/linux/tst-mount.c
> b/sysdeps/unix/sysv/linux/tst-mount.c
> >> index 502d7e3433..a0b367d9df 100644
> >> --- a/sysdeps/unix/sysv/linux/tst-mount.c
> >> +++ b/sysdeps/unix/sysv/linux/tst-mount.c
> >> @@ -20,6 +20,7 @@
> >>  #include <support/check.h>
> >>  #include <support/xunistd.h>
> >>  #include <support/namespace.h>
> >> +#include <support/test-driver.h>
> >>  #include <sys/wait.h>
> >>  #include <sys/mount.h>
> >>
> >> @@ -104,6 +105,8 @@ static int
> >>  do_test (void)
> >>  {
> >>    support_become_root ();
> >> +  if (!support_can_chroot ())
> >> +    return EXIT_UNSUPPORTED;
> >>
> >>    pid_t pid = xfork ();
> >>    if (pid == 0)
> >
> > This test still hoses the system if run as root on a system without user
> > namespaces.
>
> I had not considered that configuration when I reviewed the tests.
>
> The subprocess use of fsopen, fsconfig, and fsmount could indeed leave the
> original
> mount namespace in a potentially different state than when it started.
>

This reminds me of one of my favourite bugs ever where gccgo ignored
CloneFlags in some situations and so when you built docker with it,
pivot_root was called on the default mount namespace....

Anyway, I am afk for the weekend now and have no attachment to being the
one to push the fix for this, if someone who is still at a computer can
write up a better fix, please go ahead!

Cheers,
mwh

> I think you should call and check support_enter_mount_namespace instead,
> > to make sure that the test does not modify the original mount namespace.
>
> Like this in the child?
>
> diff --git a/sysdeps/unix/sysv/linux/tst-mount.c
> b/sysdeps/unix/sysv/linux/tst-mount.c
> index 502d7e3433..d19d70d42d 100644
> --- a/sysdeps/unix/sysv/linux/tst-mount.c
> +++ b/sysdeps/unix/sysv/linux/tst-mount.c
> @@ -107,7 +107,11 @@ do_test (void)
>
>    pid_t pid = xfork ();
>    if (pid == 0)
> -    subprocess ();
> +    {
> +      if (!support_enter_mount_namespace ())
> +        FAIL_UNSUPPORTED ("could not enter new mount namespace");
> +      subprocess ();
> +    }
>
>    int status;
>    xwaitpid (pid, &status, 0);
>
> --
> Cheers,
> Carlos.
>
>