Re: [PATCH v3] linux: return UNSUPPORTED from tst-mount if entering mount namespace fails

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

On Tue, 19 Jul 2022 at 00:59, Carlos O'Donell <carlos@redhat.com> wrote:

> On 7/17/22 19:16, Michael Hudson-Doyle via Libc-alpha wrote:
> > Before this the test fails if run in a chroot by a non-root user:
>
> LGTM. OK for glibc 2.36. Would you like me to push this for you?
>

Thanks. I pushed it and marked the patch as committed in patchwork, I think
this was my first push to master so let me know if I missed anything.

Cheers,
mwh


> Reviewed-by: Carlos O'Donell <carlos@redhat.com>
>
> > 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
> >
> > Checking that the test can enter a new mount namespace is more correct
> > than just checking the return value of support_become_root() as the test
> > code changes the mount namespace it runs in so running it as root on a
> > system that does not support mount namespaces should still skip.
>
> Agreed.
>
> >
> > Also change the test to remove the unnecessary fork.
> > ---
> > v3: check support_enter_mount_namespace() return value, remove fork
> > v2: check support_can_chroot() rather than support_become_root return
> > value
> > ---
> >  sysdeps/unix/sysv/linux/tst-mount.c | 25 ++++++-------------------
> >  1 file changed, 6 insertions(+), 19 deletions(-)
> >
> > diff --git a/sysdeps/unix/sysv/linux/tst-mount.c
> b/sysdeps/unix/sysv/linux/tst-mount.c
> > index 502d7e3433..b6333a60e6 100644
> > --- a/sysdeps/unix/sysv/linux/tst-mount.c
> > +++ b/sysdeps/unix/sysv/linux/tst-mount.c
> > @@ -20,15 +20,18 @@
> >  #include <support/check.h>
> >  #include <support/xunistd.h>
> >  #include <support/namespace.h>
> > -#include <sys/wait.h>
>
> OK.
>
> >  #include <sys/mount.h>
> >
> >  _Static_assert (sizeof (struct mount_attr) == MOUNT_ATTR_SIZE_VER0,
> >               "sizeof (struct mount_attr) != MOUNT_ATTR_SIZE_VER0");
> >
> > -static void
> > -subprocess (void)
> > +static int
> > +do_test (void)
>
> OK.
>
> >  {
> > +  support_become_root ();
> > +  if (!support_enter_mount_namespace ())
> > +    FAIL_UNSUPPORTED ("cannot enter mount namespace, skipping test");
>
> OK.
>
> > +
> >    int r = fsopen ("it_should_be_not_a_valid_mount", 0);
> >    TEST_VERIFY_EXIT (r == -1);
> >    if (errno == ENOSYS)
> > @@ -100,20 +103,4 @@ subprocess (void)
> >    _exit (0);
> >  }
> >
> > -static int
> > -do_test (void)
> > -{
> > -  support_become_root ();
> > -
> > -  pid_t pid = xfork ();
> > -  if (pid == 0)
> > -    subprocess ();
> > -
> > -  int status;
> > -  xwaitpid (pid, &status, 0);
> > -  TEST_VERIFY (WIFEXITED (status));
> > -
> > -  return 0;
> > -}
> > -
> >  #include <support/test-driver.c>
>
>
> --
> Cheers,
> Carlos.
>
>