Commit 7d4df2da authored by Andrey Konovalov's avatar Andrey Konovalov Committed by Andrew Morton

kcov: properly check for softirq context

When collecting coverage from softirqs, KCOV uses in_serving_softirq() to
check whether the code is running in the softirq context.  Unfortunately,
in_serving_softirq() is > 0 even when the code is running in the hardirq
or NMI context for hardirqs and NMIs that happened during a softirq.

As a result, if a softirq handler contains a remote coverage collection
section and a hardirq with another remote coverage collection section
happens during handling the softirq, KCOV incorrectly detects a nested
softirq coverate collection section and prints a WARNING, as reported by
syzbot.

This issue was exposed by commit a7f3813e ("usb: gadget: dummy_hcd:
Switch to hrtimer transfer scheduler"), which switched dummy_hcd to using
hrtimer and made the timer's callback be executed in the hardirq context.

Change the related checks in KCOV to account for this behavior of
in_serving_softirq() and make KCOV ignore remote coverage collection
sections in the hardirq and NMI contexts.

This prevents the WARNING printed by syzbot but does not fix the inability
of KCOV to collect coverage from the __usb_hcd_giveback_urb when dummy_hcd
is in use (caused by a7f3813e); a separate patch is required for that.

Link: https://lkml.kernel.org/r/20240729022158.92059-1-andrey.konovalov@linux.dev
Fixes: 5ff3b30a ("kcov: collect coverage from interrupts")
Signed-off-by: default avatarAndrey Konovalov <andreyknvl@gmail.com>
Reported-by: syzbot+2388cdaeb6b10f0c13ac@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=2388cdaeb6b10f0c13acAcked-by: default avatarMarco Elver <elver@google.com>
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Aleksandr Nogikh <nogikh@google.com>
Cc: Alexander Potapenko <glider@google.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Marcello Sylvester Bauer <sylv@sylv.io>
Cc: <stable@vger.kernel.org>
Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
parent 37bf7fbe
...@@ -161,6 +161,15 @@ static void kcov_remote_area_put(struct kcov_remote_area *area, ...@@ -161,6 +161,15 @@ static void kcov_remote_area_put(struct kcov_remote_area *area,
kmsan_unpoison_memory(&area->list, sizeof(area->list)); kmsan_unpoison_memory(&area->list, sizeof(area->list));
} }
/*
* Unlike in_serving_softirq(), this function returns false when called during
* a hardirq or an NMI that happened in the softirq context.
*/
static inline bool in_softirq_really(void)
{
return in_serving_softirq() && !in_hardirq() && !in_nmi();
}
static notrace bool check_kcov_mode(enum kcov_mode needed_mode, struct task_struct *t) static notrace bool check_kcov_mode(enum kcov_mode needed_mode, struct task_struct *t)
{ {
unsigned int mode; unsigned int mode;
...@@ -170,7 +179,7 @@ static notrace bool check_kcov_mode(enum kcov_mode needed_mode, struct task_stru ...@@ -170,7 +179,7 @@ static notrace bool check_kcov_mode(enum kcov_mode needed_mode, struct task_stru
* so we ignore code executed in interrupts, unless we are in a remote * so we ignore code executed in interrupts, unless we are in a remote
* coverage collection section in a softirq. * coverage collection section in a softirq.
*/ */
if (!in_task() && !(in_serving_softirq() && t->kcov_softirq)) if (!in_task() && !(in_softirq_really() && t->kcov_softirq))
return false; return false;
mode = READ_ONCE(t->kcov_mode); mode = READ_ONCE(t->kcov_mode);
/* /*
...@@ -849,7 +858,7 @@ void kcov_remote_start(u64 handle) ...@@ -849,7 +858,7 @@ void kcov_remote_start(u64 handle)
if (WARN_ON(!kcov_check_handle(handle, true, true, true))) if (WARN_ON(!kcov_check_handle(handle, true, true, true)))
return; return;
if (!in_task() && !in_serving_softirq()) if (!in_task() && !in_softirq_really())
return; return;
local_lock_irqsave(&kcov_percpu_data.lock, flags); local_lock_irqsave(&kcov_percpu_data.lock, flags);
...@@ -991,7 +1000,7 @@ void kcov_remote_stop(void) ...@@ -991,7 +1000,7 @@ void kcov_remote_stop(void)
int sequence; int sequence;
unsigned long flags; unsigned long flags;
if (!in_task() && !in_serving_softirq()) if (!in_task() && !in_softirq_really())
return; return;
local_lock_irqsave(&kcov_percpu_data.lock, flags); local_lock_irqsave(&kcov_percpu_data.lock, flags);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment