Fix error reporting (false negatives) in SGID tests

And simplify the interface of support_capture_subprogram_self_sgid.

Use the existing framework for temporary directories (now with
mode 0700) and directory/file deletion.  Handle all execution
errors within support_capture_subprogram_self_sgid.  In particular,
this includes test failures because the invoked program did not
exit with exit status zero.  Existing tests that expect exit
status 42 are adjusted to use zero instead.

In addition, fix callers not to call exit (0) with test failures
pending (which may mask them, especially when running with --direct).

Fixes commit 35fc356fa3
("elf: Fix subprocess status handling for tst-dlopen-sgid (bug 32987)").

Reviewed-by: Carlos O'Donell <carlos@redhat.com>
(cherry picked from commit 3a3fb2ed83)
This commit is contained in:
Florian Weimer 2025-05-22 14:36:37 +02:00
parent 23a02e382c
commit dbc83657e2
6 changed files with 36 additions and 132 deletions

View File

@ -70,13 +70,7 @@ do_test (void)
free (libdir); free (libdir);
int status = support_capture_subprogram_self_sgid (magic_argument); support_capture_subprogram_self_sgid (magic_argument);
if (WEXITSTATUS (status) == EXIT_UNSUPPORTED)
return EXIT_UNSUPPORTED;
if (!WIFEXITED (status))
FAIL_EXIT1 ("Unexpected exit status %d from child process\n", status);
return 0; return 0;
} }

View File

@ -105,10 +105,7 @@ do_test (int argc, char **argv)
if (ret != 0) if (ret != 0)
exit (1); exit (1);
return 0;
/* Special return code to make sure that the child executed all the way
through. */
exit (42);
} }
else else
{ {
@ -127,18 +124,7 @@ do_test (int argc, char **argv)
continue; continue;
} }
int status = support_capture_subprogram_self_sgid (buf); support_capture_subprogram_self_sgid (buf);
/* Bail out early if unsupported. */
if (WEXITSTATUS (status) == EXIT_UNSUPPORTED)
return EXIT_UNSUPPORTED;
if (WEXITSTATUS (status) != 42)
{
printf (" [%d] child failed with status %d\n", i,
WEXITSTATUS (status));
support_record_failure ();
}
} }
return 0; return 0;
} }

View File

@ -147,10 +147,7 @@ do_test (int argc, char **argv)
if (ret != 0) if (ret != 0)
exit (1); exit (1);
return 0;
/* Special return code to make sure that the child executed all the way
through. */
exit (42);
} }
else else
{ {
@ -174,17 +171,7 @@ do_test (int argc, char **argv)
free (profilepath); free (profilepath);
} }
int status = support_capture_subprogram_self_sgid (SETGID_CHILD); support_capture_subprogram_self_sgid (SETGID_CHILD);
if (WEXITSTATUS (status) == EXIT_UNSUPPORTED)
exit (EXIT_UNSUPPORTED);
if (WEXITSTATUS (status) != 42)
{
printf (" child failed with status %d\n",
WEXITSTATUS (status));
support_record_failure ();
}
return 0; return 0;
} }

View File

@ -57,13 +57,7 @@ do_test (void)
exit (1); exit (1);
} }
int status = support_capture_subprogram_self_sgid (MAGIC_ARGUMENT); support_capture_subprogram_self_sgid (MAGIC_ARGUMENT);
if (WEXITSTATUS (status) == EXIT_UNSUPPORTED)
return EXIT_UNSUPPORTED;
if (!WIFEXITED (status))
FAIL_EXIT1 ("Unexpected exit status %d from child process\n", status);
return 0; return 0;
} }
@ -82,6 +76,7 @@ alternative_main (int argc, char **argv)
if (secure_getenv ("PATH") != NULL) if (secure_getenv ("PATH") != NULL)
FAIL_EXIT (4, "PATH variable not filtered out\n"); FAIL_EXIT (4, "PATH variable not filtered out\n");
support_record_failure_barrier ();
exit (EXIT_SUCCESS); exit (EXIT_SUCCESS);
} }
} }

View File

@ -42,10 +42,12 @@ struct support_capture_subprocess support_capture_subprocess
struct support_capture_subprocess support_capture_subprogram struct support_capture_subprocess support_capture_subprogram
(const char *file, char *const argv[], char *const envp[]); (const char *file, char *const argv[], char *const envp[]);
/* Copy the running program into a setgid binary and run it with CHILD_ID /* Copy the running program into a setgid binary and run it with
argument. If execution is successful, return the exit status of the child CHILD_ID argument. If the program exits with a non-zero status,
program, otherwise return a non-zero failure exit code. */ exit with that exit status (or status 1 if the program did not exit
int support_capture_subprogram_self_sgid (const char *child_id); normally). If the test cannot be performed, exit with
EXIT_UNSUPPORTED. */
void support_capture_subprogram_self_sgid (const char *child_id);
/* Deallocate the subprocess data captured by /* Deallocate the subprocess data captured by
support_capture_subprocess. */ support_capture_subprocess. */

View File

@ -31,6 +31,7 @@
#include <support/xsocket.h> #include <support/xsocket.h>
#include <support/xspawn.h> #include <support/xspawn.h>
#include <support/support.h> #include <support/support.h>
#include <support/temp_file.h>
#include <support/test-driver.h> #include <support/test-driver.h>
static void static void
@ -113,105 +114,44 @@ support_capture_subprogram (const char *file, char *const argv[],
/* Copies the executable into a restricted directory, so that we can /* Copies the executable into a restricted directory, so that we can
safely make it SGID with the TARGET group ID. Then runs the safely make it SGID with the TARGET group ID. Then runs the
executable. */ executable. */
static int static void
copy_and_spawn_sgid (const char *child_id, gid_t gid) copy_and_spawn_sgid (const char *child_id, gid_t gid)
{ {
char *dirname = xasprintf ("%s/tst-tunables-setuid.%jd", char *dirname = support_create_temp_directory ("tst-glibc-sgid-");
test_dir, (intmax_t) getpid ());
char *execname = xasprintf ("%s/bin", dirname); char *execname = xasprintf ("%s/bin", dirname);
int infd = -1; add_temp_file (execname);
int outfd = -1;
int ret = 1, status = 1;
TEST_VERIFY (mkdir (dirname, 0700) == 0); if (access ("/proc/self/exe", R_OK) != 0)
if (support_record_failure_is_failed ())
goto err;
infd = open ("/proc/self/exe", O_RDONLY);
if (infd < 0)
FAIL_UNSUPPORTED ("unsupported: Cannot read binary from procfs\n"); FAIL_UNSUPPORTED ("unsupported: Cannot read binary from procfs\n");
outfd = open (execname, O_WRONLY | O_CREAT | O_EXCL, 0700); support_copy_file ("/proc/self/exe", execname);
TEST_VERIFY (outfd >= 0);
if (support_record_failure_is_failed ())
goto err;
char buf[4096]; if (chown (execname, getuid (), gid) != 0)
for (;;) FAIL_UNSUPPORTED ("cannot change group of \"%s\" to %jd: %m",
{ execname, (intmax_t) gid);
ssize_t rdcount = read (infd, buf, sizeof (buf));
TEST_VERIFY (rdcount >= 0);
if (support_record_failure_is_failed ())
goto err;
if (rdcount == 0)
break;
char *p = buf;
char *end = buf + rdcount;
while (p != end)
{
ssize_t wrcount = write (outfd, buf, end - p);
if (wrcount == 0)
errno = ENOSPC;
TEST_VERIFY (wrcount > 0);
if (support_record_failure_is_failed ())
goto err;
p += wrcount;
}
}
bool chowned = false; if (chmod (execname, 02750) != 0)
TEST_VERIFY ((chowned = fchown (outfd, getuid (), gid) == 0) FAIL_UNSUPPORTED ("cannot make \"%s\" SGID: %m ", execname);
|| errno == EPERM);
if (support_record_failure_is_failed ())
goto err;
else if (!chowned)
{
ret = 77;
goto err;
}
TEST_VERIFY (fchmod (outfd, 02750) == 0);
if (support_record_failure_is_failed ())
goto err;
TEST_VERIFY (close (outfd) == 0);
if (support_record_failure_is_failed ())
goto err;
TEST_VERIFY (close (infd) == 0);
if (support_record_failure_is_failed ())
goto err;
/* We have the binary, now spawn the subprocess. Avoid using /* We have the binary, now spawn the subprocess. Avoid using
support_subprogram because we only want the program exit status, not the support_subprogram because we only want the program exit status, not the
contents. */ contents. */
ret = 0;
infd = outfd = -1;
char * const args[] = {execname, (char *) child_id, NULL}; char * const args[] = {execname, (char *) child_id, NULL};
int status = support_subprogram_wait (args[0], args);
status = support_subprogram_wait (args[0], args); free (execname);
free (dirname);
err: if (WIFEXITED (status))
if (outfd >= 0)
close (outfd);
if (infd >= 0)
close (infd);
if (execname != NULL)
{ {
unlink (execname); if (WEXITSTATUS (status) == 0)
free (execname); return;
else
exit (WEXITSTATUS (status));
} }
if (dirname != NULL) else
{ FAIL_EXIT1 ("subprogram failed with status %d", status);
rmdir (dirname);
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");
return status;
} }
/* Returns true if a group with NAME has been found, and writes its /* Returns true if a group with NAME has been found, and writes its
@ -253,7 +193,7 @@ find_sgid_group (gid_t *target, const char *name)
return ok; return ok;
} }
int void
support_capture_subprogram_self_sgid (const char *child_id) support_capture_subprogram_self_sgid (const char *child_id)
{ {
const int count = 64; const int count = 64;
@ -288,7 +228,7 @@ support_capture_subprogram_self_sgid (const char *child_id)
(intmax_t) getuid ()); (intmax_t) getuid ());
} }
return copy_and_spawn_sgid (child_id, target); copy_and_spawn_sgid (child_id, target);
} }
void void