From: David Gow <davidgow@google.com>
To: "Rae Moar" <rmoar@google.com>,
"Kees Cook" <keescook@chromium.org>,
"Mickaël Salaün" <mic@digikod.net>,
"Naresh Kamboju" <naresh.kamboju@linaro.org>,
"Shuah Khan" <skhan@linuxfoundation.org>
Cc: David Gow <davidgow@google.com>, Will Deacon <will@kernel.org>,
Dan Carpenter <dan.carpenter@linaro.org>,
Guenter Roeck <linux@roeck-us.net>,
linux-kselftest@vger.kernel.org, kunit-dev@googlegroups.com,
linux-kernel@vger.kernel.org,
Brendan Higgins <brendan.higgins@linux.dev>,
Linux Kernel Functional Testing <lkft@linaro.org>
Subject: [PATCH] kunit: Fix race condition in try-catch completion
Date: Fri, 12 Apr 2024 10:59:01 +0800 [thread overview]
Message-ID: <20240412025903.489470-1-davidgow@google.com> (raw)
KUnit's try-catch infrastructure now uses vfork_done, which is always
set to a valid completion when a kthread is created, but which is set to
NULL once the thread terminates. This creates a race condition, where
the kthread exits before we can wait on it.
Keep a copy of vfork_done, which is taken before we wake_up_process()
and so valid, and wait on that instead.
Fixes: 4de2a8e4cca4 ("kunit: Handle test faults")
Reported-by: Linux Kernel Functional Testing <lkft@linaro.org>
Closes: https://lore.kernel.org/lkml/20240410102710.35911-1-naresh.kamboju@linaro.org/
Tested-by: Linux Kernel Functional Testing <lkft@linaro.org>
Acked-by: Mickaël Salaün <mic@digikod.net>
Signed-off-by: David Gow <davidgow@google.com>
---
lib/kunit/try-catch.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/lib/kunit/try-catch.c b/lib/kunit/try-catch.c
index fa687278ccc9..6bbe0025b079 100644
--- a/lib/kunit/try-catch.c
+++ b/lib/kunit/try-catch.c
@@ -63,6 +63,7 @@ void kunit_try_catch_run(struct kunit_try_catch *try_catch, void *context)
{
struct kunit *test = try_catch->test;
struct task_struct *task_struct;
+ struct completion *task_done;
int exit_code, time_remaining;
try_catch->context = context;
@@ -75,13 +76,16 @@ void kunit_try_catch_run(struct kunit_try_catch *try_catch, void *context)
return;
}
get_task_struct(task_struct);
- wake_up_process(task_struct);
/*
* As for a vfork(2), task_struct->vfork_done (pointing to the
* underlying kthread->exited) can be used to wait for the end of a
- * kernel thread.
+ * kernel thread. It is set to NULL when the thread exits, so we
+ * keep a copy here.
*/
- time_remaining = wait_for_completion_timeout(task_struct->vfork_done,
+ task_done = task_struct->vfork_done;
+ wake_up_process(task_struct);
+
+ time_remaining = wait_for_completion_timeout(task_done,
kunit_test_timeout());
if (time_remaining == 0) {
try_catch->try_result = -ETIMEDOUT;
--
2.44.0.683.g7961c838ac-goog
next reply other threads:[~2024-04-12 2:59 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-12 2:59 David Gow [this message]
2024-04-12 21:32 ` [PATCH] kunit: Fix race condition in try-catch completion Rae Moar
2024-04-13 21:04 ` Miguel Ojeda
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240412025903.489470-1-davidgow@google.com \
--to=davidgow@google.com \
--cc=brendan.higgins@linux.dev \
--cc=dan.carpenter@linaro.org \
--cc=keescook@chromium.org \
--cc=kunit-dev@googlegroups.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=linux@roeck-us.net \
--cc=lkft@linaro.org \
--cc=mic@digikod.net \
--cc=naresh.kamboju@linaro.org \
--cc=rmoar@google.com \
--cc=skhan@linuxfoundation.org \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox