From 940aa56a78dd9718d0790266fccc36e826c6f973 Mon Sep 17 00:00:00 2001 From: Florian Weimer Date: Thu, 22 May 2025 14:58:58 +0200 Subject: [PATCH] SGID test enhancements (RHEL-92697) Resolves: RHEL-92697 --- glibc-RHEL-92697-8.patch | 52 +++++++ glibc-RHEL-92697-9.patch | 314 +++++++++++++++++++++++++++++++++++++++ glibc.spec | 7 +- 3 files changed, 372 insertions(+), 1 deletion(-) create mode 100644 glibc-RHEL-92697-8.patch create mode 100644 glibc-RHEL-92697-9.patch diff --git a/glibc-RHEL-92697-8.patch b/glibc-RHEL-92697-8.patch new file mode 100644 index 0000000..208cb0f --- /dev/null +++ b/glibc-RHEL-92697-8.patch @@ -0,0 +1,52 @@ +commit 6286cca2cb8389dcffec39238a8bf15ffea96396 +Author: Siddhesh Poyarekar +Date: Thu Jun 1 07:23:15 2023 -0400 + + support: Don't fail on fchown when spawning sgid processes + + 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: Frédéric Bérat + Tested-by: Frédéric Bérat + Signed-off-by: Siddhesh Poyarekar + Reviewed-by: Carlos O'Donell + +diff --git a/support/support_capture_subprocess.c b/support/support_capture_subprocess.c +index 755ee13553b3d253..544636e52eeaee73 100644 +--- a/support/support_capture_subprocess.c ++++ b/support/support_capture_subprocess.c +@@ -157,9 +157,18 @@ copy_and_spawn_sgid (const char *child_id, gid_t gid) + p += wrcount; + } + } +- TEST_VERIFY (fchown (outfd, getuid (), gid) == 0); ++ ++ bool chowned = false; ++ TEST_VERIFY ((chowned = fchown (outfd, getuid (), gid) == 0) ++ || 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; +@@ -196,8 +205,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; + } diff --git a/glibc-RHEL-92697-9.patch b/glibc-RHEL-92697-9.patch new file mode 100644 index 0000000..0a34dbf --- /dev/null +++ b/glibc-RHEL-92697-9.patch @@ -0,0 +1,314 @@ +commit 3a3fb2ed83f79100c116c824454095ecfb335ad7 +Author: Florian Weimer +Date: Thu May 22 14:36:37 2025 +0200 + + 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 35fc356fa3b4f485bd3ba3114c9f774e5df7d3c2 + ("elf: Fix subprocess status handling for tst-dlopen-sgid (bug 32987)"). + + Reviewed-by: Carlos O'Donell + +Conflicts: + elf/tst-env-setuid.c + (missing commit 11f7e3dd8fed66e0b8740af440cd3151e55a466f + ("elf: Add all malloc tunable to unsecvars") downstream) + +diff --git a/elf/tst-dlopen-sgid.c b/elf/tst-dlopen-sgid.c +index 5688b79f2e870b1d..8aec52e19fc56aba 100644 +--- a/elf/tst-dlopen-sgid.c ++++ b/elf/tst-dlopen-sgid.c +@@ -70,13 +70,7 @@ do_test (void) + + free (libdir); + +- int status = 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); ++ support_capture_subprogram_self_sgid (magic_argument); + + return 0; + } +diff --git a/elf/tst-env-setuid-tunables.c b/elf/tst-env-setuid-tunables.c +index cd4e84364074c613..0ee5416026242c65 100644 +--- a/elf/tst-env-setuid-tunables.c ++++ b/elf/tst-env-setuid-tunables.c +@@ -127,10 +127,7 @@ do_test (int argc, char **argv) + + if (ret != 0) + exit (1); +- +- /* Special return code to make sure that the child executed all the way +- through. */ +- exit (42); ++ return 0; + } + else + { +@@ -149,18 +146,7 @@ do_test (int argc, char **argv) + continue; + } + +- int status = 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 (); +- } ++ support_capture_subprogram_self_sgid (buf); + } + return 0; + } +diff --git a/elf/tst-env-setuid.c b/elf/tst-env-setuid.c +index 49b5e319e27c2404..0b6e60836ade7958 100644 +--- a/elf/tst-env-setuid.c ++++ b/elf/tst-env-setuid.c +@@ -104,20 +104,14 @@ do_test (int argc, char **argv) + if (ret != 0) + exit (1); + +- exit (EXIT_SUCCESS); ++ return 0; + } + else + { + if (test_parent () != 0) + exit (1); + +- int status = support_capture_subprogram_self_sgid (SETGID_CHILD); +- +- if (WEXITSTATUS (status) == EXIT_UNSUPPORTED) +- return EXIT_UNSUPPORTED; +- +- if (!WIFEXITED (status)) +- FAIL_EXIT1 ("Unexpected exit status %d from child process\n", status); ++ support_capture_subprogram_self_sgid (SETGID_CHILD); + + return 0; + } +diff --git a/stdlib/tst-secure-getenv.c b/stdlib/tst-secure-getenv.c +index 5567c9ae215fd924..cd48c717623628e6 100644 +--- a/stdlib/tst-secure-getenv.c ++++ b/stdlib/tst-secure-getenv.c +@@ -57,13 +57,7 @@ do_test (void) + exit (1); + } + +- int status = 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); ++ support_capture_subprogram_self_sgid (MAGIC_ARGUMENT); + + return 0; + } +@@ -82,6 +76,7 @@ alternative_main (int argc, char **argv) + if (secure_getenv ("PATH") != NULL) + FAIL_EXIT (4, "PATH variable not filtered out\n"); + ++ support_record_failure_barrier (); + exit (EXIT_SUCCESS); + } + } +diff --git a/support/capture_subprocess.h b/support/capture_subprocess.h +index 4229300d4b0aab29..afec0b14d23be7b1 100644 +--- a/support/capture_subprocess.h ++++ b/support/capture_subprocess.h +@@ -41,10 +41,12 @@ struct support_capture_subprocess support_capture_subprocess + struct support_capture_subprocess support_capture_subprogram + (const char *file, char *const argv[]); + +-/* Copy the running program into a setgid binary and run it with CHILD_ID +- argument. If execution is successful, return the exit status of the child +- program, otherwise return a non-zero failure exit code. */ +-int support_capture_subprogram_self_sgid (const char *child_id); ++/* Copy the running program into a setgid binary and run it with ++ CHILD_ID argument. If the program exits with a non-zero status, ++ exit with that exit status (or status 1 if the program did not exit ++ 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 + support_capture_subprocess. */ +diff --git a/support/support_capture_subprocess.c b/support/support_capture_subprocess.c +index 544636e52eeaee73..73e3a16ddc63da94 100644 +--- a/support/support_capture_subprocess.c ++++ b/support/support_capture_subprocess.c +@@ -31,6 +31,7 @@ + #include + #include + #include ++#include + #include + + static void +@@ -112,105 +113,44 @@ support_capture_subprogram (const char *file, char *const argv[]) + /* Copies the executable into a restricted directory, so that we can + safely make it SGID with the TARGET group ID. Then runs the + executable. */ +-static int ++static void + copy_and_spawn_sgid (const char *child_id, gid_t gid) + { +- char *dirname = xasprintf ("%s/tst-tunables-setuid.%jd", +- test_dir, (intmax_t) getpid ()); ++ char *dirname = support_create_temp_directory ("tst-glibc-sgid-"); + char *execname = xasprintf ("%s/bin", dirname); +- int infd = -1; +- int outfd = -1; +- int ret = 1, status = 1; +- +- TEST_VERIFY (mkdir (dirname, 0700) == 0); +- if (support_record_failure_is_failed ()) +- goto err; ++ add_temp_file (execname); + +- infd = open ("/proc/self/exe", O_RDONLY); +- if (infd < 0) ++ if (access ("/proc/self/exe", R_OK) != 0) + FAIL_UNSUPPORTED ("unsupported: Cannot read binary from procfs\n"); + +- outfd = open (execname, O_WRONLY | O_CREAT | O_EXCL, 0700); +- TEST_VERIFY (outfd >= 0); +- if (support_record_failure_is_failed ()) +- goto err; +- +- char buf[4096]; +- for (;;) +- { +- 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; +- } +- } ++ support_copy_file ("/proc/self/exe", execname); + +- bool chowned = false; +- TEST_VERIFY ((chowned = fchown (outfd, getuid (), gid) == 0) +- || errno == EPERM); +- if (support_record_failure_is_failed ()) +- goto err; +- else if (!chowned) +- { +- ret = 77; +- goto err; +- } ++ if (chown (execname, getuid (), gid) != 0) ++ FAIL_UNSUPPORTED ("cannot change group of \"%s\" to %jd: %m", ++ execname, (intmax_t) gid); + +- 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; ++ if (chmod (execname, 02750) != 0) ++ FAIL_UNSUPPORTED ("cannot make \"%s\" SGID: %m ", execname); + + /* We have the binary, now spawn the subprocess. Avoid using + support_subprogram because we only want the program exit status, not the + contents. */ +- ret = 0; +- infd = outfd = -1; + + 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 (outfd >= 0) +- close (outfd); +- if (infd >= 0) +- close (infd); +- if (execname != NULL) +- { +- unlink (execname); +- free (execname); +- } +- if (dirname != NULL) ++ if (WIFEXITED (status)) + { +- rmdir (dirname); +- free (dirname); ++ if (WEXITSTATUS (status) == 0) ++ return; ++ else ++ exit (WEXITSTATUS (status)); + } +- +- 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; ++ else ++ FAIL_EXIT1 ("subprogram failed with status %d", status); + } + + /* Returns true if a group with NAME has been found, and writes its +@@ -252,7 +192,7 @@ find_sgid_group (gid_t *target, const char *name) + return ok; + } + +-int ++void + support_capture_subprogram_self_sgid (const char *child_id) + { + const int count = 64; +@@ -287,7 +227,7 @@ support_capture_subprogram_self_sgid (const char *child_id) + (intmax_t) getuid ()); + } + +- return copy_and_spawn_sgid (child_id, target); ++ copy_and_spawn_sgid (child_id, target); + } + + void diff --git a/glibc.spec b/glibc.spec index 926450e..b001e92 100644 --- a/glibc.spec +++ b/glibc.spec @@ -157,7 +157,7 @@ end \ Summary: The GNU libc libraries Name: glibc Version: %{glibcversion} -Release: 195%{?dist} +Release: 196%{?dist} # In general, GPLv2+ is used by programs, LGPLv2+ is used for # libraries. @@ -1211,6 +1211,8 @@ Patch903: glibc-RHEL-92697-3.patch Patch904: glibc-RHEL-92697-4.patch Patch905: glibc-RHEL-92697-5.patch Patch906: glibc-RHEL-92697-6.patch +Patch907: glibc-RHEL-92697-8.patch +Patch908: glibc-RHEL-92697-9.patch ############################################################################## # Continued list of core "glibc" package information: @@ -3204,6 +3206,9 @@ update_gconv_modules_cache () %endif %changelog +* Thu May 22 2025 Florian Weimer - 2.34-196 +- SGID test enhancements (RHEL-92697) + * Wed May 21 2025 Florian Weimer - 2.34-195 - CVE-2025-4802: static setuid dlopen may search LD_LIBRARY_PATH (RHEL-92697)