| Message ID | 20240426172252.1862930-4-mic@digikod.net (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | fff37bd32c7605d93bf900c4c318d56d12000048 |
| Headers | show |
| Series | Fix Kselftest's vfork() side effects | expand |
On Fri, Apr 26, 2024 at 07:22:50PM +0200, Mickaël Salaün wrote: > Make sure fixture teardowns are run when test cases failed, including > when _metadata->teardown_parent is set to true. > > Make sure only one fixture teardown is run per test case, handling the > case where the test child forks. I had to go look up __sync_bool_compare_and_swap(). :) > > Cc: Shuah Khan <skhan@linuxfoundation.org> > Cc: Shengyu Li <shengyu.li.evgeny@gmail.com> > Fixes: 72d7cb5c190b ("selftests/harness: Prevent infinite loop due to Assert in FIXTURE_TEARDOWN") > Fixes: 0710a1a73fb4 ("selftests/harness: Merge TEST_F_FORK() into TEST_F()") > Signed-off-by: Mickaël Salaün <mic@digikod.net> > Link: https://lore.kernel.org/r/20240426172252.1862930-4-mic@digikod.net Reviewed-by: Kees Cook <keescook@chromium.org> -Kees > --- > tools/testing/selftests/kselftest_harness.h | 14 +++++++++----- > 1 file changed, 9 insertions(+), 5 deletions(-) > > diff --git a/tools/testing/selftests/kselftest_harness.h b/tools/testing/selftests/kselftest_harness.h > index ba3ddeda24bf..73491efbae9e 100644 > --- a/tools/testing/selftests/kselftest_harness.h > +++ b/tools/testing/selftests/kselftest_harness.h > @@ -383,7 +383,10 @@ > FIXTURE_DATA(fixture_name) self; \ > pid_t child = 1; \ > int status = 0; \ > - bool jmp = false; \ > + /* Makes sure there is only one teardown, even when child forks again. */ \ > + bool *teardown = mmap(NULL, sizeof(*teardown), \ > + PROT_READ | PROT_WRITE, MAP_SHARED | MAP_ANONYMOUS, -1, 0); \ > + *teardown = false; \ > memset(&self, 0, sizeof(FIXTURE_DATA(fixture_name))); \ > if (setjmp(_metadata->env) == 0) { \ > /* Use the same _metadata. */ \ > @@ -400,15 +403,16 @@ > _metadata->exit_code = KSFT_FAIL; \ > } \ > } \ > - else \ > - jmp = true; \ > if (child == 0) { \ > - if (_metadata->setup_completed && !_metadata->teardown_parent && !jmp) \ > + if (_metadata->setup_completed && !_metadata->teardown_parent && \ > + __sync_bool_compare_and_swap(teardown, false, true)) \ > fixture_name##_teardown(_metadata, &self, variant->data); \ > _exit(0); \ > } \ > - if (_metadata->setup_completed && _metadata->teardown_parent) \ > + if (_metadata->setup_completed && _metadata->teardown_parent && \ > + __sync_bool_compare_and_swap(teardown, false, true)) \ > fixture_name##_teardown(_metadata, &self, variant->data); \ > + munmap(teardown, sizeof(*teardown)); \ > if (!WIFEXITED(status) && WIFSIGNALED(status)) \ > /* Forward signal to __wait_for_test(). */ \ > kill(getpid(), WTERMSIG(status)); \ > -- > 2.44.0 >
diff --git a/tools/testing/selftests/kselftest_harness.h b/tools/testing/selftests/kselftest_harness.h index ba3ddeda24bf..73491efbae9e 100644 --- a/tools/testing/selftests/kselftest_harness.h +++ b/tools/testing/selftests/kselftest_harness.h @@ -383,7 +383,10 @@ FIXTURE_DATA(fixture_name) self; \ pid_t child = 1; \ int status = 0; \ - bool jmp = false; \ + /* Makes sure there is only one teardown, even when child forks again. */ \ + bool *teardown = mmap(NULL, sizeof(*teardown), \ + PROT_READ | PROT_WRITE, MAP_SHARED | MAP_ANONYMOUS, -1, 0); \ + *teardown = false; \ memset(&self, 0, sizeof(FIXTURE_DATA(fixture_name))); \ if (setjmp(_metadata->env) == 0) { \ /* Use the same _metadata. */ \ @@ -400,15 +403,16 @@ _metadata->exit_code = KSFT_FAIL; \ } \ } \ - else \ - jmp = true; \ if (child == 0) { \ - if (_metadata->setup_completed && !_metadata->teardown_parent && !jmp) \ + if (_metadata->setup_completed && !_metadata->teardown_parent && \ + __sync_bool_compare_and_swap(teardown, false, true)) \ fixture_name##_teardown(_metadata, &self, variant->data); \ _exit(0); \ } \ - if (_metadata->setup_completed && _metadata->teardown_parent) \ + if (_metadata->setup_completed && _metadata->teardown_parent && \ + __sync_bool_compare_and_swap(teardown, false, true)) \ fixture_name##_teardown(_metadata, &self, variant->data); \ + munmap(teardown, sizeof(*teardown)); \ if (!WIFEXITED(status) && WIFSIGNALED(status)) \ /* Forward signal to __wait_for_test(). */ \ kill(getpid(), WTERMSIG(status)); \
Make sure fixture teardowns are run when test cases failed, including when _metadata->teardown_parent is set to true. Make sure only one fixture teardown is run per test case, handling the case where the test child forks. Cc: Shuah Khan <skhan@linuxfoundation.org> Cc: Shengyu Li <shengyu.li.evgeny@gmail.com> Fixes: 72d7cb5c190b ("selftests/harness: Prevent infinite loop due to Assert in FIXTURE_TEARDOWN") Fixes: 0710a1a73fb4 ("selftests/harness: Merge TEST_F_FORK() into TEST_F()") Signed-off-by: Mickaël Salaün <mic@digikod.net> Link: https://lore.kernel.org/r/20240426172252.1862930-4-mic@digikod.net --- tools/testing/selftests/kselftest_harness.h | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)