From patchwork Tue Mar 19 10:48:54 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Micka=C3=ABl_Sala=C3=BCn?= X-Patchwork-Id: 1913503 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=pass (2048-bit key; secure) header.d=lists.infradead.org header.i=@lists.infradead.org header.a=rsa-sha256 header.s=bombadil.20210309 header.b=TgW73/u0; dkim=pass (1024-bit key; secure) header.d=digikod.net header.i=@digikod.net header.a=rsa-sha256 header.s=20191114 header.b=a5FId7F9; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=none (no SPF record) smtp.mailfrom=lists.infradead.org (client-ip=2607:7c80:54:3::133; helo=bombadil.infradead.org; envelope-from=linux-um-bounces+incoming=patchwork.ozlabs.org@lists.infradead.org; receiver=patchwork.ozlabs.org) Received: from bombadil.infradead.org (bombadil.infradead.org [IPv6:2607:7c80:54:3::133]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4TzT4d5BGTz1yWs for ; Tue, 19 Mar 2024 21:49:37 +1100 (AEDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:Content-Transfer-Encoding: Content-Type:MIME-Version:References:In-Reply-To:Message-ID:Date:Subject:Cc: To:From:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=pDN6gkH4TkH28ucrXQnaaLs/AKbXbX+vMRPBeDKRbOw=; b=TgW73/u0jWqjTkl3jNoc2JA3e0 uybdHuJ36/4V/Q7u9Ue4tUt/ueiX8Fom4X+N6EwZgQnk3r5vgMPHr8+AHN5BJZz7b69G/8XF6vkbR XITGIqus2mb1xEVxiO96WVu2nl5KuFHIsUkwu1RX033ucQxqpRukqK4Vp3EO8t0PujOvIJ21faZaF LC9Pumx6F5so+rhYxGqw0hbqIkj93Moup1jqXPdZSQYQzkgspKLgSKFyKG1u1WR8Lie1gKJqitv/y qHXxiz+/krGqivhoKbjhJXmRLxupsSLoy/h+o/R6AI1XWmb7oqAm32s8UKSIqIDtBn2achY57Tv/C Hk/EhKHg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1rmX2E-0000000CMe6-1ZaK; Tue, 19 Mar 2024 10:49:34 +0000 Received: from smtp-bc09.mail.infomaniak.ch ([45.157.188.9]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1rmX27-0000000CMWr-1Rhk for linux-um@lists.infradead.org; Tue, 19 Mar 2024 10:49:29 +0000 Received: from smtp-3-0000.mail.infomaniak.ch (smtp-3-0000.mail.infomaniak.ch [10.4.36.107]) by smtp-4-3000.mail.infomaniak.ch (Postfix) with ESMTPS id 4TzT4M01KQzMKj; Tue, 19 Mar 2024 11:49:23 +0100 (CET) Received: from unknown by smtp-3-0000.mail.infomaniak.ch (Postfix) with ESMTPA id 4TzT4L2Dyczr5; Tue, 19 Mar 2024 11:49:22 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=digikod.net; s=20191114; t=1710845362; bh=wqMxCjGb6HQVq+PeRH1/HQhpoMzq4kllkrCAcm/eOKo=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=a5FId7F9PJQqeU2lxNP/NJ8BSyDg5RAkdHp951OjWW57cp80GYgOtqINt2bFG/VzA pXFgs+JgeYRT7H72W1Z1HkYuBAD5SfOGUCvm4ZI51GalA6XHTHDKmO2noqYPKnau73 Tzed9bYlE++aR6s+jZZa55qRRLqdoukJfuWCrlJU= From: =?utf-8?q?Micka=C3=ABl_Sala=C3=BCn?= To: Brendan Higgins , David Gow , Rae Moar , Shuah Khan Cc: =?utf-8?q?Micka=C3=ABl_Sala=C3=BCn?= , Alan Maguire , Borislav Petkov , Dave Hansen , "H . Peter Anvin" , Ingo Molnar , James Morris , Kees Cook , Luis Chamberlain , "Madhavan T . Venkataraman" , Marco Pagani , Paolo Bonzini , Sean Christopherson , Stephen Boyd , Thara Gopinath , Thomas Gleixner , Vitaly Kuznetsov , Wanpeng Li , Zahra Tarkhani , kvm@vger.kernel.org, linux-hardening@vger.kernel.org, linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org, linux-security-module@vger.kernel.org, linux-um@lists.infradead.org, x86@kernel.org Subject: [PATCH v3 4/7] kunit: Handle test faults Date: Tue, 19 Mar 2024 11:48:54 +0100 Message-ID: <20240319104857.70783-5-mic@digikod.net> In-Reply-To: <20240319104857.70783-1-mic@digikod.net> References: <20240319104857.70783-1-mic@digikod.net> MIME-Version: 1.0 X-Infomaniak-Routing: alpha X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240319_034927_718635_C24F4DC5 X-CRM114-Status: GOOD ( 19.61 ) X-Spam-Score: -0.2 (/) X-Spam-Report: Spam detection software, running on the system "bombadil.infradead.org", has NOT identified this incoming email as spam. The original message has been attached to this so you can view it or label similar future email. If you have any questions, see the administrator of that system for details. Content preview: Previously, when a kernel test thread crashed (e.g. NULL pointer dereference, general protection fault), the KUnit test hanged for 30 seconds and exited with a timeout error. Fix this issue by waiting on task_struct->vfork_done instead of the custom kunit_try_catch.try_completion, and track the execution state by initially setting try_result with -EINTR and only setting it [...] Content analysis details: (-0.2 points, 5.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- -0.0 SPF_PASS SPF: sender matches SPF record -0.0 SPF_HELO_PASS SPF: HELO matches SPF record 0.0 RCVD_IN_MSPIKE_H4 RBL: Very Good reputation (+4) [45.157.188.9 listed in wl.mailspike.net] -0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from author's domain 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature -0.1 DKIM_VALID_EF Message has a valid DKIM or DK signature from envelope-from domain 0.0 RCVD_IN_MSPIKE_WL Mailspike good senders -0.0 T_SCC_BODY_TEXT_LINE No description available. X-BeenThere: linux-um@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-um" Errors-To: linux-um-bounces+incoming=patchwork.ozlabs.org@lists.infradead.org Previously, when a kernel test thread crashed (e.g. NULL pointer dereference, general protection fault), the KUnit test hanged for 30 seconds and exited with a timeout error. Fix this issue by waiting on task_struct->vfork_done instead of the custom kunit_try_catch.try_completion, and track the execution state by initially setting try_result with -EINTR and only setting it to 0 if the test passed. Fix kunit_generic_run_threadfn_adapter() signature by returning 0 instead of calling kthread_complete_and_exit(). Because thread's exit code is never checked, always set it to 0 to make it clear. Fix the -EINTR error message, which couldn't be reached until now. This is tested with a following patch. Cc: Brendan Higgins Cc: Shuah Khan Reviewed-by: Kees Cook Reviewed-by: David Gow Tested-by: Rae Moar Signed-off-by: Mickaël Salaün Link: https://lore.kernel.org/r/20240319104857.70783-5-mic@digikod.net --- Changes since v2: * s/-EFAULT/-EINTR/ in commit message as spotted by Rae. * Add a comment explaining vfork_done as suggested by David. * Add David's Reviewed-by. * Add Rae's Tested-by. Changes since v1: * Add Kees's Reviewed-by. --- include/kunit/try-catch.h | 3 --- lib/kunit/try-catch.c | 19 ++++++++++++------- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/include/kunit/try-catch.h b/include/kunit/try-catch.h index c507dd43119d..7c966a1adbd3 100644 --- a/include/kunit/try-catch.h +++ b/include/kunit/try-catch.h @@ -14,13 +14,11 @@ typedef void (*kunit_try_catch_func_t)(void *); -struct completion; struct kunit; /** * struct kunit_try_catch - provides a generic way to run code which might fail. * @test: The test case that is currently being executed. - * @try_completion: Completion that the control thread waits on while test runs. * @try_result: Contains any errno obtained while running test case. * @try: The function, the test case, to attempt to run. * @catch: The function called if @try bails out. @@ -46,7 +44,6 @@ struct kunit; struct kunit_try_catch { /* private: internal use only. */ struct kunit *test; - struct completion *try_completion; int try_result; kunit_try_catch_func_t try; kunit_try_catch_func_t catch; diff --git a/lib/kunit/try-catch.c b/lib/kunit/try-catch.c index cab8b24b5d5a..7a3910dd78a6 100644 --- a/lib/kunit/try-catch.c +++ b/lib/kunit/try-catch.c @@ -18,7 +18,7 @@ void __noreturn kunit_try_catch_throw(struct kunit_try_catch *try_catch) { try_catch->try_result = -EFAULT; - kthread_complete_and_exit(try_catch->try_completion, -EFAULT); + kthread_exit(0); } EXPORT_SYMBOL_GPL(kunit_try_catch_throw); @@ -26,9 +26,12 @@ static int kunit_generic_run_threadfn_adapter(void *data) { struct kunit_try_catch *try_catch = data; + try_catch->try_result = -EINTR; try_catch->try(try_catch->context); + if (try_catch->try_result == -EINTR) + try_catch->try_result = 0; - kthread_complete_and_exit(try_catch->try_completion, 0); + return 0; } static unsigned long kunit_test_timeout(void) @@ -58,13 +61,11 @@ static unsigned long kunit_test_timeout(void) void kunit_try_catch_run(struct kunit_try_catch *try_catch, void *context) { - DECLARE_COMPLETION_ONSTACK(try_completion); struct kunit *test = try_catch->test; struct task_struct *task_struct; int exit_code, time_remaining; try_catch->context = context; - try_catch->try_completion = &try_completion; try_catch->try_result = 0; task_struct = kthread_create(kunit_generic_run_threadfn_adapter, try_catch, "kunit_try_catch_thread"); @@ -75,8 +76,12 @@ void kunit_try_catch_run(struct kunit_try_catch *try_catch, void *context) } get_task_struct(task_struct); wake_up_process(task_struct); - - time_remaining = wait_for_completion_timeout(&try_completion, + /* + * 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. + */ + time_remaining = wait_for_completion_timeout(task_struct->vfork_done, kunit_test_timeout()); if (time_remaining == 0) { try_catch->try_result = -ETIMEDOUT; @@ -92,7 +97,7 @@ void kunit_try_catch_run(struct kunit_try_catch *try_catch, void *context) if (exit_code == -EFAULT) try_catch->try_result = 0; else if (exit_code == -EINTR) - kunit_err(test, "wake_up_process() was never called\n"); + kunit_err(test, "try faulted\n"); else if (exit_code == -ETIMEDOUT) kunit_err(test, "try timed out\n"); else if (exit_code)