* [PATCH] linux: return UNSUPPORTED in tst-mount if support_become_root fails
@ 2022-07-14 3:23 Michael Hudson-Doyle
2022-07-14 11:24 ` Adhemerval Zanella Netto
` (2 more replies)
0 siblings, 3 replies; 19+ messages in thread
From: Michael Hudson-Doyle @ 2022-07-14 3:23 UTC (permalink / raw)
To: 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
---
sysdeps/unix/sysv/linux/tst-mount.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/sysdeps/unix/sysv/linux/tst-mount.c b/sysdeps/unix/sysv/linux/tst-mount.c
index 502d7e3433..bd75c9e704 100644
--- a/sysdeps/unix/sysv/linux/tst-mount.c
+++ b/sysdeps/unix/sysv/linux/tst-mount.c
@@ -103,7 +103,8 @@ subprocess (void)
static int
do_test (void)
{
- support_become_root ();
+ if (!support_become_root ())
+ FAIL_UNSUPPORTED("could not become root");
pid_t pid = xfork ();
if (pid == 0)
--
2.34.1
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH] linux: return UNSUPPORTED in tst-mount if support_become_root fails 2022-07-14 3:23 [PATCH] linux: return UNSUPPORTED in tst-mount if support_become_root fails Michael Hudson-Doyle @ 2022-07-14 11:24 ` Adhemerval Zanella Netto 2022-07-14 19:03 ` Carlos O'Donell 2022-07-14 19:18 ` DJ Delorie 2022-07-15 0:06 ` [PATCH v2] linux: return UNSUPPORTED in tst-mount if !support_can_chroot Michael Hudson-Doyle 2 siblings, 1 reply; 19+ messages in thread From: Adhemerval Zanella Netto @ 2022-07-14 11:24 UTC (permalink / raw) To: Michael Hudson-Doyle, libc-alpha On 14/07/22 00:23, Michael Hudson-Doyle via Libc-alpha wrote: > 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 > --- > sysdeps/unix/sysv/linux/tst-mount.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/sysdeps/unix/sysv/linux/tst-mount.c b/sysdeps/unix/sysv/linux/tst-mount.c > index 502d7e3433..bd75c9e704 100644 > --- a/sysdeps/unix/sysv/linux/tst-mount.c > +++ b/sysdeps/unix/sysv/linux/tst-mount.c > @@ -103,7 +103,8 @@ subprocess (void) > static int > do_test (void) > { > - support_become_root (); > + if (!support_become_root ()) > + FAIL_UNSUPPORTED("could not become root"); > I think the usual way is to check if process can chroot: support_become_root (); if (!support_can_chroot ()) return EXIT_UNSUPPORTED; As done by other tests. > pid_t pid = xfork (); > if (pid == 0) ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] linux: return UNSUPPORTED in tst-mount if support_become_root fails 2022-07-14 11:24 ` Adhemerval Zanella Netto @ 2022-07-14 19:03 ` Carlos O'Donell 0 siblings, 0 replies; 19+ messages in thread From: Carlos O'Donell @ 2022-07-14 19:03 UTC (permalink / raw) To: Adhemerval Zanella Netto, Michael Hudson-Doyle, libc-alpha On 7/14/22 07:24, Adhemerval Zanella Netto via Libc-alpha wrote: > > > On 14/07/22 00:23, Michael Hudson-Doyle via Libc-alpha wrote: >> 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 >> --- >> sysdeps/unix/sysv/linux/tst-mount.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/sysdeps/unix/sysv/linux/tst-mount.c b/sysdeps/unix/sysv/linux/tst-mount.c >> index 502d7e3433..bd75c9e704 100644 >> --- a/sysdeps/unix/sysv/linux/tst-mount.c >> +++ b/sysdeps/unix/sysv/linux/tst-mount.c >> @@ -103,7 +103,8 @@ subprocess (void) >> static int >> do_test (void) >> { >> - support_become_root (); >> + if (!support_become_root ()) >> + FAIL_UNSUPPORTED("could not become root"); >> > > I think the usual way is to check if process can chroot: > > support_become_root (); > if (!support_can_chroot ()) > return EXIT_UNSUPPORTED; > > As done by other tests. Agreed. We need this same fix for Fedora. I didn't catch this in my review. >> pid_t pid = xfork (); >> if (pid == 0) > -- Cheers, Carlos. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] linux: return UNSUPPORTED in tst-mount if support_become_root fails 2022-07-14 3:23 [PATCH] linux: return UNSUPPORTED in tst-mount if support_become_root fails Michael Hudson-Doyle 2022-07-14 11:24 ` Adhemerval Zanella Netto @ 2022-07-14 19:18 ` DJ Delorie 2022-07-14 22:50 ` Michael Hudson-Doyle 2022-07-15 0:06 ` [PATCH v2] linux: return UNSUPPORTED in tst-mount if !support_can_chroot Michael Hudson-Doyle 2 siblings, 1 reply; 19+ messages in thread From: DJ Delorie @ 2022-07-14 19:18 UTC (permalink / raw) To: Michael Hudson-Doyle; +Cc: libc-alpha Michael Hudson-Doyle via Libc-alpha <libc-alpha@sourceware.org> writes: > Otherwise the test fails if run in a chroot by a non-root user: Can this test run in the test-container container? I'm wondering if there's some subtle difference between support_become_root() and a more complete containerization that might enable it to run in more cases. (yes, I know the test-container might not have tmpfs pre-mounted. But then again, not all real systems will either) ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] linux: return UNSUPPORTED in tst-mount if support_become_root fails 2022-07-14 19:18 ` DJ Delorie @ 2022-07-14 22:50 ` Michael Hudson-Doyle 2022-07-14 23:16 ` DJ Delorie 0 siblings, 1 reply; 19+ messages in thread From: Michael Hudson-Doyle @ 2022-07-14 22:50 UTC (permalink / raw) To: DJ Delorie; +Cc: libc-alpha On Fri, 15 Jul 2022 at 07:19, DJ Delorie <dj@redhat.com> wrote: > > Michael Hudson-Doyle via Libc-alpha <libc-alpha@sourceware.org> writes: > > Otherwise the test fails if run in a chroot by a non-root user: > > Can this test run in the test-container container? I'm wondering if > there's some subtle difference between support_become_root() and a more > complete containerization that might enable it to run in more cases. > All the test-container tests in Ubuntu builds end up UNSUPPORTED with test-container.c:1130: unable to unshare user/fs: Operation not permitted so that wouldn't really help here (I should probably look into why this is). Cheers, mwh (yes, I know the test-container might not have tmpfs pre-mounted. But > then again, not all real systems will either) > > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] linux: return UNSUPPORTED in tst-mount if support_become_root fails 2022-07-14 22:50 ` Michael Hudson-Doyle @ 2022-07-14 23:16 ` DJ Delorie 0 siblings, 0 replies; 19+ messages in thread From: DJ Delorie @ 2022-07-14 23:16 UTC (permalink / raw) To: Michael Hudson-Doyle; +Cc: libc-alpha Michael Hudson-Doyle <michael.hudson@canonical.com> writes: > All the test-container tests in Ubuntu builds end up UNSUPPORTED with > test-container.c:1130: unable to unshare user/fs: Operation not permitted > so that wouldn't really help here (I should probably look into why this is). Yeah, Debian-based distros default to disabling user containers. There should be a hint printed as to how to re-enable them, but that's on a per-system basis, and up to the admin. ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2] linux: return UNSUPPORTED in tst-mount if !support_can_chroot 2022-07-14 3:23 [PATCH] linux: return UNSUPPORTED in tst-mount if support_become_root fails Michael Hudson-Doyle 2022-07-14 11:24 ` Adhemerval Zanella Netto 2022-07-14 19:18 ` DJ Delorie @ 2022-07-15 0:06 ` Michael Hudson-Doyle 2022-07-15 7:07 ` Florian Weimer 2022-07-17 23:16 ` [PATCH v3] linux: return UNSUPPORTED from tst-mount if entering mount namespace fails Michael Hudson-Doyle 2 siblings, 2 replies; 19+ messages in thread From: Michael Hudson-Doyle @ 2022-07-15 0:06 UTC (permalink / raw) To: 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) -- 2.34.1 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2] linux: return UNSUPPORTED in tst-mount if !support_can_chroot 2022-07-15 0:06 ` [PATCH v2] linux: return UNSUPPORTED in tst-mount if !support_can_chroot Michael Hudson-Doyle @ 2022-07-15 7:07 ` Florian Weimer 2022-07-15 15:35 ` Carlos O'Donell 2022-07-17 23:16 ` [PATCH v3] linux: return UNSUPPORTED from tst-mount if entering mount namespace fails Michael Hudson-Doyle 1 sibling, 1 reply; 19+ messages in thread From: Florian Weimer @ 2022-07-15 7:07 UTC (permalink / raw) To: Michael Hudson-Doyle via Libc-alpha * 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 think you should call and check support_enter_mount_namespace instead, to make sure that the test does not modify the original mount namespace. Thanks, Florian ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2] linux: return UNSUPPORTED in tst-mount if !support_can_chroot 2022-07-15 7:07 ` Florian Weimer @ 2022-07-15 15:35 ` Carlos O'Donell 2022-07-15 15:44 ` Florian Weimer 2022-07-15 21:01 ` Michael Hudson-Doyle 0 siblings, 2 replies; 19+ messages in thread From: Carlos O'Donell @ 2022-07-15 15:35 UTC (permalink / raw) To: Florian Weimer, Michael Hudson-Doyle via Libc-alpha 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. > 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. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2] linux: return UNSUPPORTED in tst-mount if !support_can_chroot 2022-07-15 15:35 ` Carlos O'Donell @ 2022-07-15 15:44 ` Florian Weimer 2022-07-17 21:44 ` Michael Hudson-Doyle 2022-07-15 21:01 ` Michael Hudson-Doyle 1 sibling, 1 reply; 19+ messages in thread From: Florian Weimer @ 2022-07-15 15:44 UTC (permalink / raw) To: Carlos O'Donell; +Cc: Michael Hudson-Doyle via Libc-alpha * Carlos O'Donell: >> 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 (); > + } Yes, except that you need to change xwaitpid (pid, &status, 0); TEST_VERIFY (WIFEXITED (status)); as well, to handle status 77. I'm not entirely sure the fork is necessary, though. Thanks, Florian ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2] linux: return UNSUPPORTED in tst-mount if !support_can_chroot 2022-07-15 15:44 ` Florian Weimer @ 2022-07-17 21:44 ` Michael Hudson-Doyle 0 siblings, 0 replies; 19+ messages in thread From: Michael Hudson-Doyle @ 2022-07-17 21:44 UTC (permalink / raw) To: Florian Weimer; +Cc: Carlos O'Donell, Michael Hudson-Doyle via Libc-alpha On Sat, 16 Jul 2022 at 03:45, Florian Weimer via Libc-alpha < libc-alpha@sourceware.org> wrote: > * Carlos O'Donell: > > >> 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 (); > > + } > > Yes, except that you need to change > > xwaitpid (pid, &status, 0); > TEST_VERIFY (WIFEXITED (status)); > > as well, to handle status 77. > I was a bit confused by this, as subprocess() already calls FAIL_UNSUPPORTED (if fsopen sets errno to ENOSYS). WIFEXITED (status) doesn't distinguish between status 0 or 77 though... So I guess I don't understand how the test machinery works here. > I'm not entirely sure the fork is necessary, though. > Yes, I don't see why it's there either. Cheers, mwh ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2] linux: return UNSUPPORTED in tst-mount if !support_can_chroot 2022-07-15 15:35 ` Carlos O'Donell 2022-07-15 15:44 ` Florian Weimer @ 2022-07-15 21:01 ` Michael Hudson-Doyle 2022-07-16 0:26 ` Carlos O'Donell 1 sibling, 1 reply; 19+ messages in thread From: Michael Hudson-Doyle @ 2022-07-15 21:01 UTC (permalink / raw) To: Carlos O'Donell; +Cc: Florian Weimer, Michael Hudson-Doyle via Libc-alpha 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. > > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2] linux: return UNSUPPORTED in tst-mount if !support_can_chroot 2022-07-15 21:01 ` Michael Hudson-Doyle @ 2022-07-16 0:26 ` Carlos O'Donell 0 siblings, 0 replies; 19+ messages in thread From: Carlos O'Donell @ 2022-07-16 0:26 UTC (permalink / raw) To: Michael Hudson-Doyle; +Cc: Florian Weimer, Michael Hudson-Doyle via Libc-alpha On 7/15/22 17:01, Michael Hudson-Doyle wrote: > 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! Thanks for the notice. We can review again next week and fix the issue before 2.36 releases. -- Cheers, Carlos. ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v3] linux: return UNSUPPORTED from tst-mount if entering mount namespace fails 2022-07-15 0:06 ` [PATCH v2] linux: return UNSUPPORTED in tst-mount if !support_can_chroot Michael Hudson-Doyle 2022-07-15 7:07 ` Florian Weimer @ 2022-07-17 23:16 ` Michael Hudson-Doyle 2022-07-18 12:59 ` Carlos O'Donell 2022-07-18 13:13 ` Mark Wielaard 1 sibling, 2 replies; 19+ messages in thread From: Michael Hudson-Doyle @ 2022-07-17 23:16 UTC (permalink / raw) To: libc-alpha Before this 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 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. 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> #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) { + support_become_root (); + if (!support_enter_mount_namespace ()) + FAIL_UNSUPPORTED ("cannot enter mount namespace, skipping test"); + 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> -- 2.34.1 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3] linux: return UNSUPPORTED from tst-mount if entering mount namespace fails 2022-07-17 23:16 ` [PATCH v3] linux: return UNSUPPORTED from tst-mount if entering mount namespace fails Michael Hudson-Doyle @ 2022-07-18 12:59 ` Carlos O'Donell 2022-07-18 18:59 ` Michael Hudson-Doyle 2022-07-18 13:13 ` Mark Wielaard 1 sibling, 1 reply; 19+ messages in thread From: Carlos O'Donell @ 2022-07-18 12:59 UTC (permalink / raw) To: Michael Hudson-Doyle, libc-alpha 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? 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. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3] linux: return UNSUPPORTED from tst-mount if entering mount namespace fails 2022-07-18 12:59 ` Carlos O'Donell @ 2022-07-18 18:59 ` Michael Hudson-Doyle 2022-07-19 2:51 ` Carlos O'Donell 0 siblings, 1 reply; 19+ messages in thread From: Michael Hudson-Doyle @ 2022-07-18 18:59 UTC (permalink / raw) To: Carlos O'Donell; +Cc: libc-alpha 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. > > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3] linux: return UNSUPPORTED from tst-mount if entering mount namespace fails 2022-07-18 18:59 ` Michael Hudson-Doyle @ 2022-07-19 2:51 ` Carlos O'Donell 0 siblings, 0 replies; 19+ messages in thread From: Carlos O'Donell @ 2022-07-19 2:51 UTC (permalink / raw) To: Michael Hudson-Doyle; +Cc: libc-alpha On 7/18/22 14:59, Michael Hudson-Doyle wrote: > 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. Looks great! Thank you for updating patchwork. Committed content matches posted content. You added the reviewers (mine) Reviewed-by: tag. All awesome. Thank you again! -- Cheers, Carlos. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3] linux: return UNSUPPORTED from tst-mount if entering mount namespace fails 2022-07-17 23:16 ` [PATCH v3] linux: return UNSUPPORTED from tst-mount if entering mount namespace fails Michael Hudson-Doyle 2022-07-18 12:59 ` Carlos O'Donell @ 2022-07-18 13:13 ` Mark Wielaard 2022-07-18 13:45 ` Carlos O'Donell 1 sibling, 1 reply; 19+ messages in thread From: Mark Wielaard @ 2022-07-18 13:13 UTC (permalink / raw) To: Michael Hudson-Doyle; +Cc: libc-alpha Hi Michael, On Mon, Jul 18, 2022 at 11:16:57AM +1200, Michael Hudson-Doyle via Libc-alpha wrote: > Before this 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 > > 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. > > Also change the test to remove the unnecessary fork. Tested on x86_64, inside a fedora container where this patch turns the FAIL into an UNSUPPORTED. And "outside" a container on fedora where this patch makes misc/tst-mount still PASS. Thanks, Mark ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3] linux: return UNSUPPORTED from tst-mount if entering mount namespace fails 2022-07-18 13:13 ` Mark Wielaard @ 2022-07-18 13:45 ` Carlos O'Donell 0 siblings, 0 replies; 19+ messages in thread From: Carlos O'Donell @ 2022-07-18 13:45 UTC (permalink / raw) To: Mark Wielaard, Michael Hudson-Doyle; +Cc: libc-alpha On 7/18/22 09:13, Mark Wielaard wrote: > Hi Michael, > > On Mon, Jul 18, 2022 at 11:16:57AM +1200, Michael Hudson-Doyle via Libc-alpha wrote: >> Before this 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 >> >> 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. >> >> Also change the test to remove the unnecessary fork. > > Tested on x86_64, inside a fedora container where this patch turns the > FAIL into an UNSUPPORTED. And "outside" a container on fedora where > this patch makes misc/tst-mount still PASS. If you test a patch please feel free to provide the Tested-by: tag which patchwork will aggregate so when I apply the patch from git-pw your tag will get added and end up in the commit (similar to b4). It records the amazing work you did by testing a patch! :-) -- Cheers, Carlos. ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2022-07-19 2:51 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-07-14 3:23 [PATCH] linux: return UNSUPPORTED in tst-mount if support_become_root fails Michael Hudson-Doyle 2022-07-14 11:24 ` Adhemerval Zanella Netto 2022-07-14 19:03 ` Carlos O'Donell 2022-07-14 19:18 ` DJ Delorie 2022-07-14 22:50 ` Michael Hudson-Doyle 2022-07-14 23:16 ` DJ Delorie 2022-07-15 0:06 ` [PATCH v2] linux: return UNSUPPORTED in tst-mount if !support_can_chroot Michael Hudson-Doyle 2022-07-15 7:07 ` Florian Weimer 2022-07-15 15:35 ` Carlos O'Donell 2022-07-15 15:44 ` Florian Weimer 2022-07-17 21:44 ` Michael Hudson-Doyle 2022-07-15 21:01 ` Michael Hudson-Doyle 2022-07-16 0:26 ` Carlos O'Donell 2022-07-17 23:16 ` [PATCH v3] linux: return UNSUPPORTED from tst-mount if entering mount namespace fails Michael Hudson-Doyle 2022-07-18 12:59 ` Carlos O'Donell 2022-07-18 18:59 ` Michael Hudson-Doyle 2022-07-19 2:51 ` Carlos O'Donell 2022-07-18 13:13 ` Mark Wielaard 2022-07-18 13:45 ` Carlos O'Donell
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).