diff mbox series

[v2] acl: improve ptest packaging

Message ID 20250324212446.1730-1-ross.burton@arm.com
State Accepted, archived
Commit 2d82d5ea612ae6d7ac177f2a2792b3e3fdac1c70
Headers show
Series [v2] acl: improve ptest packaging | expand

Commit Message

Ross Burton March 24, 2025, 9:24 p.m. UTC
As there's a small number of  test binaries in acl, instead of
installing large chunks of the build tree we can install just those and
use a boilerplate test runner.

Drop 0001-tests-do-not-hardcode-the-build-path-into-a-helper-l.patch and
replace with an explicit -DBASEDIR= flag passed at build time.

Drop 0001-test-patch-out-failing-bits.patch and delete the tests that
fail entirely as they won't work without a specific user/group setup.

Backport a patch from upstream so that some tests don't use excessive
amounts of memory.

Backport a patch from upstream to cater for both glibc and musl's
behaviour with interleaved stdout/stderr, fixing the tests on musl.

Clean up dependencies now that we're not shipping the build system.

Signed-off-by: Ross Burton <ross.burton@arm.com>
---
 ...t_uid-fix-memory-wasting-loop-if-use.patch | 49 +++++++++++++++
 ...isc.test-Don-t-mix-stdout-and-stderr.patch | 35 +++++++++++
 .../0001-test-patch-out-failing-bits.patch    | 60 ------------------
 ...dcode-the-build-path-into-a-helper-l.patch | 24 -------
 meta/recipes-support/attr/acl/run-ptest       | 37 +++++++----
 meta/recipes-support/attr/acl_2.3.2.bb        | 63 +++++++------------
 6 files changed, 133 insertions(+), 135 deletions(-)
 create mode 100644 meta/recipes-support/attr/acl/0001-libmisc-__acl_get_uid-fix-memory-wasting-loop-if-use.patch
 create mode 100644 meta/recipes-support/attr/acl/0001-test-misc.test-Don-t-mix-stdout-and-stderr.patch
 delete mode 100644 meta/recipes-support/attr/acl/0001-test-patch-out-failing-bits.patch
 delete mode 100644 meta/recipes-support/attr/acl/0001-tests-do-not-hardcode-the-build-path-into-a-helper-l.patch
diff mbox series

Patch

diff --git a/meta/recipes-support/attr/acl/0001-libmisc-__acl_get_uid-fix-memory-wasting-loop-if-use.patch b/meta/recipes-support/attr/acl/0001-libmisc-__acl_get_uid-fix-memory-wasting-loop-if-use.patch
new file mode 100644
index 00000000000..5052bdaa2f1
--- /dev/null
+++ b/meta/recipes-support/attr/acl/0001-libmisc-__acl_get_uid-fix-memory-wasting-loop-if-use.patch
@@ -0,0 +1,49 @@ 
+From 56abe432b65801f31277fb9a3bca0f9e31502315 Mon Sep 17 00:00:00 2001
+From: Matthias Gerstner <matthias.gerstner@suse.de>
+Date: Thu, 25 Apr 2024 12:43:49 +0200
+Subject: [PATCH] libmisc: __acl_get_uid(): fix memory wasting loop if user
+ does not exist
+
+I noticed that `acl_from_text()` unexpectedly returns ENOMEM for invalid
+user names. The reason for this is a missing break statement in the for
+loop in `__acl_get_uid()`, which causes the loop to act as if ERANGE was
+returned from `getpwnam_r()`, thereby exponentially increasing the
+buffer size to (in my case) multiple gigabytes, until `grow_buffer()`
+reports ENOMEM, which terminates the `__acl_get_uid()` function.
+
+This is a pretty costly "no such user" lookup that can disturb a
+process's heap memory management, but can also cause a process to fail
+e.g. if it is multithreaded and other threads encounter an ENOMEM,
+before `__acl_get_uid()` frees the gigantic heap buffer and returns.
+The allocated memory isn't actually used. Therefore on Linux it should
+not affect other processes by default, due to its overcommit memory
+and lazy memory allocation strategy.
+
+Fix this by properly terminating the for loop on any conditions except
+an ERANGE error being reported. The same break statement correctly
+exists in `__acl_get_gid()` already.
+
+Fixes: 3737f00 ("use thread-safe getpwnam_r and getgrnam_r")
+Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
+
+Upstream-Status: Backport
+Signed-off-by: Ross Burton <ross.burton@arm.com>
+---
+ libmisc/uid_gid_lookup.c | 1 +
+ 1 file changed, 1 insertion(+)
+
+diff --git a/libmisc/uid_gid_lookup.c b/libmisc/uid_gid_lookup.c
+index a4f21f6..74baab4 100644
+--- a/libmisc/uid_gid_lookup.c
++++ b/libmisc/uid_gid_lookup.c
+@@ -91,6 +91,7 @@ __acl_get_uid(const char *token, uid_t *uid_p)
+ 		if (err == ERANGE)
+ 			continue;
+ 		errno = err ? err : EINVAL;
++		break;
+ 	}
+ 	free(buffer);
+ 	return result ? 0 : -1;
+-- 
+2.43.0
+
diff --git a/meta/recipes-support/attr/acl/0001-test-misc.test-Don-t-mix-stdout-and-stderr.patch b/meta/recipes-support/attr/acl/0001-test-misc.test-Don-t-mix-stdout-and-stderr.patch
new file mode 100644
index 00000000000..5aa3f3224cb
--- /dev/null
+++ b/meta/recipes-support/attr/acl/0001-test-misc.test-Don-t-mix-stdout-and-stderr.patch
@@ -0,0 +1,35 @@ 
+From 47f8039ec9bd08b629775c8e788d11e41fa95f14 Mon Sep 17 00:00:00 2001
+From: Andreas Gruenbacher <agruenba@redhat.com>
+Date: Mon, 24 Mar 2025 21:14:09 +0100
+Subject: [PATCH] test/misc.test: Don't mix stdout and stderr
+
+In different environments, we may not get the stdout and stderr output
+in the order the run script expects, so check both separately.
+
+Fixes: https://savannah.nongnu.org/bugs/?66944
+Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
+
+Upstream-Status: Backport
+Signed-off-by: Ross Burton <ross.burton@arm.com>
+---
+ test/misc.test | 3 ++-
+ 1 file changed, 2 insertions(+), 1 deletion(-)
+
+diff --git a/test/misc.test b/test/misc.test
+index 06b3136..57c02e5 100644
+--- a/test/misc.test
++++ b/test/misc.test
+@@ -440,8 +440,9 @@ Dangling symlink test https://savannah.nongnu.org/bugs/?28131
+ 	> other::r-x
+ 	> 
+ 	$ setfacl -R -m u:bin:rw d
+-	$ getfacl -RL d
++	$ getfacl -RL d > /dev/null
+ 	> getfacl: d/b: No such file or directory
++	$ getfacl -RL d 2> /dev/null
+ 	> # file: d
+ 	> # owner: %TUSER
+ 	> # group: %TGROUP
+-- 
+2.43.0
+
diff --git a/meta/recipes-support/attr/acl/0001-test-patch-out-failing-bits.patch b/meta/recipes-support/attr/acl/0001-test-patch-out-failing-bits.patch
deleted file mode 100644
index 219feaccd07..00000000000
--- a/meta/recipes-support/attr/acl/0001-test-patch-out-failing-bits.patch
+++ /dev/null
@@ -1,60 +0,0 @@ 
-From 7dec6fa3b3494a55120402ff1ea3eb96b67138e8 Mon Sep 17 00:00:00 2001
-From: Alexander Kanavin <alex.kanavin@gmail.com>
-Date: Thu, 12 Dec 2019 15:47:49 +0100
-Subject: [PATCH] test: patch out failing bits
-
-I have confirmed on the host distro (Ubuntu 18.04) that they
-fail as well; upstream probably haven't noticed because the
-test is only executed under sudo.
-
-Upstream-Status: Inappropriate [disabling tests instead of fixing them properly]
-Signed-off-by: Alexander Kanavin <alex.kanavin@gmail.com>
----
- test/root/permissions.test | 13 -------------
- 1 file changed, 13 deletions(-)
-
-diff --git a/test/root/permissions.test b/test/root/permissions.test
-index 8f8f825..21e8a95 100644
---- a/test/root/permissions.test
-+++ b/test/root/permissions.test
-@@ -50,10 +50,6 @@ User daemon is a member in the owning group, which has only read access.
- Verify this.
- 
- 	$ su daemon
--	$ cat f
--	> root
--	> bin
--
- 	$ echo daemon >> f
- 	>~ .*f: Permission denied$
- 
-@@ -146,8 +142,6 @@ the owning group, he should still have no write access.
- 	$ setfacl -x g:daemon f
- 
- 	$ su daemon
--	$ echo daemon4 >> f
--	>~ .*f: Permission denied$
- 
- 
- Change the owning group. The other permissions should now grant user
-@@ -158,12 +152,6 @@ daemon write access.
- 
- 	$ su daemon
- 	$ echo daemon5 >> f
--	$ cat f
--	> root
--	> bin
--	> daemon
--	> daemon2
--	> daemon5
- 
- 
- Verify that permissions in separate matching ACL entries do not
-@@ -173,7 +161,6 @@ accumulate.
- 	$ setfacl -m g:bin:r,g:daemon:w f
- 
- 	$ su daemon
--	$ : < f
- 	$ : > f
- 	$ : <> f
- 	>~ .*f: Permission denied$
diff --git a/meta/recipes-support/attr/acl/0001-tests-do-not-hardcode-the-build-path-into-a-helper-l.patch b/meta/recipes-support/attr/acl/0001-tests-do-not-hardcode-the-build-path-into-a-helper-l.patch
deleted file mode 100644
index 748f37f3e7e..00000000000
--- a/meta/recipes-support/attr/acl/0001-tests-do-not-hardcode-the-build-path-into-a-helper-l.patch
+++ /dev/null
@@ -1,24 +0,0 @@ 
-From 42ae3f8a5e32ba0681ccd1552a203ddad8748a6e Mon Sep 17 00:00:00 2001
-From: Alexander Kanavin <alex.kanavin@gmail.com>
-Date: Thu, 12 Dec 2019 13:45:52 +0100
-Subject: [PATCH] tests: do not hardcode the build path into a helper library
-
-Upstream-Status: Inappropriate [oe-core specific]
-Signed-off-by: Alexander Kanavin <alex.kanavin@gmail.com>
----
- test/Makemodule.am | 2 +-
- 1 file changed, 1 insertion(+), 1 deletion(-)
-
-diff --git a/test/Makemodule.am b/test/Makemodule.am
-index e1d715d..cffe732 100644
---- a/test/Makemodule.am
-+++ b/test/Makemodule.am
-@@ -30,7 +30,7 @@ EXTRA_DIST += \
- check_LTLIBRARIES = libtestlookup.la
- 
- libtestlookup_la_SOURCES = test/test_passwd.c test/test_group.c
--libtestlookup_la_CFLAGS = -DBASEDIR=\"$(abs_srcdir)\"
-+libtestlookup_la_CFLAGS = -DBASEDIR=\"/tmp/acl-ptest\"
- libtestlookup_la_LDFLAGS = -rpath $(abs_builddir)
- 
- # Make sure translations don't break tests when matching output.
diff --git a/meta/recipes-support/attr/acl/run-ptest b/meta/recipes-support/attr/acl/run-ptest
index 3af75c84fea..f28d8c12122 100644
--- a/meta/recipes-support/attr/acl/run-ptest
+++ b/meta/recipes-support/attr/acl/run-ptest
@@ -1,16 +1,31 @@ 
 #!/bin/sh
-#
-#This script is used to run acl test suites
 
-#umask 077
+failed=0
+all=0
 
-mkdir -p /tmp/acl-ptest/test
-cp test/test.* /tmp/acl-ptest/test
 
-set +e
-make test-suite.log
-exitcode=$?
-if [ $exitcode -ne 0 -a -e test-suite.log ]; then
-    cat test-suite.log
+for f in *.test; do
+    LD_PRELOAD=$(pwd)/libtestlookup.so ./run $f
+    case "$?" in
+        0)
+            echo "PASS: $f"
+            all=$((all + 1))
+            ;;
+        77)
+            echo "SKIP: $f"
+            ;;
+        *)
+            echo "FAIL: $f"
+            failed=$((failed + 1))
+            all=$((all + 1))
+            ;;
+    esac
+done
+
+if [ "$failed" -eq 0 ] ; then
+    echo "All $all tests passed"
+    exit 0
+else
+    echo "$failed of $all tests failed"
+    exit 1
 fi
-exit $exitcode
diff --git a/meta/recipes-support/attr/acl_2.3.2.bb b/meta/recipes-support/attr/acl_2.3.2.bb
index dd959a5051b..a405cc2692e 100644
--- a/meta/recipes-support/attr/acl_2.3.2.bb
+++ b/meta/recipes-support/attr/acl_2.3.2.bb
@@ -16,11 +16,10 @@  LIC_FILES_CHKSUM = "file://doc/COPYING;md5=c781d70ed2b4d48995b790403217a249 \
 DEPENDS = "attr"
 
 SRC_URI = "${SAVANNAH_GNU_MIRROR}/acl/${BP}.tar.gz \
+           file://0001-libmisc-__acl_get_uid-fix-memory-wasting-loop-if-use.patch \
+           file://0001-test-misc.test-Don-t-mix-stdout-and-stderr.patch \
            file://run-ptest \
-           file://0001-tests-do-not-hardcode-the-build-path-into-a-helper-l.patch \
-           file://0001-test-patch-out-failing-bits.patch \
            "
-
 SRC_URI[sha256sum] = "5f2bdbad629707aa7d85c623f994aa8a1d2dec55a73de5205bac0bf6058a2f7c"
 
 inherit autotools gettext ptest
@@ -31,57 +30,41 @@  PACKAGES =+ "lib${BPN}"
 
 FILES:lib${BPN} = "${libdir}/lib*${SOLIBS}"
 
-PTEST_BUILD_HOST_FILES = "builddefs"
-PTEST_BUILD_HOST_PATTERN = "^RPM"
-
 do_compile_ptest() {
-        oe_runmake libtestlookup.la
+        oe_runmake libtestlookup.la libtestlookup_la_CFLAGS=-DBASEDIR=\\\"${PTEST_PATH}\\\"
 }
 
 do_install_ptest() {
-	cp -rf ${S}/test/ ${D}${PTEST_PATH}
-	cp -rf ${S}/build-aux/ ${D}${PTEST_PATH}
-        mkdir -p ${D}${PTEST_PATH}/.libs
-	cp -rf ${B}/.libs/libtestlookup* ${D}${PTEST_PATH}/.libs
-        cp ${B}/Makefile ${D}${PTEST_PATH}
+        install -m755 ${S}/test/run ${S}/test/sort-getfacl-output ${D}${PTEST_PATH}/
+        install -m644 ${S}/test/*.acl ${D}${PTEST_PATH}/
 
-        sed -e 's,--sysroot=${STAGING_DIR_TARGET},,g' \
-            -e 's|${DEBUG_PREFIX_MAP}||g' \
-            -e 's:${HOSTTOOLS_DIR}/::g' \
-            -e 's:${RECIPE_SYSROOT_NATIVE}::g' \
-            -e 's:${BASE_WORKDIR}/${MULTIMACH_TARGET_SYS}::g' \
-            -i ${D}${PTEST_PATH}/Makefile
+        # Install the tests
+        for t in $(makefile-getvar ${S}/test/Makemodule.am TESTS); do
+                install -m644 ${S}/$t ${D}${PTEST_PATH}/
+        done
+        # Remove the tests that are expected to fail (they need a NFS server configured)
+        for t in $(makefile-getvar ${S}/test/Makemodule.am XFAIL_TESTS); do
+                rm ${D}${PTEST_PATH}/$(basename $t)
+        done
+        # These tests need a very specific user/group setup
+        # (https://savannah.nongnu.org/bugs/index.php?66927)
+        rm -f ${D}${PTEST_PATH}/getfacl.test ${D}${PTEST_PATH}/permissions.test ${D}${PTEST_PATH}/restore.test ${D}${PTEST_PATH}/setfacl.test
 
-        sed -e "s|^srcdir =.*|srcdir = .|" \
-	    -e "s|^abs_srcdir =.*|abs_srcdir = .|" \
-	    -e "s|^abs_top_srcdir =.*|abs_top_srcdir = ..|" \
-	    -e "s|^Makefile:.*|Makefile:|" \
-	    -e "/^TEST_LOG_DRIVER =/s|(top_srcdir)|(top_builddir)|" \
-	    -i ${D}${PTEST_PATH}/Makefile
-
-        rm ${D}${PTEST_PATH}/.libs/libtestlookup.lai
-}
-
-do_install_ptest:append:libc-musl() {
-        sed -i -e '/test\/misc.test/d' ${D}${PTEST_PATH}/Makefile
+        ${B}/libtool --mode=install install ${B}/libtestlookup.la ${D}${PTEST_PATH}/
+        rm -f ${D}${PTEST_PATH}/*.la
+        install -d ${D}${PTEST_PATH}/test
+        install -m644 ${S}/test/test.passwd ${S}/test/test.group ${D}${PTEST_PATH}/test
 }
 
 RDEPENDS:${PN}-ptest = "acl \
-                        bash \
                         coreutils \
                         perl \
-                        perl-module-constant \
-                        perl-module-filehandle \
-                        perl-module-getopt-std \
-                        perl-module-posix \
-                        shadow \
-                        make \
-                        gawk \
-                        e2fsprogs-mke2fs \
                         perl-module-cwd \
                         perl-module-file-basename \
                         perl-module-file-path \
-                        perl-module-file-spec \
+                        perl-module-filehandle \
+                        perl-module-getopt-std \
+                        perl-module-posix \
                        "
 
 BBCLASSEXTEND = "native nativesdk"