| Message ID | 20230531150003.3803030-1-siddhesh@sourceware.org |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] support: Don't fail on fchown when spawning sgid processes | expand |
On Mai 31 2023, Siddhesh Poyarekar via Libc-alpha wrote: > diff --git a/support/support_capture_subprocess.c b/support/support_capture_subprocess.c > index bae7d5fb20..3881e3610a 100644 > --- a/support/support_capture_subprocess.c > +++ b/support/support_capture_subprocess.c > @@ -153,9 +153,18 @@ copy_and_spawn_sgid (char *child_id, gid_t gid) > p += wrcount; > } > } > - TEST_VERIFY (fchown (outfd, getuid (), gid) == 0); > + > + int chowned = 0; I think that can be bool.
On Wed, May 31, 2023 at 5:00 PM Siddhesh Poyarekar <siddhesh@sourceware.org> wrote: > > In some cases (e.g. when podman creates user containers), the only other > group assigned to the executing user is nobody and fchown fails with it > because the group is not mapped. Do not fail the test in this case, > instead exit as unsupported. > > Reported-by: Frederic Berat <fberat@redhat.com> > Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org> > --- > support/support_capture_subprocess.c | 15 +++++++++++++-- > 1 file changed, 13 insertions(+), 2 deletions(-) > > diff --git a/support/support_capture_subprocess.c b/support/support_capture_subprocess.c > index bae7d5fb20..3881e3610a 100644 > --- a/support/support_capture_subprocess.c > +++ b/support/support_capture_subprocess.c > @@ -153,9 +153,18 @@ copy_and_spawn_sgid (char *child_id, gid_t gid) > p += wrcount; > } > } > - TEST_VERIFY (fchown (outfd, getuid (), gid) == 0); > + > + int chowned = 0; > + TEST_VERIFY ((chowned = fchown (outfd, getuid (), gid)) == 0 > + || errno == EPERM); > if (support_record_failure_is_failed ()) > goto err; > + else if (chowned != 0) > + { > + ret = 77; > + goto err; > + } > + > TEST_VERIFY (fchmod (outfd, 02750) == 0); > if (support_record_failure_is_failed ()) > goto err; > @@ -192,8 +201,10 @@ err: > free (dirname); > } > > + if (ret == 77) > + FAIL_UNSUPPORTED ("Failed to make sgid executable for test\n"); > if (ret != 0) > - FAIL_EXIT1("Failed to make sgid executable for test\n"); > + FAIL_EXIT1 ("Failed to make sgid executable for test\n"); > > return status; > } > -- > 2.40.1 > Looks good to me: $> cat stdlib/tst-secure-getenv.test-result && cat stdlib/tst-secure-getenv.out UNSUPPORTED: stdlib/tst-secure-getenv original exit status 77 error: support_capture_subprocess.c:205: Failed to make sgid executable for test Tested-by: Frédéric Bérat <fberat@redhat.com>
On 2023-05-31 11:13, Andreas Schwab wrote: > On Mai 31 2023, Siddhesh Poyarekar via Libc-alpha wrote: > >> diff --git a/support/support_capture_subprocess.c b/support/support_capture_subprocess.c >> index bae7d5fb20..3881e3610a 100644 >> --- a/support/support_capture_subprocess.c >> +++ b/support/support_capture_subprocess.c >> @@ -153,9 +153,18 @@ copy_and_spawn_sgid (char *child_id, gid_t gid) >> p += wrcount; >> } >> } >> - TEST_VERIFY (fchown (outfd, getuid (), gid) == 0); >> + >> + int chowned = 0; > > I think that can be bool. > Thanks, fixed in v3. Sid
diff --git a/support/support_capture_subprocess.c b/support/support_capture_subprocess.c index bae7d5fb20..3881e3610a 100644 --- a/support/support_capture_subprocess.c +++ b/support/support_capture_subprocess.c @@ -153,9 +153,18 @@ copy_and_spawn_sgid (char *child_id, gid_t gid) p += wrcount; } } - TEST_VERIFY (fchown (outfd, getuid (), gid) == 0); + + int chowned = 0; + TEST_VERIFY ((chowned = fchown (outfd, getuid (), gid)) == 0 + || errno == EPERM); if (support_record_failure_is_failed ()) goto err; + else if (chowned != 0) + { + ret = 77; + goto err; + } + TEST_VERIFY (fchmod (outfd, 02750) == 0); if (support_record_failure_is_failed ()) goto err; @@ -192,8 +201,10 @@ err: free (dirname); } + if (ret == 77) + FAIL_UNSUPPORTED ("Failed to make sgid executable for test\n"); if (ret != 0) - FAIL_EXIT1("Failed to make sgid executable for test\n"); + FAIL_EXIT1 ("Failed to make sgid executable for test\n"); return status; }
In some cases (e.g. when podman creates user containers), the only other group assigned to the executing user is nobody and fchown fails with it because the group is not mapped. Do not fail the test in this case, instead exit as unsupported. Reported-by: Frederic Berat <fberat@redhat.com> Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org> --- support/support_capture_subprocess.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-)