Testing the whole series, I found that some Landlock tests are flaky starting with this patch. I tried to not use the longjmp in the grandchild but it didn't change. I suspect missing volatiles but I didn't find the faulty one(s) yet. :/ I'll continue investigating tomorrow but help would be much appreciated!
On Wed, Feb 28, 2024 at 04:59:09PM -0800, Jakub Kicinski wrote:
From: Mickaël Salaün mic@digikod.net
Replace Landlock-specific TEST_F_FORK() with an improved TEST_F() which brings four related changes:
Run TEST_F()'s tests in a grandchild process to make it possible to drop privileges and delegate teardown to the parent.
Compared to TEST_F_FORK(), simplify handling of the test grandchild process thanks to vfork(2), and makes it generic (e.g. no explicit conversion between exit code and _metadata).
Compared to TEST_F_FORK(), run teardown even when tests failed with an assert thanks to commit 63e6b2a42342 ("selftests/harness: Run TEARDOWN for ASSERT failures").
Simplify the test harness code by removing the no_print and step fields which are not used. I added this feature just after I made kselftest_harness.h more broadly available but this step counter remained even though it wasn't needed after all. See commit 369130b63178 ("selftests: Enhance kselftest_harness.h to print which assert failed").
Replace spaces with tabs in one line of __TEST_F_IMPL().
Cc: Günther Noack gnoack@google.com Cc: Shuah Khan shuah@kernel.org Cc: Will Drewry wad@chromium.org Signed-off-by: Mickaël Salaün mic@digikod.net Reviewed-by: Kees Cook keescook@chromium.org Signed-off-by: Jakub Kicinski kuba@kernel.org -- v4:
- GAND -> GRAND
- init child to 1, otherwise assert in setup triggers a longjmp which in turn reads child without it ever getting initialized (or being 0, i.e. we mistakenly assume we're in the grandchild)
Good catch!