* [PATCH (draft)] kcov: fix potential kcov_mode corruption under CONFIG_PREEMPT_RT
@ 2026-05-04 15:30 Tetsuo Handa
2026-05-04 15:39 ` Tetsuo Handa
2026-05-05 8:15 ` [PATCH] " Tetsuo Handa
0 siblings, 2 replies; 29+ messages in thread
From: Tetsuo Handa @ 2026-05-04 15:30 UTC (permalink / raw)
To: kasan-dev, LKML; +Cc: Dmitry Vyukov, Alexander Potapenko
Problem Description:
syzbot has reported intermittent kcov_mode corruption when running with
CONFIG_PREEMPT_RT=y. Specifically, struct task_struct->kcov_mode is found
to be unexpectedly modified (e.g., changed from 0 to 2) during workqueue
execution, even when the work handler itself does not use KCOV [1].
Root Cause Analysis:
The issue stems from the ambiguity of in_task() under CONFIG_PREEMPT_RT.
Since commit 5ff3b30ab57d ("kcov: collect coverage from interrupts"),
kcov_remote_start() uses in_task() to decide whether to use a per-CPU
preallocated buffer (irq_area) or a task-specific buffer (allocated via
vmalloc).
In a CONFIG_PREEMPT_RT kernel, in_task() returns true even for threaded
softirqs (e.g., ksoftirqd or threaded IRQ handlers), unless they are
explicitly within a local_bh_disable() region at the time of the check.
This leads to several critical issues:
1. Context Mismatch: A threaded interrupt handler (like usb_giveback_urb_bh)
might be identified as a "task" context. If it starts a remote KCOV
session, it incorrectly modifies the kworker's task_struct->kcov_mode
using the logic intended for system calls.
2. State Leakage: If the result of in_task() differs between
kcov_remote_start() and kcov_remote_stop() due to the subtle state
changes in RT scheduling, the cleanup code (kcov_stop()) might be skipped
or applied to the wrong context. This leaves kcov_mode set to a non-zero
value (e.g., KCOV_MODE_TRACE_CMP) when the worker returns to
process_one_work().
3. Evidence: Debug logs show the pr_err from process_one_work()
carrying a [ C1] (CPU-based) prefix instead of a [Txxx] (Task-based)
prefix. This confirms that at the moment of the error,
in_serving_softirq() was true, causing in_task() to be false, which
contradicts the state when the work started.
Proposed Fix:
Introduce a more strict context check, in_task_really(), which
explicitly excludes any softirq context (including threaded ones) by
checking in_serving_softirq() regardless of the CONFIG_PREEMPT_RT
configuration. This ensures that threaded interrupts always use the
irq_area path and do not corrupt the task_struct state of the worker
threads they happen to preempt.
I wrote this patch's body, but patch description above was generated
using Google AI mode. I can't tell whether above explanation is correct.
Therefore, please review but don't accept yet. For now:
Link: https://syzkaller.appspot.com/bug?extid=a21650c1666eae7b2aae [1]
Analyzed-by: https://share.google/aimode/5aL6wtYxiS3koWnAx (expires in 7 days)
Not-yet-signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Fixes: 5ff3b30ab57d ("kcov: collect coverage from interrupts")
Reported-by: syzbot+3f51ad7ac3ae57a6fdcc@syzkaller.appspotmail.com
Maybe-closes: https://syzkaller.appspot.com/bug?extid=3f51ad7ac3ae57a6fdcc
---
kernel/kcov.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/kernel/kcov.c b/kernel/kcov.c
index 0b369e88c7c9..c20d3c2712d7 100644
--- a/kernel/kcov.c
+++ b/kernel/kcov.c
@@ -171,6 +171,15 @@ static __always_inline bool in_softirq_really(void)
return in_serving_softirq() && !in_hardirq() && !in_nmi();
}
+static __always_inline bool in_task_really(void)
+{
+#ifdef CONFIG_PREEMPT_RT
+ return !in_serving_softirq() && !in_hardirq() && !in_nmi();
+#else
+ return in_task();
+#endif
+}
+
static notrace bool check_kcov_mode(enum kcov_mode needed_mode, struct task_struct *t)
{
unsigned int mode;
@@ -903,7 +912,7 @@ void kcov_remote_start(u64 handle)
return;
}
kcov_debug("handle = %llx, context: %s\n", handle,
- in_task() ? "task" : "softirq");
+ in_task_really() ? "task" : "softirq");
kcov = remote->kcov;
/* Put in kcov_remote_stop(). */
kcov_get(kcov);
@@ -915,7 +924,7 @@ void kcov_remote_start(u64 handle)
*/
mode = context_unsafe(kcov->mode);
sequence = kcov->sequence;
- if (in_task()) {
+ if (in_task_really()) {
size = kcov->remote_size;
area = kcov_remote_area_get(size);
} else {
@@ -924,7 +933,7 @@ void kcov_remote_start(u64 handle)
}
spin_unlock(&kcov_remote_lock);
- /* Can only happen when in_task(). */
+ /* Can only happen when in_task_really(). */
if (!area) {
local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
area = vmalloc(size * sizeof(unsigned long));
@@ -1069,7 +1078,7 @@ void kcov_remote_stop(void)
kcov_move_area(kcov->mode, kcov->area, kcov->size, area);
spin_unlock(&kcov->lock);
- if (in_task()) {
+ if (in_task_really()) {
spin_lock(&kcov_remote_lock);
kcov_remote_area_put(area, size);
spin_unlock(&kcov_remote_lock);
--
2.47.3
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH (draft)] kcov: fix potential kcov_mode corruption under CONFIG_PREEMPT_RT
2026-05-04 15:30 [PATCH (draft)] kcov: fix potential kcov_mode corruption under CONFIG_PREEMPT_RT Tetsuo Handa
@ 2026-05-04 15:39 ` Tetsuo Handa
2026-05-05 8:15 ` [PATCH] " Tetsuo Handa
1 sibling, 0 replies; 29+ messages in thread
From: Tetsuo Handa @ 2026-05-04 15:39 UTC (permalink / raw)
To: kasan-dev, LKML; +Cc: Dmitry Vyukov, Alexander Potapenko
On 2026/05/05 0:30, Tetsuo Handa wrote:
> Link: https://syzkaller.appspot.com/bug?extid=a21650c1666eae7b2aae [1]
Oops. Link was wrong.
Link: https://syzkaller.appspot.com/bug?extid=e6686317bd9fe911591a [1]
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH] kcov: fix potential kcov_mode corruption under CONFIG_PREEMPT_RT
2026-05-04 15:30 [PATCH (draft)] kcov: fix potential kcov_mode corruption under CONFIG_PREEMPT_RT Tetsuo Handa
2026-05-04 15:39 ` Tetsuo Handa
@ 2026-05-05 8:15 ` Tetsuo Handa
2026-05-06 11:55 ` [PATCH v2] kcov: fix state corruption under CONFIG_PREEMPT_RT by eliminating per-cpu data Tetsuo Handa
1 sibling, 1 reply; 29+ messages in thread
From: Tetsuo Handa @ 2026-05-05 8:15 UTC (permalink / raw)
To: kasan-dev, LKML; +Cc: Dmitry Vyukov, Alexander Potapenko
Problem Description:
syzbot has reported intermittent kcov_mode corruption when running with
CONFIG_PREEMPT_RT=y. Specifically, struct task_struct->kcov_mode is found
to be unexpectedly modified (e.g., changed from 0 to 2) during workqueue
execution, even when the work handler itself does not use KCOV.
[ 224.426704][ T5698] BUG: workqueue function defense_work_handler changed kcov_mode from 0 to 2
[ 224.441977][ C1] BUG: workqueue function usb_giveback_urb_bh changed kcov_mode from 2 to 0
Root Cause Analysis:
The issue stems from the ambiguity of in_task() under CONFIG_PREEMPT_RT.
Since commit 5ff3b30ab57d ("kcov: collect coverage from interrupts"),
kcov_remote_start() uses in_task() to decide whether to use a per-CPU
preallocated buffer (irq_area) or a task-specific buffer (allocated via
vmalloc).
In a CONFIG_PREEMPT_RT kernel, in_task() returns true even for threaded
softirqs (e.g., ksoftirqd or threaded IRQ handlers), unless they are
explicitly within a local_bh_disable() region at the time of the check.
This leads to several critical issues:
1. Context Mismatch: A threaded interrupt handler (like
usb_giveback_urb_bh()) might be identified as a "task" context. If it
starts a remote KCOV session, it incorrectly modifies the kworker's
task_struct->kcov_mode using the logic intended for system calls.
2. State Leakage: If the result of in_task() differs between
kcov_remote_start() and kcov_remote_stop() due to the subtle state
changes in RT scheduling, the cleanup code (kcov_stop()) might be
skipped or applied to the wrong context. This leaves kcov_mode set to
a non-zero value (e.g., KCOV_MODE_TRACE_CMP) when the worker returns
to process_one_work().
3. Evidence: The message quoted in Problem Description showed the
pr_err() from process_one_work() carrying a [ C1] (CPU-based) prefix
instead of a [Txxx] (Task-based) prefix. This confirms that at the
moment of the error, in_serving_softirq() was true, causing in_task()
to be false, which contradicts the state when the work started.
Proposed Fix:
Introduce a more strict context check, in_task_really(), which
explicitly excludes any softirq context (including threaded ones) by
checking in_serving_softirq() regardless of the CONFIG_PREEMPT_RT
configuration. This ensures that threaded interrupts always use the
irq_area path and do not corrupt the task_struct state of the worker
threads they happen to preempt.
Also, replace !in_task() && !in_serving_softirq() test in
kcov_remote_{start,stop}() with in_hardirq() || in_nmi() test, for
relying on unstable in_serving_softirq() test might break symmetry of
kcov_remote_{start,stop}() calls.
Link: https://syzkaller.appspot.com/bug?extid=e6686317bd9fe911591a
Analyzed-by: Google AI mode (no mail address)
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Fixes: 5ff3b30ab57d ("kcov: collect coverage from interrupts")
---
kernel/kcov.c | 26 +++++++++++++++++++-------
1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/kernel/kcov.c b/kernel/kcov.c
index 0b369e88c7c9..f77bcd507701 100644
--- a/kernel/kcov.c
+++ b/kernel/kcov.c
@@ -171,6 +171,16 @@ static __always_inline bool in_softirq_really(void)
return in_serving_softirq() && !in_hardirq() && !in_nmi();
}
+static __always_inline bool in_task_really(void)
+{
+ /* Caller won't call this function if in_hardirq() || in_nmi(). */
+#ifdef CONFIG_PREEMPT_RT
+ return !in_serving_softirq();
+#else
+ return in_task();
+#endif
+}
+
static notrace bool check_kcov_mode(enum kcov_mode needed_mode, struct task_struct *t)
{
unsigned int mode;
@@ -869,9 +879,10 @@ void kcov_remote_start(u64 handle)
int sequence;
unsigned long flags;
- if (WARN_ON(!kcov_check_handle(handle, true, true, true)))
+ /* Don't use in_task() in order to allow consistent checks in RT kernels. */
+ if (in_hardirq() || in_nmi())
return;
- if (!in_task() && !in_softirq_really())
+ if (WARN_ON(!kcov_check_handle(handle, true, true, true)))
return;
local_lock_irqsave(&kcov_percpu_data.lock, flags);
@@ -903,7 +914,7 @@ void kcov_remote_start(u64 handle)
return;
}
kcov_debug("handle = %llx, context: %s\n", handle,
- in_task() ? "task" : "softirq");
+ in_task_really() ? "task" : "softirq");
kcov = remote->kcov;
/* Put in kcov_remote_stop(). */
kcov_get(kcov);
@@ -915,7 +926,7 @@ void kcov_remote_start(u64 handle)
*/
mode = context_unsafe(kcov->mode);
sequence = kcov->sequence;
- if (in_task()) {
+ if (in_task_really()) {
size = kcov->remote_size;
area = kcov_remote_area_get(size);
} else {
@@ -924,7 +935,7 @@ void kcov_remote_start(u64 handle)
}
spin_unlock(&kcov_remote_lock);
- /* Can only happen when in_task(). */
+ /* Can only happen when in_task_really(). */
if (!area) {
local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
area = vmalloc(size * sizeof(unsigned long));
@@ -1024,7 +1035,8 @@ void kcov_remote_stop(void)
int sequence;
unsigned long flags;
- if (!in_task() && !in_softirq_really())
+ /* Don't use in_task() in order to allow consistent checks in RT kernels. */
+ if (in_hardirq() || in_nmi())
return;
local_lock_irqsave(&kcov_percpu_data.lock, flags);
@@ -1069,7 +1081,7 @@ void kcov_remote_stop(void)
kcov_move_area(kcov->mode, kcov->area, kcov->size, area);
spin_unlock(&kcov->lock);
- if (in_task()) {
+ if (in_task_really()) {
spin_lock(&kcov_remote_lock);
kcov_remote_area_put(area, size);
spin_unlock(&kcov_remote_lock);
--
2.47.3
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v2] kcov: fix state corruption under CONFIG_PREEMPT_RT by eliminating per-cpu data
2026-05-05 8:15 ` [PATCH] " Tetsuo Handa
@ 2026-05-06 11:55 ` Tetsuo Handa
2026-05-09 8:09 ` [PATCH v3] kcov: move kcov_remote_data to task_struct for RT and remove local_lock Tetsuo Handa
0 siblings, 1 reply; 29+ messages in thread
From: Tetsuo Handa @ 2026-05-06 11:55 UTC (permalink / raw)
To: kasan-dev, LKML; +Cc: Dmitry Vyukov, Alexander Potapenko
Problem:
In CONFIG_PREEMPT_RT=y kernels, KCOV experiences logical errors and
WARNINGs (as reported by syzbot). The root cause is a twofold mismatch
between KCOV's design and the RT preemptive model:
1. Reentrancy on the same CPU: KCOV uses per_cpu variables
(kcov_percpu_data) to save/restore states and provide a temporary
irq_area. In RT kernels, local_lock_irqsave() does not disable
preemption. Thus, a task executing KCOV code can be preempted by a
threaded Softirq on the same CPU. If the Softirq also triggers KCOV,
it overwrites the per_cpu data, corrupting the preempted task's state.
2. Context Confusion: PREEMPT_RT often executes Softirqs within the
task_struct context of the currently running task (e.g., a kworker or
threaded IRQ handler). Since KCOV relies on in_task() to decide whether
to modify current->kcov_mode, it mistakenly modifies the kcov_mode of
an unrelated kworker when a Softirq "borrows" its context.
Solution:
This patch eliminates the use of per_cpu data structures for KCOV when
CONFIG_PREEMPT_RT is enabled, moving the necessary state into task_struct.
* Task-local Storage: Added kcov_saved_* fields to task_struct under
CONFIG_PREEMPT_RT. This ensures that even if a task is preempted during
remote coverage collection, its state remains isolated and travels with
the task itself.
* Eliminate irq_area Sharing: In RT kernels, kcov_remote_start() now
always utilizes kcov_remote_area_get() (the remote area pool) instead
of the shared per-cpu irq_area. This prevents buffer collision between
preempting contexts on the same CPU.
* Consistent Context Checks: Replaced unreliable in_task() checks with
explicit in_hardirq() || in_nmi() guards in critical paths to ensure
KCOV's state machine remains consistent regardless of whether a Softirq
is threaded or "borrowing" a task context.
* Conditional Lock Removal: Since all data is now task-local in the RT
case, the requirement for local_lock is removed for
CONFIG_PREEMPT_RT=y, reducing unnecessary locking overhead.
This change ensures that KCOV is fully reentrant and safe for the
PREEMPT_RT execution model without sacrificing the performance of
non-RT kernels.
Link: https://syzkaller.appspot.com/bug?extid=e6686317bd9fe911591a
Analyzed-by: AI Mode in Google Search (no mail address)
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Fixes: 5ff3b30ab57d ("kcov: collect coverage from interrupts")
---
Only compile tested. I don't have environment to measure how ignoring
CONFIG_KCOV_IRQ_AREA_SIZE affects reliability / performance. We might
need to enforce CONFIG_KCOV_IRQ_AREA_SIZE for in_serving_softirq() case
if remote_arg->area_size < CONFIG_KCOV_IRQ_AREA_SIZE in
ioctl(KCOV_REMOTE_ENABLE) request. Please be sure to test this patch
using CONFIG_PREEMPT_RT=y and CONFIG_PREEMPT_RT=n kernels in local
syzkaller environment before sending upstream.
include/linux/sched.h | 10 +++++
kernel/kcov.c | 87 ++++++++++++++++++++++++++++++++++---------
2 files changed, 79 insertions(+), 18 deletions(-)
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 368c7b4d7cb5..2c963f4271d6 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1522,6 +1522,16 @@ struct task_struct {
/* Collect coverage from softirq context: */
unsigned int kcov_softirq;
+
+#ifdef CONFIG_PREEMPT_RT
+ /* Temporary storage for preempting remote coverage collection: */
+ unsigned int kcov_saved_mode;
+ unsigned int kcov_saved_size;
+ void *kcov_saved_area;
+ struct kcov *kcov_saved_kcov;
+ int kcov_saved_sequence;
+#endif
+
#endif
#ifdef CONFIG_MEMCG_V1
diff --git a/kernel/kcov.c b/kernel/kcov.c
index 0b369e88c7c9..3178a0e03c3b 100644
--- a/kernel/kcov.c
+++ b/kernel/kcov.c
@@ -88,6 +88,7 @@ static DEFINE_SPINLOCK(kcov_remote_lock);
static DEFINE_HASHTABLE(kcov_remote_map, 4);
static struct list_head kcov_remote_areas = LIST_HEAD_INIT(kcov_remote_areas);
+#ifndef CONFIG_PREEMPT_RT
struct kcov_percpu_data {
void *irq_area;
local_lock_t lock;
@@ -102,6 +103,7 @@ struct kcov_percpu_data {
static DEFINE_PER_CPU(struct kcov_percpu_data, kcov_percpu_data) = {
.lock = INIT_LOCAL_LOCK(lock),
};
+#endif
/* Must be called with kcov_remote_lock locked. */
static struct kcov_remote *kcov_remote_find(u64 handle)
@@ -823,6 +825,44 @@ static inline bool kcov_mode_enabled(unsigned int mode)
return (mode & ~KCOV_IN_CTXSW) != KCOV_MODE_DISABLED;
}
+#ifdef CONFIG_PREEMPT_RT
+static inline void kcov_local_lock_irqsave(unsigned long flags) { }
+static inline void kcov_local_unlock_irqrestore(unsigned long flags) { }
+
+static void kcov_remote_softirq_start(struct task_struct *t)
+{
+ unsigned int mode;
+
+ mode = READ_ONCE(t->kcov_mode);
+ barrier();
+ if (kcov_mode_enabled(mode)) {
+ t->kcov_saved_mode = mode;
+ t->kcov_saved_size = t->kcov_size;
+ t->kcov_saved_area = t->kcov_area;
+ t->kcov_saved_sequence = t->kcov_sequence;
+ t->kcov_saved_kcov = t->kcov;
+ kcov_stop(t);
+ }
+}
+
+static void kcov_remote_softirq_stop(struct task_struct *t)
+{
+ if (t->kcov_saved_kcov) {
+ kcov_start(t, t->kcov_saved_kcov, t->kcov_saved_size,
+ t->kcov_saved_area, t->kcov_saved_mode,
+ t->kcov_saved_sequence);
+ t->kcov_saved_mode = 0;
+ t->kcov_saved_size = 0;
+ t->kcov_saved_area = NULL;
+ t->kcov_saved_sequence = 0;
+ t->kcov_saved_kcov = NULL;
+ }
+}
+
+#else
+#define kcov_local_lock_irqsave(flags) local_lock_irqsave(&kcov_percpu_data.lock, flags)
+#define kcov_local_unlock_irqrestore(flags) local_unlock_irqrestore(&kcov_percpu_data.lock, flags)
+
static void kcov_remote_softirq_start(struct task_struct *t)
__must_hold(&kcov_percpu_data.lock)
{
@@ -858,6 +898,8 @@ static void kcov_remote_softirq_stop(struct task_struct *t)
}
}
+#endif
+
void kcov_remote_start(u64 handle)
{
struct task_struct *t = current;
@@ -869,12 +911,13 @@ void kcov_remote_start(u64 handle)
int sequence;
unsigned long flags;
- if (WARN_ON(!kcov_check_handle(handle, true, true, true)))
+ /* Don't use in_task() in order to allow consistent checks in RT kernels. */
+ if (in_hardirq() || in_nmi())
return;
- if (!in_task() && !in_softirq_really())
+ if (WARN_ON(!kcov_check_handle(handle, true, true, true)))
return;
- local_lock_irqsave(&kcov_percpu_data.lock, flags);
+ kcov_local_lock_irqsave(flags);
/*
* Check that kcov_remote_start() is not called twice in background
@@ -882,7 +925,7 @@ void kcov_remote_start(u64 handle)
*/
mode = READ_ONCE(t->kcov_mode);
if (WARN_ON(in_task() && kcov_mode_enabled(mode))) {
- local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
+ kcov_local_unlock_irqrestore(flags);
return;
}
/*
@@ -891,7 +934,7 @@ void kcov_remote_start(u64 handle)
* happened while collecting coverage from a background thread.
*/
if (WARN_ON(in_serving_softirq() && t->kcov_softirq)) {
- local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
+ kcov_local_unlock_irqrestore(flags);
return;
}
@@ -899,11 +942,11 @@ void kcov_remote_start(u64 handle)
remote = kcov_remote_find(handle);
if (!remote) {
spin_unlock(&kcov_remote_lock);
- local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
+ kcov_local_unlock_irqrestore(flags);
return;
}
kcov_debug("handle = %llx, context: %s\n", handle,
- in_task() ? "task" : "softirq");
+ IS_ENABLED(CONFIG_PREEMPT_RT) || in_task() ? "task" : "softirq");
kcov = remote->kcov;
/* Put in kcov_remote_stop(). */
kcov_get(kcov);
@@ -915,6 +958,10 @@ void kcov_remote_start(u64 handle)
*/
mode = context_unsafe(kcov->mode);
sequence = kcov->sequence;
+#ifdef CONFIG_PREEMPT_RT
+ size = kcov->remote_size;
+ area = kcov_remote_area_get(size);
+#else
if (in_task()) {
size = kcov->remote_size;
area = kcov_remote_area_get(size);
@@ -922,17 +969,18 @@ void kcov_remote_start(u64 handle)
size = CONFIG_KCOV_IRQ_AREA_SIZE;
area = this_cpu_ptr(&kcov_percpu_data)->irq_area;
}
+#endif
spin_unlock(&kcov_remote_lock);
- /* Can only happen when in_task(). */
+ /* Can only happen when CONFIG_PREEMPT_RT=y or in_task(). */
if (!area) {
- local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
+ kcov_local_unlock_irqrestore(flags);
area = vmalloc(size * sizeof(unsigned long));
if (!area) {
kcov_put(kcov);
return;
}
- local_lock_irqsave(&kcov_percpu_data.lock, flags);
+ kcov_local_lock_irqsave(flags);
}
/* Reset coverage size. */
@@ -944,7 +992,7 @@ void kcov_remote_start(u64 handle)
}
kcov_start(t, kcov, size, area, mode, sequence);
- local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
+ kcov_local_unlock_irqrestore(flags);
}
EXPORT_SYMBOL(kcov_remote_start);
@@ -1024,15 +1072,16 @@ void kcov_remote_stop(void)
int sequence;
unsigned long flags;
- if (!in_task() && !in_softirq_really())
+ /* Don't use in_task() in order to allow consistent checks in RT kernels. */
+ if (in_hardirq() || in_nmi())
return;
- local_lock_irqsave(&kcov_percpu_data.lock, flags);
+ kcov_local_lock_irqsave(flags);
mode = READ_ONCE(t->kcov_mode);
barrier();
if (!kcov_mode_enabled(mode)) {
- local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
+ kcov_local_unlock_irqrestore(flags);
return;
}
/*
@@ -1040,12 +1089,12 @@ void kcov_remote_stop(void)
* actually found the remote handle and started collecting coverage.
*/
if (in_serving_softirq() && !t->kcov_softirq) {
- local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
+ kcov_local_unlock_irqrestore(flags);
return;
}
/* Make sure that kcov_softirq is only set when in softirq. */
if (WARN_ON(!in_serving_softirq() && t->kcov_softirq)) {
- local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
+ kcov_local_unlock_irqrestore(flags);
return;
}
@@ -1069,13 +1118,13 @@ void kcov_remote_stop(void)
kcov_move_area(kcov->mode, kcov->area, kcov->size, area);
spin_unlock(&kcov->lock);
- if (in_task()) {
+ if (IS_ENABLED(CONFIG_PREEMPT_RT) || in_task()) {
spin_lock(&kcov_remote_lock);
kcov_remote_area_put(area, size);
spin_unlock(&kcov_remote_lock);
}
- local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
+ kcov_local_unlock_irqrestore(flags);
/* Get in kcov_remote_start(). */
kcov_put(kcov);
@@ -1119,6 +1168,7 @@ static void __init selftest(void)
static int __init kcov_init(void)
{
+#ifndef CONFIG_PREEMPT_RT
int cpu;
for_each_possible_cpu(cpu) {
@@ -1128,6 +1178,7 @@ static int __init kcov_init(void)
return -ENOMEM;
per_cpu_ptr(&kcov_percpu_data, cpu)->irq_area = area;
}
+#endif
/*
* The kcov debugfs file won't ever get removed and thus,
--
2.47.3
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v3] kcov: move kcov_remote_data to task_struct for RT and remove local_lock
2026-05-06 11:55 ` [PATCH v2] kcov: fix state corruption under CONFIG_PREEMPT_RT by eliminating per-cpu data Tetsuo Handa
@ 2026-05-09 8:09 ` Tetsuo Handa
2026-05-15 7:10 ` Dmitry Vyukov
` (3 more replies)
0 siblings, 4 replies; 29+ messages in thread
From: Tetsuo Handa @ 2026-05-09 8:09 UTC (permalink / raw)
To: kasan-dev, LKML, Dmitry Vyukov, Alexander Potapenko
Cc: Alan Stern, Andrew Morton, Andrey Konovalov, Andrey Konovalov,
Clark Williams, Greg Kroah-Hartman, Marco Elver,
Sebastian Andrzej Siewior
In CONFIG_PREEMPT_RT=y kernels, softirqs are executed in a per-CPU
ksoftirqd thread or in the context of the task that raised the softirq.
This means in_task() can return true even while serving softirqs. This
behavior causes KCOV to incorrectly identify the context, leading to state
corruption and various kcov-related warnings reported by syzbot.
Furthermore, commit d5d2c51f1e5f ("kcov: replace local_irq_save() with a
local_lock_t") introduced local_lock to protect per-CPU kcov_percpu_data.
However, the need for this physical CPU-level locking is questionable
because:
1. kcov_remote_start/stop() already bail out early if called from
in_hardirq() or in_nmi().
2. The core tracing function check_kcov_mode() also skips hardirq/NMI
contexts, meaning no KCOV state is accessed during hardirqs even if
they interrupt a KCOV-enabled task/softirq.
3. In non-RT kernels, softirqs do not nest, so no concurrent access to
per-CPU data occurs between softirqs.
4. In RT kernels, while softirqs can be preempted, this patch moves the
KCOV state from per-CPU variables to task_struct (per-task), eliminating
the contention on shared per-CPU resources.
By moving kcov_remote_data to task_struct for RT kernels and replacing
the in_task() check with !in_serving_softirq(), we ensure consistent
context detection. Since the data is now isolated per-task and not accessed
by hardirqs, the local_lock (and the original local_irq_save) is no longer
necessary and is removed to reduce overhead.
Changes:
* Move remote coverage state from kcov_percpu_data to task_struct under
CONFIG_PREEMPT_RT.
* Replace in_task() with !in_serving_softirq() in kcov_remote_start/stop()
for accurate context tracking.
* Remove local_lock and IRQ disabling from kcov_remote_start/stop() as the
state is now task-local and hardirqs are already excluded.
* Ensure CONFIG_PREEMPT_RT=y uses kcov_remote_area_get() (the
vmalloc-backed pool) instead of the single per-CPU irq_area.
Analyzed-by: AI Mode in Google Search (no mail address)
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Fixes: 5ff3b30ab57d ("kcov: collect coverage from interrupts")
---
Only compile tested. I don't have environment to measure how not
preallocating CONFIG_KCOV_IRQ_AREA_SIZE bytes of buffers and
add to the global pool in kcov_init() impacts reliability / performance.
Please be sure to test this patch using CONFIG_PREEMPT_RT=y and
CONFIG_PREEMPT_RT=n kernels in local syzkaller environment before
sending upstream.
This patch is expected to address various KCOV related reports.
But I don't add Closes: lines because we can't tell whether this patch
alone is sufficient for marking as Closes:. We will find the answer
some time after being sent to upstream.
https://syzkaller.appspot.com/bug?extid=90984d3713722683112e
https://syzkaller.appspot.com/bug?extid=47cf95ca1f9dcca872c8
https://syzkaller.appspot.com/bug?extid=8a173e13208949931dc7
https://syzkaller.appspot.com/bug?extid=3f51ad7ac3ae57a6fdcc
https://syzkaller.appspot.com/bug?extid=e6686317bd9fe911591a
include/linux/sched.h | 10 +++++
kernel/kcov.c | 94 +++++++++++++++++++++++++------------------
lib/Kconfig.debug | 1 +
3 files changed, 65 insertions(+), 40 deletions(-)
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 368c7b4d7cb5..2c963f4271d6 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1522,6 +1522,16 @@ struct task_struct {
/* Collect coverage from softirq context: */
unsigned int kcov_softirq;
+
+#ifdef CONFIG_PREEMPT_RT
+ /* Temporary storage for preempting remote coverage collection: */
+ unsigned int kcov_saved_mode;
+ unsigned int kcov_saved_size;
+ void *kcov_saved_area;
+ struct kcov *kcov_saved_kcov;
+ int kcov_saved_sequence;
+#endif
+
#endif
#ifdef CONFIG_MEMCG_V1
diff --git a/kernel/kcov.c b/kernel/kcov.c
index 0b369e88c7c9..965c11a75b36 100644
--- a/kernel/kcov.c
+++ b/kernel/kcov.c
@@ -88,9 +88,9 @@ static DEFINE_SPINLOCK(kcov_remote_lock);
static DEFINE_HASHTABLE(kcov_remote_map, 4);
static struct list_head kcov_remote_areas = LIST_HEAD_INIT(kcov_remote_areas);
+#ifndef CONFIG_PREEMPT_RT
struct kcov_percpu_data {
void *irq_area;
- local_lock_t lock;
unsigned int saved_mode;
unsigned int saved_size;
@@ -100,8 +100,8 @@ struct kcov_percpu_data {
};
static DEFINE_PER_CPU(struct kcov_percpu_data, kcov_percpu_data) = {
- .lock = INIT_LOCAL_LOCK(lock),
};
+#endif
/* Must be called with kcov_remote_lock locked. */
static struct kcov_remote *kcov_remote_find(u64 handle)
@@ -823,8 +823,38 @@ static inline bool kcov_mode_enabled(unsigned int mode)
return (mode & ~KCOV_IN_CTXSW) != KCOV_MODE_DISABLED;
}
+#ifdef CONFIG_PREEMPT_RT
+static void kcov_remote_softirq_start(struct task_struct *t)
+{
+ unsigned int mode;
+
+ mode = READ_ONCE(t->kcov_mode);
+ barrier();
+ if (kcov_mode_enabled(mode)) {
+ t->kcov_saved_mode = mode;
+ t->kcov_saved_size = t->kcov_size;
+ t->kcov_saved_area = t->kcov_area;
+ t->kcov_saved_sequence = t->kcov_sequence;
+ t->kcov_saved_kcov = t->kcov;
+ kcov_stop(t);
+ }
+}
+
+static void kcov_remote_softirq_stop(struct task_struct *t)
+{
+ if (t->kcov_saved_kcov) {
+ kcov_start(t, t->kcov_saved_kcov, t->kcov_saved_size,
+ t->kcov_saved_area, t->kcov_saved_mode,
+ t->kcov_saved_sequence);
+ t->kcov_saved_mode = 0;
+ t->kcov_saved_size = 0;
+ t->kcov_saved_area = NULL;
+ t->kcov_saved_sequence = 0;
+ t->kcov_saved_kcov = NULL;
+ }
+}
+#else
static void kcov_remote_softirq_start(struct task_struct *t)
- __must_hold(&kcov_percpu_data.lock)
{
struct kcov_percpu_data *data = this_cpu_ptr(&kcov_percpu_data);
unsigned int mode;
@@ -842,7 +872,6 @@ static void kcov_remote_softirq_start(struct task_struct *t)
}
static void kcov_remote_softirq_stop(struct task_struct *t)
- __must_hold(&kcov_percpu_data.lock)
{
struct kcov_percpu_data *data = this_cpu_ptr(&kcov_percpu_data);
@@ -857,6 +886,7 @@ static void kcov_remote_softirq_stop(struct task_struct *t)
data->saved_kcov = NULL;
}
}
+#endif
void kcov_remote_start(u64 handle)
{
@@ -867,43 +897,35 @@ void kcov_remote_start(u64 handle)
void *area;
unsigned int size;
int sequence;
- unsigned long flags;
- if (WARN_ON(!kcov_check_handle(handle, true, true, true)))
+ /* Don't use in_task() in order to allow consistent checks in RT kernels. */
+ if (in_hardirq() || in_nmi())
return;
- if (!in_task() && !in_softirq_really())
+ if (WARN_ON(!kcov_check_handle(handle, true, true, true)))
return;
- local_lock_irqsave(&kcov_percpu_data.lock, flags);
-
/*
* Check that kcov_remote_start() is not called twice in background
* threads nor called by user tasks (with enabled kcov).
*/
- mode = READ_ONCE(t->kcov_mode);
- if (WARN_ON(in_task() && kcov_mode_enabled(mode))) {
- local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
+ if (WARN_ON(!in_serving_softirq() && kcov_mode_enabled(READ_ONCE(t->kcov_mode))))
return;
- }
/*
* Check that kcov_remote_start() is not called twice in softirqs.
* Note, that kcov_remote_start() can be called from a softirq that
* happened while collecting coverage from a background thread.
*/
- if (WARN_ON(in_serving_softirq() && t->kcov_softirq)) {
- local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
+ if (WARN_ON(in_serving_softirq() && t->kcov_softirq))
return;
- }
spin_lock(&kcov_remote_lock);
remote = kcov_remote_find(handle);
if (!remote) {
spin_unlock(&kcov_remote_lock);
- local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
return;
}
kcov_debug("handle = %llx, context: %s\n", handle,
- in_task() ? "task" : "softirq");
+ !in_serving_softirq() ? "task" : "softirq");
kcov = remote->kcov;
/* Put in kcov_remote_stop(). */
kcov_get(kcov);
@@ -915,6 +937,10 @@ void kcov_remote_start(u64 handle)
*/
mode = context_unsafe(kcov->mode);
sequence = kcov->sequence;
+#ifdef CONFIG_PREEMPT_RT
+ size = kcov->remote_size;
+ area = kcov_remote_area_get(size);
+#else
if (in_task()) {
size = kcov->remote_size;
area = kcov_remote_area_get(size);
@@ -922,17 +948,16 @@ void kcov_remote_start(u64 handle)
size = CONFIG_KCOV_IRQ_AREA_SIZE;
area = this_cpu_ptr(&kcov_percpu_data)->irq_area;
}
+#endif
spin_unlock(&kcov_remote_lock);
- /* Can only happen when in_task(). */
+ /* Can only happen when CONFIG_PREEMPT_RT=y or in_task(). */
if (!area) {
- local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
area = vmalloc(size * sizeof(unsigned long));
if (!area) {
kcov_put(kcov);
return;
}
- local_lock_irqsave(&kcov_percpu_data.lock, flags);
}
/* Reset coverage size. */
@@ -943,9 +968,6 @@ void kcov_remote_start(u64 handle)
t->kcov_softirq = 1;
}
kcov_start(t, kcov, size, area, mode, sequence);
-
- local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
-
}
EXPORT_SYMBOL(kcov_remote_start);
@@ -1022,32 +1044,24 @@ void kcov_remote_stop(void)
void *area;
unsigned int size;
int sequence;
- unsigned long flags;
- if (!in_task() && !in_softirq_really())
+ /* Don't use in_task() in order to allow consistent checks in RT kernels. */
+ if (in_hardirq() || in_nmi())
return;
- local_lock_irqsave(&kcov_percpu_data.lock, flags);
-
mode = READ_ONCE(t->kcov_mode);
barrier();
- if (!kcov_mode_enabled(mode)) {
- local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
+ if (!kcov_mode_enabled(mode))
return;
- }
/*
* When in softirq, check if the corresponding kcov_remote_start()
* actually found the remote handle and started collecting coverage.
*/
- if (in_serving_softirq() && !t->kcov_softirq) {
- local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
+ if (in_serving_softirq() && !t->kcov_softirq)
return;
- }
/* Make sure that kcov_softirq is only set when in softirq. */
- if (WARN_ON(!in_serving_softirq() && t->kcov_softirq)) {
- local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
+ if (WARN_ON(!in_serving_softirq() && t->kcov_softirq))
return;
- }
kcov = t->kcov;
area = t->kcov_area;
@@ -1069,14 +1083,12 @@ void kcov_remote_stop(void)
kcov_move_area(kcov->mode, kcov->area, kcov->size, area);
spin_unlock(&kcov->lock);
- if (in_task()) {
+ if (IS_ENABLED(CONFIG_PREEMPT_RT) || in_task()) {
spin_lock(&kcov_remote_lock);
kcov_remote_area_put(area, size);
spin_unlock(&kcov_remote_lock);
}
- local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
-
/* Get in kcov_remote_start(). */
kcov_put(kcov);
}
@@ -1119,6 +1131,7 @@ static void __init selftest(void)
static int __init kcov_init(void)
{
+#ifndef CONFIG_PREEMPT_RT
int cpu;
for_each_possible_cpu(cpu) {
@@ -1128,6 +1141,7 @@ static int __init kcov_init(void)
return -ENOMEM;
per_cpu_ptr(&kcov_percpu_data, cpu)->irq_area = area;
}
+#endif
/*
* The kcov debugfs file won't ever get removed and thus,
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 13f3297aa823..493be2c73c9d 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -2247,6 +2247,7 @@ config KCOV_INSTRUMENT_ALL
config KCOV_IRQ_AREA_SIZE
hex "Size of interrupt coverage collection area in words"
depends on KCOV
+ depends on !PREEMPT_RT
default 0x40000
help
KCOV uses preallocated per-cpu areas to collect coverage from
--
2.47.3
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3] kcov: move kcov_remote_data to task_struct for RT and remove local_lock
2026-05-09 8:09 ` [PATCH v3] kcov: move kcov_remote_data to task_struct for RT and remove local_lock Tetsuo Handa
@ 2026-05-15 7:10 ` Dmitry Vyukov
2026-05-20 10:33 ` Tetsuo Handa
2026-05-20 15:12 ` Sebastian Andrzej Siewior
` (2 subsequent siblings)
3 siblings, 1 reply; 29+ messages in thread
From: Dmitry Vyukov @ 2026-05-15 7:10 UTC (permalink / raw)
To: Tetsuo Handa, Aleksandr Nogikh
Cc: kasan-dev, LKML, Alexander Potapenko, Alan Stern, Andrew Morton,
Andrey Konovalov, Clark Williams, Greg Kroah-Hartman,
Marco Elver, Sebastian Andrzej Siewior
On Sat, 9 May 2026 at 10:09, Tetsuo Handa
<penguin-kernel@i-love.sakura.ne.jp> wrote:
>
> In CONFIG_PREEMPT_RT=y kernels, softirqs are executed in a per-CPU
> ksoftirqd thread or in the context of the task that raised the softirq.
> This means in_task() can return true even while serving softirqs. This
> behavior causes KCOV to incorrectly identify the context, leading to state
> corruption and various kcov-related warnings reported by syzbot.
>
> Furthermore, commit d5d2c51f1e5f ("kcov: replace local_irq_save() with a
> local_lock_t") introduced local_lock to protect per-CPU kcov_percpu_data.
> However, the need for this physical CPU-level locking is questionable
> because:
>
> 1. kcov_remote_start/stop() already bail out early if called from
> in_hardirq() or in_nmi().
> 2. The core tracing function check_kcov_mode() also skips hardirq/NMI
> contexts, meaning no KCOV state is accessed during hardirqs even if
> they interrupt a KCOV-enabled task/softirq.
> 3. In non-RT kernels, softirqs do not nest, so no concurrent access to
> per-CPU data occurs between softirqs.
> 4. In RT kernels, while softirqs can be preempted, this patch moves the
> KCOV state from per-CPU variables to task_struct (per-task), eliminating
> the contention on shared per-CPU resources.
>
> By moving kcov_remote_data to task_struct for RT kernels and replacing
> the in_task() check with !in_serving_softirq(), we ensure consistent
> context detection. Since the data is now isolated per-task and not accessed
> by hardirqs, the local_lock (and the original local_irq_save) is no longer
> necessary and is removed to reduce overhead.
>
> Changes:
>
> * Move remote coverage state from kcov_percpu_data to task_struct under
> CONFIG_PREEMPT_RT.
> * Replace in_task() with !in_serving_softirq() in kcov_remote_start/stop()
> for accurate context tracking.
> * Remove local_lock and IRQ disabling from kcov_remote_start/stop() as the
> state is now task-local and hardirqs are already excluded.
> * Ensure CONFIG_PREEMPT_RT=y uses kcov_remote_area_get() (the
> vmalloc-backed pool) instead of the single per-CPU irq_area.
>
> Analyzed-by: AI Mode in Google Search (no mail address)
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Fixes: 5ff3b30ab57d ("kcov: collect coverage from interrupts")
> ---
> Only compile tested. I don't have environment to measure how not
+Aleksandr Nogikh
I don't see this patch on syzbot CI:
https://ci.syzbot.org/?name=kcov%3A+move+kcov_remote_data+to+task_struct+for+RT+and+remove+local_lock
How can we make it at least tested on syzbot CI? Otherwise it risks
breaking syzbot when merged.
> preallocating CONFIG_KCOV_IRQ_AREA_SIZE bytes of buffers and
> add to the global pool in kcov_init() impacts reliability / performance.
> Please be sure to test this patch using CONFIG_PREEMPT_RT=y and
> CONFIG_PREEMPT_RT=n kernels in local syzkaller environment before
> sending upstream.
>
> This patch is expected to address various KCOV related reports.
> But I don't add Closes: lines because we can't tell whether this patch
> alone is sufficient for marking as Closes:. We will find the answer
> some time after being sent to upstream.
>
> https://syzkaller.appspot.com/bug?extid=90984d3713722683112e
> https://syzkaller.appspot.com/bug?extid=47cf95ca1f9dcca872c8
> https://syzkaller.appspot.com/bug?extid=8a173e13208949931dc7
> https://syzkaller.appspot.com/bug?extid=3f51ad7ac3ae57a6fdcc
> https://syzkaller.appspot.com/bug?extid=e6686317bd9fe911591a
>
> include/linux/sched.h | 10 +++++
> kernel/kcov.c | 94 +++++++++++++++++++++++++------------------
> lib/Kconfig.debug | 1 +
> 3 files changed, 65 insertions(+), 40 deletions(-)
>
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index 368c7b4d7cb5..2c963f4271d6 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -1522,6 +1522,16 @@ struct task_struct {
>
> /* Collect coverage from softirq context: */
> unsigned int kcov_softirq;
> +
> +#ifdef CONFIG_PREEMPT_RT
Is it possible to unify the logic between RT and non-RT kernels (not
have any ifdef's)?
The logic is already rather complicated, having 2 work modes, and
limited testing will be problematic in the future.
> + /* Temporary storage for preempting remote coverage collection: */
> + unsigned int kcov_saved_mode;
> + unsigned int kcov_saved_size;
> + void *kcov_saved_area;
> + struct kcov *kcov_saved_kcov;
> + int kcov_saved_sequence;
> +#endif
> +
> #endif
>
> #ifdef CONFIG_MEMCG_V1
> diff --git a/kernel/kcov.c b/kernel/kcov.c
> index 0b369e88c7c9..965c11a75b36 100644
> --- a/kernel/kcov.c
> +++ b/kernel/kcov.c
> @@ -88,9 +88,9 @@ static DEFINE_SPINLOCK(kcov_remote_lock);
> static DEFINE_HASHTABLE(kcov_remote_map, 4);
> static struct list_head kcov_remote_areas = LIST_HEAD_INIT(kcov_remote_areas);
>
> +#ifndef CONFIG_PREEMPT_RT
> struct kcov_percpu_data {
> void *irq_area;
> - local_lock_t lock;
>
> unsigned int saved_mode;
> unsigned int saved_size;
> @@ -100,8 +100,8 @@ struct kcov_percpu_data {
> };
>
> static DEFINE_PER_CPU(struct kcov_percpu_data, kcov_percpu_data) = {
> - .lock = INIT_LOCAL_LOCK(lock),
> };
> +#endif
>
> /* Must be called with kcov_remote_lock locked. */
> static struct kcov_remote *kcov_remote_find(u64 handle)
> @@ -823,8 +823,38 @@ static inline bool kcov_mode_enabled(unsigned int mode)
> return (mode & ~KCOV_IN_CTXSW) != KCOV_MODE_DISABLED;
> }
>
> +#ifdef CONFIG_PREEMPT_RT
> +static void kcov_remote_softirq_start(struct task_struct *t)
> +{
> + unsigned int mode;
> +
> + mode = READ_ONCE(t->kcov_mode);
> + barrier();
> + if (kcov_mode_enabled(mode)) {
> + t->kcov_saved_mode = mode;
> + t->kcov_saved_size = t->kcov_size;
> + t->kcov_saved_area = t->kcov_area;
> + t->kcov_saved_sequence = t->kcov_sequence;
> + t->kcov_saved_kcov = t->kcov;
> + kcov_stop(t);
> + }
> +}
> +
> +static void kcov_remote_softirq_stop(struct task_struct *t)
> +{
> + if (t->kcov_saved_kcov) {
> + kcov_start(t, t->kcov_saved_kcov, t->kcov_saved_size,
> + t->kcov_saved_area, t->kcov_saved_mode,
> + t->kcov_saved_sequence);
> + t->kcov_saved_mode = 0;
> + t->kcov_saved_size = 0;
> + t->kcov_saved_area = NULL;
> + t->kcov_saved_sequence = 0;
> + t->kcov_saved_kcov = NULL;
> + }
> +}
> +#else
> static void kcov_remote_softirq_start(struct task_struct *t)
> - __must_hold(&kcov_percpu_data.lock)
> {
> struct kcov_percpu_data *data = this_cpu_ptr(&kcov_percpu_data);
> unsigned int mode;
> @@ -842,7 +872,6 @@ static void kcov_remote_softirq_start(struct task_struct *t)
> }
>
> static void kcov_remote_softirq_stop(struct task_struct *t)
> - __must_hold(&kcov_percpu_data.lock)
> {
> struct kcov_percpu_data *data = this_cpu_ptr(&kcov_percpu_data);
>
> @@ -857,6 +886,7 @@ static void kcov_remote_softirq_stop(struct task_struct *t)
> data->saved_kcov = NULL;
> }
> }
> +#endif
>
> void kcov_remote_start(u64 handle)
> {
> @@ -867,43 +897,35 @@ void kcov_remote_start(u64 handle)
> void *area;
> unsigned int size;
> int sequence;
> - unsigned long flags;
>
> - if (WARN_ON(!kcov_check_handle(handle, true, true, true)))
> + /* Don't use in_task() in order to allow consistent checks in RT kernels. */
> + if (in_hardirq() || in_nmi())
> return;
> - if (!in_task() && !in_softirq_really())
> + if (WARN_ON(!kcov_check_handle(handle, true, true, true)))
> return;
>
> - local_lock_irqsave(&kcov_percpu_data.lock, flags);
> -
> /*
> * Check that kcov_remote_start() is not called twice in background
> * threads nor called by user tasks (with enabled kcov).
> */
> - mode = READ_ONCE(t->kcov_mode);
> - if (WARN_ON(in_task() && kcov_mode_enabled(mode))) {
> - local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
> + if (WARN_ON(!in_serving_softirq() && kcov_mode_enabled(READ_ONCE(t->kcov_mode))))
> return;
> - }
> /*
> * Check that kcov_remote_start() is not called twice in softirqs.
> * Note, that kcov_remote_start() can be called from a softirq that
> * happened while collecting coverage from a background thread.
> */
> - if (WARN_ON(in_serving_softirq() && t->kcov_softirq)) {
> - local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
> + if (WARN_ON(in_serving_softirq() && t->kcov_softirq))
> return;
> - }
>
> spin_lock(&kcov_remote_lock);
> remote = kcov_remote_find(handle);
> if (!remote) {
> spin_unlock(&kcov_remote_lock);
> - local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
> return;
> }
> kcov_debug("handle = %llx, context: %s\n", handle,
> - in_task() ? "task" : "softirq");
> + !in_serving_softirq() ? "task" : "softirq");
> kcov = remote->kcov;
> /* Put in kcov_remote_stop(). */
> kcov_get(kcov);
> @@ -915,6 +937,10 @@ void kcov_remote_start(u64 handle)
> */
> mode = context_unsafe(kcov->mode);
> sequence = kcov->sequence;
> +#ifdef CONFIG_PREEMPT_RT
> + size = kcov->remote_size;
> + area = kcov_remote_area_get(size);
> +#else
> if (in_task()) {
> size = kcov->remote_size;
> area = kcov_remote_area_get(size);
> @@ -922,17 +948,16 @@ void kcov_remote_start(u64 handle)
> size = CONFIG_KCOV_IRQ_AREA_SIZE;
> area = this_cpu_ptr(&kcov_percpu_data)->irq_area;
> }
> +#endif
> spin_unlock(&kcov_remote_lock);
>
> - /* Can only happen when in_task(). */
> + /* Can only happen when CONFIG_PREEMPT_RT=y or in_task(). */
> if (!area) {
> - local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
> area = vmalloc(size * sizeof(unsigned long));
> if (!area) {
> kcov_put(kcov);
> return;
> }
> - local_lock_irqsave(&kcov_percpu_data.lock, flags);
> }
>
> /* Reset coverage size. */
> @@ -943,9 +968,6 @@ void kcov_remote_start(u64 handle)
> t->kcov_softirq = 1;
> }
> kcov_start(t, kcov, size, area, mode, sequence);
> -
> - local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
> -
> }
> EXPORT_SYMBOL(kcov_remote_start);
>
> @@ -1022,32 +1044,24 @@ void kcov_remote_stop(void)
> void *area;
> unsigned int size;
> int sequence;
> - unsigned long flags;
>
> - if (!in_task() && !in_softirq_really())
> + /* Don't use in_task() in order to allow consistent checks in RT kernels. */
> + if (in_hardirq() || in_nmi())
> return;
>
> - local_lock_irqsave(&kcov_percpu_data.lock, flags);
> -
> mode = READ_ONCE(t->kcov_mode);
> barrier();
> - if (!kcov_mode_enabled(mode)) {
> - local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
> + if (!kcov_mode_enabled(mode))
> return;
> - }
> /*
> * When in softirq, check if the corresponding kcov_remote_start()
> * actually found the remote handle and started collecting coverage.
> */
> - if (in_serving_softirq() && !t->kcov_softirq) {
> - local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
> + if (in_serving_softirq() && !t->kcov_softirq)
> return;
> - }
> /* Make sure that kcov_softirq is only set when in softirq. */
> - if (WARN_ON(!in_serving_softirq() && t->kcov_softirq)) {
> - local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
> + if (WARN_ON(!in_serving_softirq() && t->kcov_softirq))
> return;
> - }
>
> kcov = t->kcov;
> area = t->kcov_area;
> @@ -1069,14 +1083,12 @@ void kcov_remote_stop(void)
> kcov_move_area(kcov->mode, kcov->area, kcov->size, area);
> spin_unlock(&kcov->lock);
>
> - if (in_task()) {
> + if (IS_ENABLED(CONFIG_PREEMPT_RT) || in_task()) {
> spin_lock(&kcov_remote_lock);
> kcov_remote_area_put(area, size);
> spin_unlock(&kcov_remote_lock);
> }
>
> - local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
> -
> /* Get in kcov_remote_start(). */
> kcov_put(kcov);
> }
> @@ -1119,6 +1131,7 @@ static void __init selftest(void)
>
> static int __init kcov_init(void)
> {
> +#ifndef CONFIG_PREEMPT_RT
> int cpu;
>
> for_each_possible_cpu(cpu) {
> @@ -1128,6 +1141,7 @@ static int __init kcov_init(void)
> return -ENOMEM;
> per_cpu_ptr(&kcov_percpu_data, cpu)->irq_area = area;
> }
> +#endif
>
> /*
> * The kcov debugfs file won't ever get removed and thus,
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 13f3297aa823..493be2c73c9d 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -2247,6 +2247,7 @@ config KCOV_INSTRUMENT_ALL
> config KCOV_IRQ_AREA_SIZE
> hex "Size of interrupt coverage collection area in words"
> depends on KCOV
> + depends on !PREEMPT_RT
> default 0x40000
> help
> KCOV uses preallocated per-cpu areas to collect coverage from
> --
> 2.47.3
>
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3] kcov: move kcov_remote_data to task_struct for RT and remove local_lock
2026-05-15 7:10 ` Dmitry Vyukov
@ 2026-05-20 10:33 ` Tetsuo Handa
0 siblings, 0 replies; 29+ messages in thread
From: Tetsuo Handa @ 2026-05-20 10:33 UTC (permalink / raw)
To: Dmitry Vyukov, Aleksandr Nogikh
Cc: kasan-dev, LKML, Alexander Potapenko, Alan Stern, Andrew Morton,
Andrey Konovalov, Clark Williams, Greg Kroah-Hartman,
Marco Elver, Sebastian Andrzej Siewior
On 2026/05/15 16:10, Dmitry Vyukov wrote:
>> Only compile tested. I don't have environment to measure how not
>
> +Aleksandr Nogikh
>
> I don't see this patch on syzbot CI:
> https://ci.syzbot.org/?name=kcov%3A+move+kcov_remote_data+to+task_struct+for+RT+and+remove+local_lock
I didn't include syzbot related mail address.
What is needed to make this patch detected by syzbot CI?
>
> How can we make it at least tested on syzbot CI? Otherwise it risks
> breaking syzbot when merged.
Someone who can run syzkaller will be able to test this patch.
If nobody can try, I can try sending this patch to only linux-next tree via my tree.
>> +#ifdef CONFIG_PREEMPT_RT
>
> Is it possible to unify the logic between RT and non-RT kernels (not
> have any ifdef's)?
> The logic is already rather complicated, having 2 work modes, and
> limited testing will be problematic in the future.
Always using per task_struct temporary storage will be possible if we don't mind
bloating sizeof(struct task_struct). But we can't unify buffer allocation part
because allocating large buffer using kmalloc(GFP_ATOMIC) for CONFIG_PREEMPT_RT=n
is unreliable. Preallocating global non-percpu buffers for CONFIG_PREEMPT_RT=y
would be possible, but we don't know how many we should allocate due to
possibility of preemption in CONFIG_PREEMPT_RT=y.
>> @@ -915,6 +937,10 @@ void kcov_remote_start(u64 handle)
>> */
>> mode = context_unsafe(kcov->mode);
>> sequence = kcov->sequence;
>> +#ifdef CONFIG_PREEMPT_RT
>> + size = kcov->remote_size;
>> + area = kcov_remote_area_get(size);
>> +#else
>> if (in_task()) {
>> size = kcov->remote_size;
>> area = kcov_remote_area_get(size);
>> @@ -922,17 +948,16 @@ void kcov_remote_start(u64 handle)
>> size = CONFIG_KCOV_IRQ_AREA_SIZE;
>> area = this_cpu_ptr(&kcov_percpu_data)->irq_area;
>> }
>> +#endif
>> spin_unlock(&kcov_remote_lock);
>>
>> - /* Can only happen when in_task(). */
>> + /* Can only happen when CONFIG_PREEMPT_RT=y or in_task(). */
>> if (!area) {
>> - local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
>> area = vmalloc(size * sizeof(unsigned long));
>> if (!area) {
>> kcov_put(kcov);
>> return;
>> }
>> - local_lock_irqsave(&kcov_percpu_data.lock, flags);
>> }
>>
>> /* Reset coverage size. */
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3] kcov: move kcov_remote_data to task_struct for RT and remove local_lock
2026-05-09 8:09 ` [PATCH v3] kcov: move kcov_remote_data to task_struct for RT and remove local_lock Tetsuo Handa
2026-05-15 7:10 ` Dmitry Vyukov
@ 2026-05-20 15:12 ` Sebastian Andrzej Siewior
2026-05-21 8:38 ` Sebastian Andrzej Siewior
2026-05-21 13:06 ` [PATCH v3] kcov: move kcov_remote_data to task_struct for RT and remove local_lock Alexander Potapenko
3 siblings, 0 replies; 29+ messages in thread
From: Sebastian Andrzej Siewior @ 2026-05-20 15:12 UTC (permalink / raw)
To: Tetsuo Handa
Cc: kasan-dev, LKML, Dmitry Vyukov, Alexander Potapenko, Alan Stern,
Andrew Morton, Andrey Konovalov, Andrey Konovalov,
Clark Williams, Greg Kroah-Hartman, Marco Elver
On 2026-05-09 17:09:44 [+0900], Tetsuo Handa wrote:
> In CONFIG_PREEMPT_RT=y kernels, softirqs are executed in a per-CPU
> ksoftirqd thread or in the context of the task that raised the softirq.
> This means in_task() can return true even while serving softirqs. This
> behavior causes KCOV to incorrectly identify the context, leading to state
> corruption and various kcov-related warnings reported by syzbot.
I need to digest the remaining email but in_task() should not return on
PREEMPT_RT if a softirq is served. Regardless if it happens in ksoftirqd
or in the context of the task raised via local_bh_enable().
I will digest this later.
Sebastian
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3] kcov: move kcov_remote_data to task_struct for RT and remove local_lock
2026-05-09 8:09 ` [PATCH v3] kcov: move kcov_remote_data to task_struct for RT and remove local_lock Tetsuo Handa
2026-05-15 7:10 ` Dmitry Vyukov
2026-05-20 15:12 ` Sebastian Andrzej Siewior
@ 2026-05-21 8:38 ` Sebastian Andrzej Siewior
2026-05-21 15:28 ` Tetsuo Handa
2026-05-21 13:06 ` [PATCH v3] kcov: move kcov_remote_data to task_struct for RT and remove local_lock Alexander Potapenko
3 siblings, 1 reply; 29+ messages in thread
From: Sebastian Andrzej Siewior @ 2026-05-21 8:38 UTC (permalink / raw)
To: Tetsuo Handa
Cc: kasan-dev, LKML, Dmitry Vyukov, Alexander Potapenko, Alan Stern,
Andrew Morton, Andrey Konovalov, Andrey Konovalov,
Clark Williams, Greg Kroah-Hartman, Marco Elver
On 2026-05-09 17:09:44 [+0900], Tetsuo Handa wrote:
> 3. In non-RT kernels, softirqs do not nest, so no concurrent access to
> per-CPU data occurs between softirqs.
> 4. In RT kernels, while softirqs can be preempted, this patch moves the
> KCOV state from per-CPU variables to task_struct (per-task), eliminating
> the contention on shared per-CPU resources.
This is correct: The softirq handling can be preempted _and_ another
softirq can be handled. softirq handling is still per-task. However this
could mess up the per-CPU data if it is not meant to be shared by two
tasks doing softirq in parallel.
> By moving kcov_remote_data to task_struct for RT kernels and replacing
> the in_task() check with !in_serving_softirq(), we ensure consistent
This I don't get. in_task() returns only true if it is a pure
task-context: no hardirq, or softirq handling. in_serving_softirq()
returns true if softirqs are handled but also if the current context got
interrupted by a hardirq _while_ softirqs were served. That is why there
is the in_softirq_really() to limit it just the softirq-only case.
> context detection. Since the data is now isolated per-task and not accessed
> by hardirqs, the local_lock (and the original local_irq_save) is no longer
> necessary and is removed to reduce overhead.
>
> Changes:
>
> * Move remote coverage state from kcov_percpu_data to task_struct under
> CONFIG_PREEMPT_RT.
> * Replace in_task() with !in_serving_softirq() in kcov_remote_start/stop()
> for accurate context tracking.
Doesn't this exclude softirq handling and allows hardirq/ nmi?
> Only compile tested. I don't have environment to measure how not
> preallocating CONFIG_KCOV_IRQ_AREA_SIZE bytes of buffers and
> add to the global pool in kcov_init() impacts reliability / performance.
> Please be sure to test this patch using CONFIG_PREEMPT_RT=y and
> CONFIG_PREEMPT_RT=n kernels in local syzkaller environment before
> sending upstream.
>
> This patch is expected to address various KCOV related reports.
> But I don't add Closes: lines because we can't tell whether this patch
> alone is sufficient for marking as Closes:. We will find the answer
> some time after being sent to upstream.
>
> https://syzkaller.appspot.com/bug?extid=90984d3713722683112e
> https://syzkaller.appspot.com/bug?extid=47cf95ca1f9dcca872c8
> https://syzkaller.appspot.com/bug?extid=8a173e13208949931dc7
> https://syzkaller.appspot.com/bug?extid=3f51ad7ac3ae57a6fdcc
> https://syzkaller.appspot.com/bug?extid=e6686317bd9fe911591a
None of these have a reproducer which is unfortunate. The parallel
softirq handling is only possible since commit 3253cb49cbad4 ("softirq:
Allow to drop the softirq-BKL lock on PREEMPT_RT") which is v6.18-rc1.
This is true most of these reports since they are from 2026 except for
two in the 3f51ad7ac3ae57a6fdcc report. But this might be another issue
which got fixed since then.
If handling softirqs in parallel might be indeed a reasonable solution.
You would need to use the per-task area instead per-CPU for this in the
softirq case on PREEMPT_RT. But the other checks, such as in_task()
work as intended on both.
Sebastian
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3] kcov: move kcov_remote_data to task_struct for RT and remove local_lock
2026-05-09 8:09 ` [PATCH v3] kcov: move kcov_remote_data to task_struct for RT and remove local_lock Tetsuo Handa
` (2 preceding siblings ...)
2026-05-21 8:38 ` Sebastian Andrzej Siewior
@ 2026-05-21 13:06 ` Alexander Potapenko
2026-05-21 14:39 ` Tetsuo Handa
3 siblings, 1 reply; 29+ messages in thread
From: Alexander Potapenko @ 2026-05-21 13:06 UTC (permalink / raw)
To: Tetsuo Handa
Cc: kasan-dev, LKML, Dmitry Vyukov, Alan Stern, Andrew Morton,
Andrey Konovalov, Andrey Konovalov, Clark Williams,
Greg Kroah-Hartman, Marco Elver, Sebastian Andrzej Siewior
I think a deadlock is possible if a softirq is delivered while a
background task is executing kcov_remote_start():
> void kcov_remote_start(u64 handle)
> {
> @@ -867,43 +897,35 @@ void kcov_remote_start(u64 handle)
> void *area;
> unsigned int size;
> int sequence;
> - unsigned long flags;
>
> - if (WARN_ON(!kcov_check_handle(handle, true, true, true)))
> + /* Don't use in_task() in order to allow consistent checks in RT kernels. */
> + if (in_hardirq() || in_nmi())
> return;
> - if (!in_task() && !in_softirq_really())
> + if (WARN_ON(!kcov_check_handle(handle, true, true, true)))
> return;
>
> - local_lock_irqsave(&kcov_percpu_data.lock, flags);
> -
Removing this lock allows softirqs on the current task (that task
could be e.g. a background task in vhost_kthread_worker_create())
> /*
> * Check that kcov_remote_start() is not called twice in background
> * threads nor called by user tasks (with enabled kcov).
> */
> - mode = READ_ONCE(t->kcov_mode);
> - if (WARN_ON(in_task() && kcov_mode_enabled(mode))) {
> - local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
> + if (WARN_ON(!in_serving_softirq() && kcov_mode_enabled(READ_ONCE(t->kcov_mode))))
> return;
> - }
> /*
> * Check that kcov_remote_start() is not called twice in softirqs.
> * Note, that kcov_remote_start() can be called from a softirq that
> * happened while collecting coverage from a background thread.
> */
> - if (WARN_ON(in_serving_softirq() && t->kcov_softirq)) {
> - local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
> + if (WARN_ON(in_serving_softirq() && t->kcov_softirq))
> return;
> - }
>
> spin_lock(&kcov_remote_lock);
A softirq may kick in here and start executing e.g.
__usb_hcd_giveback_urb(), which calls kcov_remote_start() in a nested
fashion.
in_hardirq() and in_nmi() is now false, and in_serving_softirq() is
true, so we will slide past all the WARN_ON checks.
This includes `WARN_ON(in_serving_softirq() && t->kcov_softirq)`,
because we haven't set `t->kcov_softirq` yet.
As a result, the task will attempt to grab `kcov_remote_lock` recursively.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3] kcov: move kcov_remote_data to task_struct for RT and remove local_lock
2026-05-21 13:06 ` [PATCH v3] kcov: move kcov_remote_data to task_struct for RT and remove local_lock Alexander Potapenko
@ 2026-05-21 14:39 ` Tetsuo Handa
0 siblings, 0 replies; 29+ messages in thread
From: Tetsuo Handa @ 2026-05-21 14:39 UTC (permalink / raw)
To: Alexander Potapenko
Cc: kasan-dev, LKML, Dmitry Vyukov, Alan Stern, Andrew Morton,
Andrey Konovalov, Andrey Konovalov, Clark Williams,
Greg Kroah-Hartman, Marco Elver, Sebastian Andrzej Siewior
On 2026/05/21 22:06, Alexander Potapenko wrote:
> I think a deadlock is possible if a softirq is delivered while a
> background task is executing kcov_remote_start():
Indeed. Since kcov_remote_start() is called by
static int vhost_run_work_kthread_list(void *data)
{
kcov_remote_start_common(worker->kcov_handle);
work->fn(work);
kcov_remote_stop();
}
and it can be preempted by
static void usb_giveback_urb_bh(struct work_struct *work)
{
static void __usb_hcd_giveback_urb(struct urb *urb)
{
kcov_remote_start_usb_softirq((u64)urb->dev->bus->busnum);
urb->complete(urb);
kcov_remote_stop_softirq();
}
}
, we need to use _irq{save,restore} version. Any other concerns?
kernel/kcov.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/kernel/kcov.c b/kernel/kcov.c
index 965c11a75b36..70ce99c3205d 100644
--- a/kernel/kcov.c
+++ b/kernel/kcov.c
@@ -897,6 +897,7 @@ void kcov_remote_start(u64 handle)
void *area;
unsigned int size;
int sequence;
+ unsigned long flags;
/* Don't use in_task() in order to allow consistent checks in RT kernels. */
if (in_hardirq() || in_nmi())
@@ -918,10 +919,10 @@ void kcov_remote_start(u64 handle)
if (WARN_ON(in_serving_softirq() && t->kcov_softirq))
return;
- spin_lock(&kcov_remote_lock);
+ spin_lock_irqsave(&kcov_remote_lock, flags);
remote = kcov_remote_find(handle);
if (!remote) {
- spin_unlock(&kcov_remote_lock);
+ spin_unlock_irqrestore(&kcov_remote_lock, flags);
return;
}
kcov_debug("handle = %llx, context: %s\n", handle,
@@ -949,7 +950,7 @@ void kcov_remote_start(u64 handle)
area = this_cpu_ptr(&kcov_percpu_data)->irq_area;
}
#endif
- spin_unlock(&kcov_remote_lock);
+ spin_unlock_irqrestore(&kcov_remote_lock, flags);
/* Can only happen when CONFIG_PREEMPT_RT=y or in_task(). */
if (!area) {
@@ -1044,6 +1045,7 @@ void kcov_remote_stop(void)
void *area;
unsigned int size;
int sequence;
+ unsigned long flags;
/* Don't use in_task() in order to allow consistent checks in RT kernels. */
if (in_hardirq() || in_nmi())
@@ -1084,9 +1086,9 @@ void kcov_remote_stop(void)
spin_unlock(&kcov->lock);
if (IS_ENABLED(CONFIG_PREEMPT_RT) || in_task()) {
- spin_lock(&kcov_remote_lock);
+ spin_lock_irqsave(&kcov_remote_lock, flags);
kcov_remote_area_put(area, size);
- spin_unlock(&kcov_remote_lock);
+ spin_unlock_irqrestore(&kcov_remote_lock, flags);
}
/* Get in kcov_remote_start(). */
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3] kcov: move kcov_remote_data to task_struct for RT and remove local_lock
2026-05-21 8:38 ` Sebastian Andrzej Siewior
@ 2026-05-21 15:28 ` Tetsuo Handa
2026-05-21 18:14 ` Sebastian Andrzej Siewior
0 siblings, 1 reply; 29+ messages in thread
From: Tetsuo Handa @ 2026-05-21 15:28 UTC (permalink / raw)
To: Sebastian Andrzej Siewior
Cc: kasan-dev, LKML, Dmitry Vyukov, Alexander Potapenko, Alan Stern,
Andrew Morton, Andrey Konovalov, Andrey Konovalov,
Clark Williams, Greg Kroah-Hartman, Marco Elver
On 2026/05/21 17:38, Sebastian Andrzej Siewior wrote:
>> By moving kcov_remote_data to task_struct for RT kernels and replacing
>> the in_task() check with !in_serving_softirq(), we ensure consistent
>
> This I don't get. in_task() returns only true if it is a pure
> task-context: no hardirq, or softirq handling. in_serving_softirq()
> returns true if softirqs are handled but also if the current context got
> interrupted by a hardirq _while_ softirqs were served. That is why there
> is the in_softirq_really() to limit it just the softirq-only case.
This patch already excludes hardirq / nmi contexts at the beginning of
kcov_remote_start()/kcov_remote_stop(). Therefore, only task / softirq
contexts can reach the location in question.
>
>> context detection. Since the data is now isolated per-task and not accessed
>> by hardirqs, the local_lock (and the original local_irq_save) is no longer
>> necessary and is removed to reduce overhead.
>>
>> Changes:
>>
>> * Move remote coverage state from kcov_percpu_data to task_struct under
>> CONFIG_PREEMPT_RT.
>> * Replace in_task() with !in_serving_softirq() in kcov_remote_start/stop()
>> for accurate context tracking.
>
> Doesn't this exclude softirq handling and allows hardirq/ nmi?
Ditto.
> If handling softirqs in parallel might be indeed a reasonable solution.
> You would need to use the per-task area instead per-CPU for this in the
> softirq case on PREEMPT_RT. But the other checks, such as in_task()
> work as intended on both.
https://share.google/aimode/PveuuU5od6d1lAwit (expires in 7 days) says
Non-PREEMPT_RT Kernel: in_task() returns false while executing a softirq.
PREEMPT_RT Kernel: in_task() returns true while executing a softirq (unless
nested inside a hard interrupt or NMI).
Is this explanation wrong?
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3] kcov: move kcov_remote_data to task_struct for RT and remove local_lock
2026-05-21 15:28 ` Tetsuo Handa
@ 2026-05-21 18:14 ` Sebastian Andrzej Siewior
2026-05-22 2:39 ` Tetsuo Handa
0 siblings, 1 reply; 29+ messages in thread
From: Sebastian Andrzej Siewior @ 2026-05-21 18:14 UTC (permalink / raw)
To: Tetsuo Handa
Cc: kasan-dev, LKML, Dmitry Vyukov, Alexander Potapenko, Alan Stern,
Andrew Morton, Andrey Konovalov, Andrey Konovalov,
Clark Williams, Greg Kroah-Hartman, Marco Elver
On 2026-05-22 00:28:26 [+0900], Tetsuo Handa wrote:
> > If handling softirqs in parallel might be indeed a reasonable solution.
> > You would need to use the per-task area instead per-CPU for this in the
> > softirq case on PREEMPT_RT. But the other checks, such as in_task()
> > work as intended on both.
>
> https://share.google/aimode/PveuuU5od6d1lAwit (expires in 7 days) says
>
> Non-PREEMPT_RT Kernel: in_task() returns false while executing a softirq.
>
> PREEMPT_RT Kernel: in_task() returns true while executing a softirq (unless
> nested inside a hard interrupt or NMI).
>
> Is this explanation wrong?
Yes. If a softirq is executed (in_serving_softirq() returns true) then
in_task() returns false. On PREEMPT_RT and on non-PREEMPT_RT kernels. It
is irrelevant if the softirq is executed at the end of an interrupt
handler and within ksoftirqd.
Sebastian
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3] kcov: move kcov_remote_data to task_struct for RT and remove local_lock
2026-05-21 18:14 ` Sebastian Andrzej Siewior
@ 2026-05-22 2:39 ` Tetsuo Handa
2026-05-22 11:10 ` [PATCH v4] kcov: move kcov_remote_data to task_struct " Tetsuo Handa
0 siblings, 1 reply; 29+ messages in thread
From: Tetsuo Handa @ 2026-05-22 2:39 UTC (permalink / raw)
To: Sebastian Andrzej Siewior, Alexander Potapenko, Dmitry Vyukov
Cc: kasan-dev, LKML, Alan Stern, Andrew Morton, Andrey Konovalov,
Andrey Konovalov, Clark Williams, Greg Kroah-Hartman,
Marco Elver
On 2026/05/22 3:14, Sebastian Andrzej Siewior wrote:
> On 2026-05-22 00:28:26 [+0900], Tetsuo Handa wrote:
>>> If handling softirqs in parallel might be indeed a reasonable solution.
>>> You would need to use the per-task area instead per-CPU for this in the
>>> softirq case on PREEMPT_RT. But the other checks, such as in_task()
>>> work as intended on both.
>>
>> https://share.google/aimode/PveuuU5od6d1lAwit (expires in 7 days) says
>>
>> Non-PREEMPT_RT Kernel: in_task() returns false while executing a softirq.
>>
>> PREEMPT_RT Kernel: in_task() returns true while executing a softirq (unless
>> nested inside a hard interrupt or NMI).
>>
>> Is this explanation wrong?
>
> Yes. If a softirq is executed (in_serving_softirq() returns true) then
> in_task() returns false. On PREEMPT_RT and on non-PREEMPT_RT kernels. It
> is irrelevant if the softirq is executed at the end of an interrupt
> handler and within ksoftirqd.
OK. Google AI mode admitted that this explanation was wrong.
Also, I heard from https://share.google/aimode/olzr3v2VEBJXepuiN that
I should not change in_*() checks.
Then, combining
(1) "Is it possible to unify the logic between RT and non-RT kernels (not have any ifdef's)?"
from Dmitry Vyukov
(2) "I think a deadlock is possible if a softirq is delivered while a background task is
executing kcov_remote_start():" from Alexander Potapenko
(3) "This I don't get. in_task() returns only true if it is a pure task-context:" from
Sebastian Andrzej Siewior
comments, the updated patch would look like below diff?
(I added "range 128 16777216" check to CONFIG_KCOV_IRQ_AREA_SIZE value, for size of buffer
has to be larger than sizeof(struct kcov_remote_area) bytes.)
include/linux/sched.h | 8 +++
kernel/kcov.c | 111 ++++++++++++++----------------------------
lib/Kconfig.debug | 5 +-
3 files changed, 47 insertions(+), 77 deletions(-)
diff --git a/include/linux/sched.h b/include/linux/sched.h
index ee06cba5c6f5..eaf975138359 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1525,6 +1525,14 @@ struct task_struct {
/* Collect coverage from softirq context: */
unsigned int kcov_softirq;
+
+ /* Temporary storage for preempting remote coverage collection: */
+ unsigned int kcov_saved_mode;
+ unsigned int kcov_saved_size;
+ void *kcov_saved_area;
+ struct kcov *kcov_saved_kcov;
+ int kcov_saved_sequence;
+
#endif
#ifdef CONFIG_MEMCG_V1
diff --git a/kernel/kcov.c b/kernel/kcov.c
index 0b369e88c7c9..3b0444ef8588 100644
--- a/kernel/kcov.c
+++ b/kernel/kcov.c
@@ -88,21 +88,6 @@ static DEFINE_SPINLOCK(kcov_remote_lock);
static DEFINE_HASHTABLE(kcov_remote_map, 4);
static struct list_head kcov_remote_areas = LIST_HEAD_INIT(kcov_remote_areas);
-struct kcov_percpu_data {
- void *irq_area;
- local_lock_t lock;
-
- unsigned int saved_mode;
- unsigned int saved_size;
- void *saved_area;
- struct kcov *saved_kcov;
- int saved_sequence;
-};
-
-static DEFINE_PER_CPU(struct kcov_percpu_data, kcov_percpu_data) = {
- .lock = INIT_LOCAL_LOCK(lock),
-};
-
/* Must be called with kcov_remote_lock locked. */
static struct kcov_remote *kcov_remote_find(u64 handle)
{
@@ -824,37 +809,32 @@ static inline bool kcov_mode_enabled(unsigned int mode)
}
static void kcov_remote_softirq_start(struct task_struct *t)
- __must_hold(&kcov_percpu_data.lock)
{
- struct kcov_percpu_data *data = this_cpu_ptr(&kcov_percpu_data);
unsigned int mode;
mode = READ_ONCE(t->kcov_mode);
barrier();
if (kcov_mode_enabled(mode)) {
- data->saved_mode = mode;
- data->saved_size = t->kcov_size;
- data->saved_area = t->kcov_area;
- data->saved_sequence = t->kcov_sequence;
- data->saved_kcov = t->kcov;
+ t->kcov_saved_mode = mode;
+ t->kcov_saved_size = t->kcov_size;
+ t->kcov_saved_area = t->kcov_area;
+ t->kcov_saved_sequence = t->kcov_sequence;
+ t->kcov_saved_kcov = t->kcov;
kcov_stop(t);
}
}
static void kcov_remote_softirq_stop(struct task_struct *t)
- __must_hold(&kcov_percpu_data.lock)
{
- struct kcov_percpu_data *data = this_cpu_ptr(&kcov_percpu_data);
-
- if (data->saved_kcov) {
- kcov_start(t, data->saved_kcov, data->saved_size,
- data->saved_area, data->saved_mode,
- data->saved_sequence);
- data->saved_mode = 0;
- data->saved_size = 0;
- data->saved_area = NULL;
- data->saved_sequence = 0;
- data->saved_kcov = NULL;
+ if (t->kcov_saved_kcov) {
+ kcov_start(t, t->kcov_saved_kcov, t->kcov_saved_size,
+ t->kcov_saved_area, t->kcov_saved_mode,
+ t->kcov_saved_sequence);
+ t->kcov_saved_mode = 0;
+ t->kcov_saved_size = 0;
+ t->kcov_saved_area = NULL;
+ t->kcov_saved_sequence = 0;
+ t->kcov_saved_kcov = NULL;
}
}
@@ -874,15 +854,12 @@ void kcov_remote_start(u64 handle)
if (!in_task() && !in_softirq_really())
return;
- local_lock_irqsave(&kcov_percpu_data.lock, flags);
-
/*
* Check that kcov_remote_start() is not called twice in background
* threads nor called by user tasks (with enabled kcov).
*/
mode = READ_ONCE(t->kcov_mode);
if (WARN_ON(in_task() && kcov_mode_enabled(mode))) {
- local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
return;
}
/*
@@ -891,15 +868,13 @@ void kcov_remote_start(u64 handle)
* happened while collecting coverage from a background thread.
*/
if (WARN_ON(in_serving_softirq() && t->kcov_softirq)) {
- local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
return;
}
- spin_lock(&kcov_remote_lock);
+ spin_lock_irqsave(&kcov_remote_lock, flags);
remote = kcov_remote_find(handle);
if (!remote) {
- spin_unlock(&kcov_remote_lock);
- local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
+ spin_unlock_irqrestore(&kcov_remote_lock, flags);
return;
}
kcov_debug("handle = %llx, context: %s\n", handle,
@@ -915,24 +890,17 @@ void kcov_remote_start(u64 handle)
*/
mode = context_unsafe(kcov->mode);
sequence = kcov->sequence;
- if (in_task()) {
- size = kcov->remote_size;
- area = kcov_remote_area_get(size);
- } else {
- size = CONFIG_KCOV_IRQ_AREA_SIZE;
- area = this_cpu_ptr(&kcov_percpu_data)->irq_area;
- }
- spin_unlock(&kcov_remote_lock);
+ size = in_task() ? kcov->remote_size : CONFIG_KCOV_IRQ_AREA_SIZE;
+ area = kcov_remote_area_get(size);
+ spin_unlock_irqrestore(&kcov_remote_lock, flags);
- /* Can only happen when in_task(). */
- if (!area) {
- local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
+ /* Allocate new buffer if we can sleep. */
+ if (!area && in_task()) {
area = vmalloc(size * sizeof(unsigned long));
if (!area) {
kcov_put(kcov);
return;
}
- local_lock_irqsave(&kcov_percpu_data.lock, flags);
}
/* Reset coverage size. */
@@ -943,9 +911,6 @@ void kcov_remote_start(u64 handle)
t->kcov_softirq = 1;
}
kcov_start(t, kcov, size, area, mode, sequence);
-
- local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
-
}
EXPORT_SYMBOL(kcov_remote_start);
@@ -1027,12 +992,9 @@ void kcov_remote_stop(void)
if (!in_task() && !in_softirq_really())
return;
- local_lock_irqsave(&kcov_percpu_data.lock, flags);
-
mode = READ_ONCE(t->kcov_mode);
barrier();
if (!kcov_mode_enabled(mode)) {
- local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
return;
}
/*
@@ -1040,12 +1002,10 @@ void kcov_remote_stop(void)
* actually found the remote handle and started collecting coverage.
*/
if (in_serving_softirq() && !t->kcov_softirq) {
- local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
return;
}
/* Make sure that kcov_softirq is only set when in softirq. */
if (WARN_ON(!in_serving_softirq() && t->kcov_softirq)) {
- local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
return;
}
@@ -1069,13 +1029,9 @@ void kcov_remote_stop(void)
kcov_move_area(kcov->mode, kcov->area, kcov->size, area);
spin_unlock(&kcov->lock);
- if (in_task()) {
- spin_lock(&kcov_remote_lock);
- kcov_remote_area_put(area, size);
- spin_unlock(&kcov_remote_lock);
- }
-
- local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
+ spin_lock_irqsave(&kcov_remote_lock, flags);
+ kcov_remote_area_put(area, size);
+ spin_unlock_irqrestore(&kcov_remote_lock, flags);
/* Get in kcov_remote_start(). */
kcov_put(kcov);
@@ -1119,14 +1075,19 @@ static void __init selftest(void)
static int __init kcov_init(void)
{
- int cpu;
+ int i, cpu = num_possible_cpus();
+ unsigned long flags;
- for_each_possible_cpu(cpu) {
- void *area = vmalloc_node(CONFIG_KCOV_IRQ_AREA_SIZE *
- sizeof(unsigned long), cpu_to_node(cpu));
- if (!area)
- return -ENOMEM;
- per_cpu_ptr(&kcov_percpu_data, cpu)->irq_area = area;
+#ifdef CONFIG_PREEMPT_RT
+ /* Allocate some extra buffers in order to prepare for softirq preemption. */
+ cpu = cpu >= 4 ? cpu * 2 : cpu + 4;
+#endif
+ for (i = 0; i < cpu; i++) {
+ struct kcov_remote_area *area =
+ vmalloc(CONFIG_KCOV_IRQ_AREA_SIZE * sizeof(unsigned long));
+ spin_lock_irqsave(&kcov_remote_lock, flags);
+ kcov_remote_area_put(area, CONFIG_KCOV_IRQ_AREA_SIZE);
+ spin_unlock_irqrestore(&kcov_remote_lock, flags);
}
/*
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 8ff5adcfe1e0..1f3e917dbac7 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -2247,10 +2247,11 @@ config KCOV_INSTRUMENT_ALL
config KCOV_IRQ_AREA_SIZE
hex "Size of interrupt coverage collection area in words"
depends on KCOV
+ range 128 16777216
default 0x40000
help
- KCOV uses preallocated per-cpu areas to collect coverage from
- soft interrupts. This specifies the size of those areas in the
+ KCOV uses preallocated areas to collect coverage from soft
+ interrupts. This specifies the size of those areas in the
number of unsigned long words.
config KCOV_SELFTEST
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v4] kcov: move kcov_remote_data to task_struct and remove local_lock
2026-05-22 2:39 ` Tetsuo Handa
@ 2026-05-22 11:10 ` Tetsuo Handa
2026-05-22 13:27 ` Sebastian Andrzej Siewior
2026-05-29 6:35 ` Tetsuo Handa
0 siblings, 2 replies; 29+ messages in thread
From: Tetsuo Handa @ 2026-05-22 11:10 UTC (permalink / raw)
To: Sebastian Andrzej Siewior, Alexander Potapenko, Dmitry Vyukov
Cc: kasan-dev, LKML, Alan Stern, Andrew Morton, Andrey Konovalov,
Andrey Konovalov, Clark Williams, Greg Kroah-Hartman,
Marco Elver
Softirq handling can be preempted in RT kernels. If we use per-CPU buffers
for saving/restoring temporary KCOV contexts, various kcov-related warnings
happen due to state corruption.
This patch unconditionally moves temporary KCOV context buffers from
per-CPU to per-task. Although it is safe to continue using per-CPU buffers
in !RT kernels, this patch tries to unify the logic, for Dmitry Vyukov
commented
Is it possible to unify the logic between RT and non-RT kernels (not
have any ifdef's)?
The logic is already rather complicated, having 2 work modes, and
limited testing will be problematic in the future.
.
The local_lock_t is removed, for per-CPU buffers are removed.
But kcov_remote_lock and _irqsave/irqrestore remain in order to serialize
access to kcov_remote_areas lists, for kcov_remote_{start,stop}() are
called from both task context and softirq context.
Analyzed-by: AI Mode in Google Search (no mail address)
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Fixes: 5ff3b30ab57d ("kcov: collect coverage from interrupts")
---
Only compile tested.
include/linux/sched.h | 8 +++
kernel/kcov.c | 111 +++++++++++++++---------------------------
lib/Kconfig.debug | 5 +-
3 files changed, 51 insertions(+), 73 deletions(-)
diff --git a/include/linux/sched.h b/include/linux/sched.h
index ee06cba5c6f5..eaf975138359 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1525,6 +1525,14 @@ struct task_struct {
/* Collect coverage from softirq context: */
unsigned int kcov_softirq;
+
+ /* Temporary storage for preempting remote coverage collection: */
+ unsigned int kcov_saved_mode;
+ unsigned int kcov_saved_size;
+ void *kcov_saved_area;
+ struct kcov *kcov_saved_kcov;
+ int kcov_saved_sequence;
+
#endif
#ifdef CONFIG_MEMCG_V1
diff --git a/kernel/kcov.c b/kernel/kcov.c
index 0b369e88c7c9..b96d6ad68f92 100644
--- a/kernel/kcov.c
+++ b/kernel/kcov.c
@@ -86,21 +86,8 @@ struct kcov_remote {
static DEFINE_SPINLOCK(kcov_remote_lock);
static DEFINE_HASHTABLE(kcov_remote_map, 4);
-static struct list_head kcov_remote_areas = LIST_HEAD_INIT(kcov_remote_areas);
-
-struct kcov_percpu_data {
- void *irq_area;
- local_lock_t lock;
-
- unsigned int saved_mode;
- unsigned int saved_size;
- void *saved_area;
- struct kcov *saved_kcov;
- int saved_sequence;
-};
-
-static DEFINE_PER_CPU(struct kcov_percpu_data, kcov_percpu_data) = {
- .lock = INIT_LOCAL_LOCK(lock),
+static struct list_head kcov_remote_areas[2] = {
+ LIST_HEAD_INIT(kcov_remote_areas[0]), LIST_HEAD_INIT(kcov_remote_areas[1])
};
/* Must be called with kcov_remote_lock locked. */
@@ -136,8 +123,9 @@ static struct kcov_remote_area *kcov_remote_area_get(unsigned int size)
{
struct kcov_remote_area *area;
struct list_head *pos;
+ struct list_head *list = &kcov_remote_areas[size == CONFIG_KCOV_IRQ_AREA_SIZE];
- list_for_each(pos, &kcov_remote_areas) {
+ list_for_each(pos, list) {
area = list_entry(pos, struct kcov_remote_area, list);
if (area->size == size) {
list_del(&area->list);
@@ -153,7 +141,7 @@ static void kcov_remote_area_put(struct kcov_remote_area *area,
{
INIT_LIST_HEAD(&area->list);
area->size = size;
- list_add(&area->list, &kcov_remote_areas);
+ list_add(&area->list, &kcov_remote_areas[size == CONFIG_KCOV_IRQ_AREA_SIZE]);
/*
* KMSAN doesn't instrument this file, so it may not know area->list
* is initialized. Unpoison it explicitly to avoid reports in
@@ -824,37 +812,32 @@ static inline bool kcov_mode_enabled(unsigned int mode)
}
static void kcov_remote_softirq_start(struct task_struct *t)
- __must_hold(&kcov_percpu_data.lock)
{
- struct kcov_percpu_data *data = this_cpu_ptr(&kcov_percpu_data);
unsigned int mode;
mode = READ_ONCE(t->kcov_mode);
barrier();
if (kcov_mode_enabled(mode)) {
- data->saved_mode = mode;
- data->saved_size = t->kcov_size;
- data->saved_area = t->kcov_area;
- data->saved_sequence = t->kcov_sequence;
- data->saved_kcov = t->kcov;
+ t->kcov_saved_mode = mode;
+ t->kcov_saved_size = t->kcov_size;
+ t->kcov_saved_area = t->kcov_area;
+ t->kcov_saved_sequence = t->kcov_sequence;
+ t->kcov_saved_kcov = t->kcov;
kcov_stop(t);
}
}
static void kcov_remote_softirq_stop(struct task_struct *t)
- __must_hold(&kcov_percpu_data.lock)
{
- struct kcov_percpu_data *data = this_cpu_ptr(&kcov_percpu_data);
-
- if (data->saved_kcov) {
- kcov_start(t, data->saved_kcov, data->saved_size,
- data->saved_area, data->saved_mode,
- data->saved_sequence);
- data->saved_mode = 0;
- data->saved_size = 0;
- data->saved_area = NULL;
- data->saved_sequence = 0;
- data->saved_kcov = NULL;
+ if (t->kcov_saved_kcov) {
+ kcov_start(t, t->kcov_saved_kcov, t->kcov_saved_size,
+ t->kcov_saved_area, t->kcov_saved_mode,
+ t->kcov_saved_sequence);
+ t->kcov_saved_mode = 0;
+ t->kcov_saved_size = 0;
+ t->kcov_saved_area = NULL;
+ t->kcov_saved_sequence = 0;
+ t->kcov_saved_kcov = NULL;
}
}
@@ -874,15 +857,12 @@ void kcov_remote_start(u64 handle)
if (!in_task() && !in_softirq_really())
return;
- local_lock_irqsave(&kcov_percpu_data.lock, flags);
-
/*
* Check that kcov_remote_start() is not called twice in background
* threads nor called by user tasks (with enabled kcov).
*/
mode = READ_ONCE(t->kcov_mode);
if (WARN_ON(in_task() && kcov_mode_enabled(mode))) {
- local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
return;
}
/*
@@ -891,15 +871,13 @@ void kcov_remote_start(u64 handle)
* happened while collecting coverage from a background thread.
*/
if (WARN_ON(in_serving_softirq() && t->kcov_softirq)) {
- local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
return;
}
- spin_lock(&kcov_remote_lock);
+ spin_lock_irqsave(&kcov_remote_lock, flags);
remote = kcov_remote_find(handle);
if (!remote) {
- spin_unlock(&kcov_remote_lock);
- local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
+ spin_unlock_irqrestore(&kcov_remote_lock, flags);
return;
}
kcov_debug("handle = %llx, context: %s\n", handle,
@@ -920,19 +898,17 @@ void kcov_remote_start(u64 handle)
area = kcov_remote_area_get(size);
} else {
size = CONFIG_KCOV_IRQ_AREA_SIZE;
- area = this_cpu_ptr(&kcov_percpu_data)->irq_area;
+ area = kcov_remote_area_get(size);
}
- spin_unlock(&kcov_remote_lock);
+ spin_unlock_irqrestore(&kcov_remote_lock, flags);
- /* Can only happen when in_task(). */
+ /* Allocate new buffer if we can sleep. */
if (!area) {
- local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
- area = vmalloc(size * sizeof(unsigned long));
+ area = in_task() ? vmalloc(size * sizeof(unsigned long)) : NULL;
if (!area) {
kcov_put(kcov);
return;
}
- local_lock_irqsave(&kcov_percpu_data.lock, flags);
}
/* Reset coverage size. */
@@ -943,9 +919,6 @@ void kcov_remote_start(u64 handle)
t->kcov_softirq = 1;
}
kcov_start(t, kcov, size, area, mode, sequence);
-
- local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
-
}
EXPORT_SYMBOL(kcov_remote_start);
@@ -1027,12 +1000,9 @@ void kcov_remote_stop(void)
if (!in_task() && !in_softirq_really())
return;
- local_lock_irqsave(&kcov_percpu_data.lock, flags);
-
mode = READ_ONCE(t->kcov_mode);
barrier();
if (!kcov_mode_enabled(mode)) {
- local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
return;
}
/*
@@ -1040,12 +1010,10 @@ void kcov_remote_stop(void)
* actually found the remote handle and started collecting coverage.
*/
if (in_serving_softirq() && !t->kcov_softirq) {
- local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
return;
}
/* Make sure that kcov_softirq is only set when in softirq. */
if (WARN_ON(!in_serving_softirq() && t->kcov_softirq)) {
- local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
return;
}
@@ -1069,13 +1037,9 @@ void kcov_remote_stop(void)
kcov_move_area(kcov->mode, kcov->area, kcov->size, area);
spin_unlock(&kcov->lock);
- if (in_task()) {
- spin_lock(&kcov_remote_lock);
- kcov_remote_area_put(area, size);
- spin_unlock(&kcov_remote_lock);
- }
-
- local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
+ spin_lock_irqsave(&kcov_remote_lock, flags);
+ kcov_remote_area_put(area, size);
+ spin_unlock_irqrestore(&kcov_remote_lock, flags);
/* Get in kcov_remote_start(). */
kcov_put(kcov);
@@ -1119,14 +1083,19 @@ static void __init selftest(void)
static int __init kcov_init(void)
{
- int cpu;
+ int cpu = num_possible_cpus();
- for_each_possible_cpu(cpu) {
- void *area = vmalloc_node(CONFIG_KCOV_IRQ_AREA_SIZE *
- sizeof(unsigned long), cpu_to_node(cpu));
- if (!area)
- return -ENOMEM;
- per_cpu_ptr(&kcov_percpu_data, cpu)->irq_area = area;
+#ifdef CONFIG_PREEMPT_RT
+ /* Allocate some extra buffers in order to prepare for softirq preemption. */
+ cpu = cpu >= 4 ? cpu * 2 : cpu + 4;
+#endif
+ while (cpu--) {
+ void *area = vmalloc(CONFIG_KCOV_IRQ_AREA_SIZE * sizeof(unsigned long));
+ unsigned long flags;
+
+ spin_lock_irqsave(&kcov_remote_lock, flags);
+ kcov_remote_area_put(area, CONFIG_KCOV_IRQ_AREA_SIZE);
+ spin_unlock_irqrestore(&kcov_remote_lock, flags);
}
/*
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 8ff5adcfe1e0..b9e6798dc66d 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -2247,10 +2247,11 @@ config KCOV_INSTRUMENT_ALL
config KCOV_IRQ_AREA_SIZE
hex "Size of interrupt coverage collection area in words"
depends on KCOV
+ range 0x80 0x1000000
default 0x40000
help
- KCOV uses preallocated per-cpu areas to collect coverage from
- soft interrupts. This specifies the size of those areas in the
+ KCOV uses preallocated areas to collect coverage from soft
+ interrupts. This specifies the size of those areas in the
number of unsigned long words.
config KCOV_SELFTEST
--
2.54.0
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v4] kcov: move kcov_remote_data to task_struct and remove local_lock
2026-05-22 11:10 ` [PATCH v4] kcov: move kcov_remote_data to task_struct " Tetsuo Handa
@ 2026-05-22 13:27 ` Sebastian Andrzej Siewior
2026-05-29 6:35 ` Tetsuo Handa
1 sibling, 0 replies; 29+ messages in thread
From: Sebastian Andrzej Siewior @ 2026-05-22 13:27 UTC (permalink / raw)
To: Tetsuo Handa
Cc: Alexander Potapenko, Dmitry Vyukov, kasan-dev, LKML, Alan Stern,
Andrew Morton, Andrey Konovalov, Andrey Konovalov,
Clark Williams, Greg Kroah-Hartman, Marco Elver
On 2026-05-22 20:10:47 [+0900], Tetsuo Handa wrote:
> --- a/kernel/kcov.c
> +++ b/kernel/kcov.c
> @@ -86,21 +86,8 @@ struct kcov_remote {
>
> static DEFINE_SPINLOCK(kcov_remote_lock);
> static DEFINE_HASHTABLE(kcov_remote_map, 4);
> -static struct list_head kcov_remote_areas = LIST_HEAD_INIT(kcov_remote_areas);
> -
> -struct kcov_percpu_data {
> - void *irq_area;
> - local_lock_t lock;
> -
> - unsigned int saved_mode;
> - unsigned int saved_size;
> - void *saved_area;
> - struct kcov *saved_kcov;
> - int saved_sequence;
> -};
> -
> -static DEFINE_PER_CPU(struct kcov_percpu_data, kcov_percpu_data) = {
> - .lock = INIT_LOCAL_LOCK(lock),
> +static struct list_head kcov_remote_areas[2] = {
> + LIST_HEAD_INIT(kcov_remote_areas[0]), LIST_HEAD_INIT(kcov_remote_areas[1])
> };
>
> /* Must be called with kcov_remote_lock locked. */
> @@ -136,8 +123,9 @@ static struct kcov_remote_area *kcov_remote_area_get(unsigned int size)
> {
> struct kcov_remote_area *area;
> struct list_head *pos;
> + struct list_head *list = &kcov_remote_areas[size == CONFIG_KCOV_IRQ_AREA_SIZE];
This gets probably the job done but the two lists and check condition
looks a bit hackish. The size is not really important, it is user-sized
buffer vs softirq-buffer. Having an explicit might not be bad.
> @@ -1119,14 +1083,19 @@ static void __init selftest(void)
>
> static int __init kcov_init(void)
> {
> - int cpu;
> + int cpu = num_possible_cpus();
>
> - for_each_possible_cpu(cpu) {
> - void *area = vmalloc_node(CONFIG_KCOV_IRQ_AREA_SIZE *
> - sizeof(unsigned long), cpu_to_node(cpu));
> - if (!area)
> - return -ENOMEM;
> - per_cpu_ptr(&kcov_percpu_data, cpu)->irq_area = area;
> +#ifdef CONFIG_PREEMPT_RT
> + /* Allocate some extra buffers in order to prepare for softirq preemption. */
> + cpu = cpu >= 4 ? cpu * 2 : cpu + 4;
> +#endif
Maybe IS_ENABLED(). This assumes that we have twice as many tasks in
softirq context as we have CPUs. Is probably good enough.
Are the sizes somehow important? Any reason why there should not be a
"sane" lower size and then fallback to the user-buffer if it runs out of
softirqs buffers?
The user may request multiple sized buffers via KCOV_REMOTE_ENABLE, are
they cleaned up? The question would could the softirq use the user sized
buffers if any.
> + while (cpu--) {
> + void *area = vmalloc(CONFIG_KCOV_IRQ_AREA_SIZE * sizeof(unsigned long));
> + unsigned long flags;
> +
> + spin_lock_irqsave(&kcov_remote_lock, flags);
> + kcov_remote_area_put(area, CONFIG_KCOV_IRQ_AREA_SIZE);
> + spin_unlock_irqrestore(&kcov_remote_lock, flags);
> }
>
> /*
Sebastian
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v4] kcov: move kcov_remote_data to task_struct and remove local_lock
2026-05-22 11:10 ` [PATCH v4] kcov: move kcov_remote_data to task_struct " Tetsuo Handa
2026-05-22 13:27 ` Sebastian Andrzej Siewior
@ 2026-05-29 6:35 ` Tetsuo Handa
2026-06-27 9:58 ` [PATCH v5] kcov: fix data corruption and race conditions on PREEMPT_RT by moving saved remote state to task_struct Tetsuo Handa
1 sibling, 1 reply; 29+ messages in thread
From: Tetsuo Handa @ 2026-05-29 6:35 UTC (permalink / raw)
To: Sebastian Andrzej Siewior, Alexander Potapenko, Dmitry Vyukov
Cc: kasan-dev, LKML, Alan Stern, Andrew Morton, Andrey Konovalov,
Andrey Konovalov, Clark Williams, Greg Kroah-Hartman,
Marco Elver
On 2026/05/22 22:27, Sebastian Andrzej Siewior wrote:
>> @@ -136,8 +123,9 @@ static struct kcov_remote_area *kcov_remote_area_get(unsigned int size)
>> {
>> struct kcov_remote_area *area;
>> struct list_head *pos;
>> + struct list_head *list = &kcov_remote_areas[size == CONFIG_KCOV_IRQ_AREA_SIZE];
>
> This gets probably the job done but the two lists and check condition
> looks a bit hackish. The size is not really important, it is user-sized
> buffer vs softirq-buffer. Having an explicit might not be bad.
We can add a bool argument if we want.
>
> Are the sizes somehow important? Any reason why there should not be a
> "sane" lower size and then fallback to the user-buffer if it runs out of
> softirqs buffers?
This question should be answered by KCOV users.
>
> The user may request multiple sized buffers via KCOV_REMOTE_ENABLE, are
> they cleaned up? The question would could the softirq use the user sized
> buffers if any.
kcov_remote_stop() does not clean up (i.e. caches for future reuse).
But /sys/kernel/debug/kcov is protected as root:root 0600 file.
Dmitry, Sebastian, and Alexander, can we start testing this version?
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v5] kcov: fix data corruption and race conditions on PREEMPT_RT by moving saved remote state to task_struct
2026-05-29 6:35 ` Tetsuo Handa
@ 2026-06-27 9:58 ` Tetsuo Handa
2026-07-07 13:36 ` Tetsuo Handa
2026-07-08 15:28 ` Alexander Potapenko
0 siblings, 2 replies; 29+ messages in thread
From: Tetsuo Handa @ 2026-06-27 9:58 UTC (permalink / raw)
To: Sebastian Andrzej Siewior, Alexander Potapenko, Dmitry Vyukov
Cc: kasan-dev, LKML, Alan Stern, Andrew Morton, Andrey Konovalov,
Andrey Konovalov, Clark Williams, Greg Kroah-Hartman,
Marco Elver
syzbot is reporting KCOV state corruption on PREEMPT_RT kernels, for the
temporary storage used for saving/restoring remote KCOV state is currently
allocated as the per-CPU area.
On PREEMPT_RT kernels, softirq handlers run as preemptible task threads
(e.g., ksoftirqd). If a softirq context preempts a task running a remote
KCOV session, it safely saves the task's state into the per-CPU area.
However, if that softirq thread is subsequently preempted by a higher-
priority softirq thread on the same CPU, the second softirq will overwrite
the same per-CPU area, permanently destroying the original task's KCOV
state.
Fix this data corruption by moving the temporary storage from the per-CPU
area to the per-thread area. Since each softirq thread now owns its own
task context, nested softirq preemption no longer causes data overwrites.
Note that while the temporary storage is now on a per-thread basis, the
per-CPU kcov_percpu_data.lock must be retained, for we need to ensure that
kcov_remote_start() and kcov_remote_stop() operate atomically without
racing against asynchronous interrupts that manipulate the current task's
KCOV state.
Analyzed-by: AI Mode in Google Search (no mail address)
Fixes: 5ff3b30ab57d ("kcov: collect coverage from interrupts")
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
---
Only compile tested. This version preserves kcov_percpu_data.lock which the past
versions attempted to remove ( https://sashiko.dev/#/message/7aff4d71-a6a0-4898-9491-2a3973e9d0cc%40I-love.SAKURA.ne.jp )
and explicitly passes context type instead of relying on buffer size in case
a task context by chance requested buffer size which is equals to the size of
buffers preallocated for the softirq context.
include/linux/sched.h | 8 +++++
kernel/kcov.c | 82 +++++++++++++++++++++----------------------
lib/Kconfig.debug | 5 +--
3 files changed, 51 insertions(+), 44 deletions(-)
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 373bcc0598d1..7a53c15cecb5 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1543,6 +1543,14 @@ struct task_struct {
/* Collect coverage from softirq context: */
unsigned int kcov_softirq;
+
+ /* Temporary storage for preempting remote coverage collection: */
+ unsigned int kcov_saved_mode;
+ unsigned int kcov_saved_size;
+ void *kcov_saved_area;
+ struct kcov *kcov_saved_kcov;
+ int kcov_saved_sequence;
+
#endif
#ifdef CONFIG_MEMCG_V1
diff --git a/kernel/kcov.c b/kernel/kcov.c
index 1df373fb562b..76363a024f00 100644
--- a/kernel/kcov.c
+++ b/kernel/kcov.c
@@ -86,17 +86,12 @@ struct kcov_remote {
static DEFINE_SPINLOCK(kcov_remote_lock);
static DEFINE_HASHTABLE(kcov_remote_map, 4);
-static struct list_head kcov_remote_areas = LIST_HEAD_INIT(kcov_remote_areas);
+static struct list_head kcov_remote_areas[2] = {
+ LIST_HEAD_INIT(kcov_remote_areas[0]), LIST_HEAD_INIT(kcov_remote_areas[1])
+};
struct kcov_percpu_data {
- void *irq_area;
local_lock_t lock;
-
- unsigned int saved_mode;
- unsigned int saved_size;
- void *saved_area;
- struct kcov *saved_kcov;
- int saved_sequence;
};
static DEFINE_PER_CPU(struct kcov_percpu_data, kcov_percpu_data) = {
@@ -132,12 +127,13 @@ static struct kcov_remote *kcov_remote_add(struct kcov *kcov, u64 handle)
}
/* Must be called with kcov_remote_lock locked. */
-static struct kcov_remote_area *kcov_remote_area_get(unsigned int size)
+static struct kcov_remote_area *kcov_remote_area_get(unsigned int size, bool irq)
{
struct kcov_remote_area *area;
struct list_head *pos;
+ struct list_head *list = &kcov_remote_areas[irq];
- list_for_each(pos, &kcov_remote_areas) {
+ list_for_each(pos, list) {
area = list_entry(pos, struct kcov_remote_area, list);
if (area->size == size) {
list_del(&area->list);
@@ -149,11 +145,11 @@ static struct kcov_remote_area *kcov_remote_area_get(unsigned int size)
/* Must be called with kcov_remote_lock locked. */
static void kcov_remote_area_put(struct kcov_remote_area *area,
- unsigned int size)
+ unsigned int size, bool irq)
{
INIT_LIST_HEAD(&area->list);
area->size = size;
- list_add(&area->list, &kcov_remote_areas);
+ list_add(&area->list, &kcov_remote_areas[irq]);
/*
* KMSAN doesn't instrument this file, so it may not know area->list
* is initialized. Unpoison it explicitly to avoid reports in
@@ -836,17 +832,16 @@ static inline bool kcov_mode_enabled(unsigned int mode)
static void kcov_remote_softirq_start(struct task_struct *t)
__must_hold(&kcov_percpu_data.lock)
{
- struct kcov_percpu_data *data = this_cpu_ptr(&kcov_percpu_data);
unsigned int mode;
mode = READ_ONCE(t->kcov_mode);
barrier();
if (kcov_mode_enabled(mode)) {
- data->saved_mode = mode;
- data->saved_size = t->kcov_size;
- data->saved_area = t->kcov_area;
- data->saved_sequence = t->kcov_sequence;
- data->saved_kcov = t->kcov;
+ t->kcov_saved_mode = mode;
+ t->kcov_saved_size = t->kcov_size;
+ t->kcov_saved_area = t->kcov_area;
+ t->kcov_saved_sequence = t->kcov_sequence;
+ t->kcov_saved_kcov = t->kcov;
kcov_stop(t);
}
}
@@ -854,17 +849,15 @@ static void kcov_remote_softirq_start(struct task_struct *t)
static void kcov_remote_softirq_stop(struct task_struct *t)
__must_hold(&kcov_percpu_data.lock)
{
- struct kcov_percpu_data *data = this_cpu_ptr(&kcov_percpu_data);
-
- if (data->saved_kcov) {
- kcov_start(t, data->saved_kcov, data->saved_size,
- data->saved_area, data->saved_mode,
- data->saved_sequence);
- data->saved_mode = 0;
- data->saved_size = 0;
- data->saved_area = NULL;
- data->saved_sequence = 0;
- data->saved_kcov = NULL;
+ if (t->kcov_saved_kcov) {
+ kcov_start(t, t->kcov_saved_kcov, t->kcov_saved_size,
+ t->kcov_saved_area, t->kcov_saved_mode,
+ t->kcov_saved_sequence);
+ t->kcov_saved_mode = 0;
+ t->kcov_saved_size = 0;
+ t->kcov_saved_area = NULL;
+ t->kcov_saved_sequence = 0;
+ t->kcov_saved_kcov = NULL;
}
}
@@ -927,17 +920,17 @@ void kcov_remote_start(u64 handle)
sequence = kcov->sequence;
if (in_task()) {
size = kcov->remote_size;
- area = kcov_remote_area_get(size);
+ area = kcov_remote_area_get(size, false);
} else {
size = CONFIG_KCOV_IRQ_AREA_SIZE;
- area = this_cpu_ptr(&kcov_percpu_data)->irq_area;
+ area = kcov_remote_area_get(size, true);
}
spin_unlock(&kcov_remote_lock);
- /* Can only happen when in_task(). */
+ /* Allocate new buffer if we can sleep. */
if (!area) {
local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
- area = vmalloc(size * sizeof(unsigned long));
+ area = in_task() ? vmalloc(size * sizeof(unsigned long)) : NULL;
if (!area) {
kcov_put(kcov);
return;
@@ -1079,9 +1072,9 @@ void kcov_remote_stop(void)
kcov_move_area(kcov->mode, kcov->area, kcov->size, area);
spin_unlock(&kcov->lock);
- if (in_task()) {
+ if (1) {
spin_lock(&kcov_remote_lock);
- kcov_remote_area_put(area, size);
+ kcov_remote_area_put(area, size, !in_task());
spin_unlock(&kcov_remote_lock);
}
@@ -1129,14 +1122,19 @@ static void __init selftest(void)
static int __init kcov_init(void)
{
- int cpu;
+ int cpu = num_possible_cpus();
- for_each_possible_cpu(cpu) {
- void *area = vmalloc_node(CONFIG_KCOV_IRQ_AREA_SIZE *
- sizeof(unsigned long), cpu_to_node(cpu));
- if (!area)
- return -ENOMEM;
- per_cpu_ptr(&kcov_percpu_data, cpu)->irq_area = area;
+#ifdef CONFIG_PREEMPT_RT
+ /* Allocate some extra buffers in order to prepare for softirq preemption. */
+ cpu = cpu >= 4 ? cpu * 2 : cpu + 4;
+#endif
+ while (cpu--) {
+ void *area = vmalloc(CONFIG_KCOV_IRQ_AREA_SIZE * sizeof(unsigned long));
+ unsigned long flags;
+
+ spin_lock_irqsave(&kcov_remote_lock, flags);
+ kcov_remote_area_put(area, CONFIG_KCOV_IRQ_AREA_SIZE, true);
+ spin_unlock_irqrestore(&kcov_remote_lock, flags);
}
/*
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 1244dcac2294..12786379bf1d 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -2247,10 +2247,11 @@ config KCOV_INSTRUMENT_ALL
config KCOV_IRQ_AREA_SIZE
hex "Size of interrupt coverage collection area in words"
depends on KCOV
+ range 0x80 0x1000000
default 0x40000
help
- KCOV uses preallocated per-cpu areas to collect coverage from
- soft interrupts. This specifies the size of those areas in the
+ KCOV uses preallocated areas to collect coverage from soft
+ interrupts. This specifies the size of those areas in the
number of unsigned long words.
config KCOV_SELFTEST
--
2.52.0
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v5] kcov: fix data corruption and race conditions on PREEMPT_RT by moving saved remote state to task_struct
2026-06-27 9:58 ` [PATCH v5] kcov: fix data corruption and race conditions on PREEMPT_RT by moving saved remote state to task_struct Tetsuo Handa
@ 2026-07-07 13:36 ` Tetsuo Handa
2026-07-07 15:00 ` Alexander Potapenko
2026-07-08 15:28 ` Alexander Potapenko
1 sibling, 1 reply; 29+ messages in thread
From: Tetsuo Handa @ 2026-07-07 13:36 UTC (permalink / raw)
To: Sebastian Andrzej Siewior, Alexander Potapenko, Dmitry Vyukov
Cc: kasan-dev, LKML, Alan Stern, Andrew Morton, Andrey Konovalov,
Andrey Konovalov, Clark Williams, Greg Kroah-Hartman,
Marco Elver
On 2026/06/27 18:58, Tetsuo Handa wrote:
> Only compile tested. This version preserves kcov_percpu_data.lock which the past
> versions attempted to remove ( https://sashiko.dev/#/message/7aff4d71-a6a0-4898-9491-2a3973e9d0cc%40I-love.SAKURA.ne.jp )
> and explicitly passes context type instead of relying on buffer size in case
> a task context by chance requested buffer size which is equals to the size of
> buffers preallocated for the softirq context.
Since sashiko did not find new problem, can we start testing this version?
( https://sashiko.dev/#/patchset/8eef915c-96d4-4901-b404-60a19d5f9f54@I-love.SAKURA.ne.jp )
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v5] kcov: fix data corruption and race conditions on PREEMPT_RT by moving saved remote state to task_struct
2026-07-07 13:36 ` Tetsuo Handa
@ 2026-07-07 15:00 ` Alexander Potapenko
2026-07-07 22:07 ` Tetsuo Handa
0 siblings, 1 reply; 29+ messages in thread
From: Alexander Potapenko @ 2026-07-07 15:00 UTC (permalink / raw)
To: Tetsuo Handa
Cc: Sebastian Andrzej Siewior, Dmitry Vyukov, kasan-dev, LKML,
Alan Stern, Andrew Morton, Andrey Konovalov, Andrey Konovalov,
Clark Williams, Greg Kroah-Hartman, Marco Elver
On Tue, Jul 7, 2026 at 3:36 PM Tetsuo Handa
<penguin-kernel@i-love.sakura.ne.jp> wrote:
>
> On 2026/06/27 18:58, Tetsuo Handa wrote:
> > Only compile tested. This version preserves kcov_percpu_data.lock which the past
> > versions attempted to remove ( https://sashiko.dev/#/message/7aff4d71-a6a0-4898-9491-2a3973e9d0cc%40I-love.SAKURA.ne.jp )
> > and explicitly passes context type instead of relying on buffer size in case
> > a task context by chance requested buffer size which is equals to the size of
> > buffers preallocated for the softirq context.
>
> Since sashiko did not find new problem, can we start testing this version?
> ( https://sashiko.dev/#/patchset/8eef915c-96d4-4901-b404-60a19d5f9f54@I-love.SAKURA.ne.jp )
>
Hi Tetsuo,
Sorry, this has been on my plate, I will take a look tomorrow.
Do you want to test this patch by pulling it into linux-next?
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v5] kcov: fix data corruption and race conditions on PREEMPT_RT by moving saved remote state to task_struct
2026-07-07 15:00 ` Alexander Potapenko
@ 2026-07-07 22:07 ` Tetsuo Handa
0 siblings, 0 replies; 29+ messages in thread
From: Tetsuo Handa @ 2026-07-07 22:07 UTC (permalink / raw)
To: Alexander Potapenko
Cc: Sebastian Andrzej Siewior, Dmitry Vyukov, kasan-dev, LKML,
Alan Stern, Andrew Morton, Andrey Konovalov, Andrey Konovalov,
Clark Williams, Greg Kroah-Hartman, Marco Elver
On 2026/07/08 0:00, Alexander Potapenko wrote:
> On Tue, Jul 7, 2026 at 3:36 PM Tetsuo Handa
> <penguin-kernel@i-love.sakura.ne.jp> wrote:
>>
>> On 2026/06/27 18:58, Tetsuo Handa wrote:
>>> Only compile tested. This version preserves kcov_percpu_data.lock which the past
>>> versions attempted to remove ( https://sashiko.dev/#/message/7aff4d71-a6a0-4898-9491-2a3973e9d0cc%40I-love.SAKURA.ne.jp )
>>> and explicitly passes context type instead of relying on buffer size in case
>>> a task context by chance requested buffer size which is equals to the size of
>>> buffers preallocated for the softirq context.
>>
>> Since sashiko did not find new problem, can we start testing this version?
>> ( https://sashiko.dev/#/patchset/8eef915c-96d4-4901-b404-60a19d5f9f54@I-love.SAKURA.ne.jp )
>>
> Hi Tetsuo,
>
> Sorry, this has been on my plate, I will take a look tomorrow.
> Do you want to test this patch by pulling it into linux-next?
I can test this patch in linux-next using my tree (where changes will be
later reverted by "git push -f") so that we can see whether this patch can
close syzbot reports, before you commit this patch as a permanent change
via your tree.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v5] kcov: fix data corruption and race conditions on PREEMPT_RT by moving saved remote state to task_struct
2026-06-27 9:58 ` [PATCH v5] kcov: fix data corruption and race conditions on PREEMPT_RT by moving saved remote state to task_struct Tetsuo Handa
2026-07-07 13:36 ` Tetsuo Handa
@ 2026-07-08 15:28 ` Alexander Potapenko
2026-07-09 13:35 ` Tetsuo Handa
1 sibling, 1 reply; 29+ messages in thread
From: Alexander Potapenko @ 2026-07-08 15:28 UTC (permalink / raw)
To: Tetsuo Handa
Cc: Sebastian Andrzej Siewior, Dmitry Vyukov, kasan-dev, LKML,
Alan Stern, Andrew Morton, Andrey Konovalov, Andrey Konovalov,
Clark Williams, Greg Kroah-Hartman, Marco Elver, Roman Gushchin
>
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index 373bcc0598d1..7a53c15cecb5 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -1543,6 +1543,14 @@ struct task_struct {
>
> /* Collect coverage from softirq context: */
> unsigned int kcov_softirq;
> +
> + /* Temporary storage for preempting remote coverage collection: */
> + unsigned int kcov_saved_mode;
> + unsigned int kcov_saved_size;
> + void *kcov_saved_area;
> + struct kcov *kcov_saved_kcov;
> + int kcov_saved_sequence;
I think we need to land "kcov: factor out struct kcov_state"
(https://lore.kernel.org/all/20250731115139.3035888-4-glider@google.com/#r),
these stray members of task_struct are getting out of control.
Let me send it separately from the rest of my changes.
Also note these new fields will need to be initialized in kcov_task_init().
>
> - if (in_task()) {
> + if (1) {
Why? Can you remove this if statement?
(I am also curious why Sashiko did not report it, CC Roman)
> +#ifdef CONFIG_PREEMPT_RT
> + /* Allocate some extra buffers in order to prepare for softirq preemption. */
> + cpu = cpu >= 4 ? cpu * 2 : cpu + 4;
> +#endif
> + while (cpu--) {
> + void *area = vmalloc(CONFIG_KCOV_IRQ_AREA_SIZE * sizeof(unsigned long));
We need a NULL check here, kcov_remote_area_put() is going to dereference area.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v5] kcov: fix data corruption and race conditions on PREEMPT_RT by moving saved remote state to task_struct
2026-07-08 15:28 ` Alexander Potapenko
@ 2026-07-09 13:35 ` Tetsuo Handa
2026-07-10 10:43 ` Alexander Potapenko
0 siblings, 1 reply; 29+ messages in thread
From: Tetsuo Handa @ 2026-07-09 13:35 UTC (permalink / raw)
To: Alexander Potapenko
Cc: Sebastian Andrzej Siewior, Dmitry Vyukov, kasan-dev, LKML,
Alan Stern, Andrew Morton, Andrey Konovalov, Andrey Konovalov,
Clark Williams, Greg Kroah-Hartman, Marco Elver, Roman Gushchin
On 2026/07/09 0:28, Alexander Potapenko wrote:
>>
>> diff --git a/include/linux/sched.h b/include/linux/sched.h
>> index 373bcc0598d1..7a53c15cecb5 100644
>> --- a/include/linux/sched.h
>> +++ b/include/linux/sched.h
>> @@ -1543,6 +1543,14 @@ struct task_struct {
>>
>> /* Collect coverage from softirq context: */
>> unsigned int kcov_softirq;
>> +
>> + /* Temporary storage for preempting remote coverage collection: */
>> + unsigned int kcov_saved_mode;
>> + unsigned int kcov_saved_size;
>> + void *kcov_saved_area;
>> + struct kcov *kcov_saved_kcov;
>> + int kcov_saved_sequence;
>
> I think we need to land "kcov: factor out struct kcov_state"
> (https://lore.kernel.org/all/20250731115139.3035888-4-glider@google.com/#r),
> these stray members of task_struct are getting out of control.
> Let me send it separately from the rest of my changes.
Can we apply my patch first for smaller changes and easier backporting?
I sent my patch to linux-next-20260708, and so far
https://syzkaller.appspot.com/bug?extid=3f51ad7ac3ae57a6fdcc
https://syzkaller.appspot.com/bug?extid=47cf95ca1f9dcca872c8
https://syzkaller.appspot.com/bug?extid=8a173e13208949931dc7
https://syzkaller.appspot.com/bug?extid=90984d3713722683112e
are not occurring in linux-next for 22 hours.
>
> Also note these new fields will need to be initialized in kcov_task_init().
I and Sashiko don't think so. You can check
**Analysis of `kcov` Initialization and Forking Behavior**
section in https://sashiko.dev/#/log/87568 . These new fields are used only during
KCOV collection is running. Since these fields are duplicated by dup_task_struct()
where KCOV collection is not running, there is no need to initialize.
>
>>
>> - if (in_task()) {
>> + if (1) {
>
> Why? Can you remove this if statement?
> (I am also curious why Sashiko did not report it, CC Roman)
Just to minimize lines changed for easier review. Since the compiler will
optimize, I don't think this "if (1)" will cause performance penalty.
>
>
>> +#ifdef CONFIG_PREEMPT_RT
>> + /* Allocate some extra buffers in order to prepare for softirq preemption. */
>> + cpu = cpu >= 4 ? cpu * 2 : cpu + 4;
>> +#endif
>> + while (cpu--) {
>> + void *area = vmalloc(CONFIG_KCOV_IRQ_AREA_SIZE * sizeof(unsigned long));
>
> We need a NULL check here, kcov_remote_area_put() is going to dereference area.
Although Sashiko knows lack of NULL check here, Sashiko did not report this change in
the final report, for this function is executed as __init function of built-in module.
If GFP_KERNEL memory allocation by vmalloc() cannot find memory, the system won't be able
to survive even if we handled allocation failure. Also, since there is no userspace process
which the OOM killer can select as an OOM victim, the OOM killer will call
panic("System is deadlocked on memory\n") if size of GFP_KERNEL memory allocation within
vmalloc() request is up to 8 pages (i.e. 32KB in many architectures).
That is, skipping NULL check is acceptable for __init function of built-in module.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v5] kcov: fix data corruption and race conditions on PREEMPT_RT by moving saved remote state to task_struct
2026-07-09 13:35 ` Tetsuo Handa
@ 2026-07-10 10:43 ` Alexander Potapenko
2026-07-10 11:32 ` [PATCH v6] " Tetsuo Handa
0 siblings, 1 reply; 29+ messages in thread
From: Alexander Potapenko @ 2026-07-10 10:43 UTC (permalink / raw)
To: Tetsuo Handa
Cc: Sebastian Andrzej Siewior, Dmitry Vyukov, kasan-dev, LKML,
Alan Stern, Andrew Morton, Andrey Konovalov, Andrey Konovalov,
Clark Williams, Greg Kroah-Hartman, Marco Elver, Roman Gushchin
> > I think we need to land "kcov: factor out struct kcov_state"
> > (https://lore.kernel.org/all/20250731115139.3035888-4-glider@google.com/#r),
> > these stray members of task_struct are getting out of control.
> > Let me send it separately from the rest of my changes.
>
> Can we apply my patch first for smaller changes and easier backporting?
Fine, we can do that.
> I sent my patch to linux-next-20260708, and so far
>
> https://syzkaller.appspot.com/bug?extid=3f51ad7ac3ae57a6fdcc
> https://syzkaller.appspot.com/bug?extid=47cf95ca1f9dcca872c8
> https://syzkaller.appspot.com/bug?extid=8a173e13208949931dc7
> https://syzkaller.appspot.com/bug?extid=90984d3713722683112e
>
> are not occurring in linux-next for 22 hours.
>
> >
> > Also note these new fields will need to be initialized in kcov_task_init().
>
> I and Sashiko don't think so. You can check
> **Analysis of `kcov` Initialization and Forking Behavior**
> section in https://sashiko.dev/#/log/87568 . These new fields are used only during
> KCOV collection is running. Since these fields are duplicated by dup_task_struct()
> where KCOV collection is not running, there is no need to initialize.
This is all assuming that kcov_remote_start()/kcov_remote_stop() are
placed correctly.
If for some reason a developer fails to call kcov_remote_stop() for a
certain module, we have stale pointers to the remote context, which
will leak to the child task upon fork.
If we reset them upon fork, the stale pointers are contained to the
original task.
> >
> >>
> >> - if (in_task()) {
> >> + if (1) {
> >
> > Why? Can you remove this if statement?
> > (I am also curious why Sashiko did not report it, CC Roman)
>
> Just to minimize lines changed for easier review. Since the compiler will
> optimize, I don't think this "if (1)" will cause performance penalty.
It won't, but it is still a coding style we don't want to tolerate.
Please remove it.
> > We need a NULL check here, kcov_remote_area_put() is going to dereference area.
>
> Although Sashiko knows lack of NULL check here, Sashiko did not report this change in
> the final report, for this function is executed as __init function of built-in module.
Sashiko did not report the if (1) either, so I wouldn't bet money on
it (although I love Sashiko).
> If GFP_KERNEL memory allocation by vmalloc() cannot find memory, the system won't be able
> to survive even if we handled allocation failure. Also, since there is no userspace process
> which the OOM killer can select as an OOM victim, the OOM killer will call
> panic("System is deadlocked on memory\n") if size of GFP_KERNEL memory allocation within
> vmalloc() request is up to 8 pages (i.e. 32KB in many architectures).
>
> That is, skipping NULL check is acceptable for __init function of built-in module.
>
Removing the NULL check here is a regression compared to the previous
version of this code.
Restoring it is easier than adding a comment explaining its absence,
which would otherwise be necessary.
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v6] kcov: fix data corruption and race conditions on PREEMPT_RT by moving saved remote state to task_struct
2026-07-10 10:43 ` Alexander Potapenko
@ 2026-07-10 11:32 ` Tetsuo Handa
2026-07-10 12:16 ` Alexander Potapenko
2026-07-10 14:24 ` [PATCH v6] kcov: fix data corruption and race conditions on PREEMPT_RT by moving saved remote state to task_struct Sebastian Andrzej Siewior
0 siblings, 2 replies; 29+ messages in thread
From: Tetsuo Handa @ 2026-07-10 11:32 UTC (permalink / raw)
To: Alexander Potapenko, Sebastian Andrzej Siewior, Dmitry Vyukov
Cc: kasan-dev, LKML, Alan Stern, Andrew Morton, Andrey Konovalov,
Andrey Konovalov, Clark Williams, Greg Kroah-Hartman,
Marco Elver, Roman Gushchin
syzbot is reporting KCOV state corruption on PREEMPT_RT kernels, for the
temporary storage used for saving/restoring remote KCOV state is currently
allocated as the per-CPU area.
On PREEMPT_RT kernels, softirq handlers run as preemptible task threads
(e.g., ksoftirqd). If a softirq context preempts a task running a remote
KCOV session, it safely saves the task's state into the per-CPU area.
However, if that softirq thread is subsequently preempted by a higher-
priority softirq thread on the same CPU, the second softirq will overwrite
the same per-CPU area, permanently destroying the original task's KCOV
state.
Fix this data corruption by moving the temporary storage from the per-CPU
area to the per-thread area. Since each softirq thread now owns its own
task context, nested softirq preemption no longer causes data overwrites.
Note that while the temporary storage is now on a per-thread basis, the
per-CPU kcov_percpu_data.lock must be retained, for we need to ensure that
kcov_remote_start() and kcov_remote_stop() operate atomically without
racing against asynchronous interrupts that manipulate the current task's
KCOV state.
Reported-by: syzbot+3f51ad7ac3ae57a6fdcc@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=3f51ad7ac3ae57a6fdcc
Reported-by: syzbot+47cf95ca1f9dcca872c8@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=47cf95ca1f9dcca872c8
Reported-by: syzbot+8a173e13208949931dc7@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=8a173e13208949931dc7
Reported-by: syzbot+90984d3713722683112e@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=90984d3713722683112e
Analyzed-by: AI Mode in Google Search (no mail address)
Fixes: 5ff3b30ab57d ("kcov: collect coverage from interrupts")
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
---
include/linux/sched.h | 8 ++++
kernel/kcov.c | 89 ++++++++++++++++++++++---------------------
lib/Kconfig.debug | 5 ++-
3 files changed, 57 insertions(+), 45 deletions(-)
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 373bcc0598d1..7a53c15cecb5 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1543,6 +1543,14 @@ struct task_struct {
/* Collect coverage from softirq context: */
unsigned int kcov_softirq;
+
+ /* Temporary storage for preempting remote coverage collection: */
+ unsigned int kcov_saved_mode;
+ unsigned int kcov_saved_size;
+ void *kcov_saved_area;
+ struct kcov *kcov_saved_kcov;
+ int kcov_saved_sequence;
+
#endif
#ifdef CONFIG_MEMCG_V1
diff --git a/kernel/kcov.c b/kernel/kcov.c
index 1df373fb562b..dceb833c19b5 100644
--- a/kernel/kcov.c
+++ b/kernel/kcov.c
@@ -86,17 +86,12 @@ struct kcov_remote {
static DEFINE_SPINLOCK(kcov_remote_lock);
static DEFINE_HASHTABLE(kcov_remote_map, 4);
-static struct list_head kcov_remote_areas = LIST_HEAD_INIT(kcov_remote_areas);
+static struct list_head kcov_remote_areas[2] = {
+ LIST_HEAD_INIT(kcov_remote_areas[0]), LIST_HEAD_INIT(kcov_remote_areas[1])
+};
struct kcov_percpu_data {
- void *irq_area;
local_lock_t lock;
-
- unsigned int saved_mode;
- unsigned int saved_size;
- void *saved_area;
- struct kcov *saved_kcov;
- int saved_sequence;
};
static DEFINE_PER_CPU(struct kcov_percpu_data, kcov_percpu_data) = {
@@ -132,12 +127,13 @@ static struct kcov_remote *kcov_remote_add(struct kcov *kcov, u64 handle)
}
/* Must be called with kcov_remote_lock locked. */
-static struct kcov_remote_area *kcov_remote_area_get(unsigned int size)
+static struct kcov_remote_area *kcov_remote_area_get(unsigned int size, bool irq)
{
struct kcov_remote_area *area;
struct list_head *pos;
+ struct list_head *list = &kcov_remote_areas[irq];
- list_for_each(pos, &kcov_remote_areas) {
+ list_for_each(pos, list) {
area = list_entry(pos, struct kcov_remote_area, list);
if (area->size == size) {
list_del(&area->list);
@@ -149,11 +145,11 @@ static struct kcov_remote_area *kcov_remote_area_get(unsigned int size)
/* Must be called with kcov_remote_lock locked. */
static void kcov_remote_area_put(struct kcov_remote_area *area,
- unsigned int size)
+ unsigned int size, bool irq)
{
INIT_LIST_HEAD(&area->list);
area->size = size;
- list_add(&area->list, &kcov_remote_areas);
+ list_add(&area->list, &kcov_remote_areas[irq]);
/*
* KMSAN doesn't instrument this file, so it may not know area->list
* is initialized. Unpoison it explicitly to avoid reports in
@@ -390,6 +386,11 @@ void kcov_task_init(struct task_struct *t)
kcov_task_reset(t);
t->kcov_remote = NULL;
t->kcov_handle = current->kcov_handle;
+ t->kcov_saved_mode = 0;
+ t->kcov_saved_size = 0;
+ t->kcov_saved_area = NULL;
+ t->kcov_saved_kcov = NULL;
+ t->kcov_saved_sequence = 0;
}
static void kcov_reset(struct kcov *kcov)
@@ -836,17 +837,16 @@ static inline bool kcov_mode_enabled(unsigned int mode)
static void kcov_remote_softirq_start(struct task_struct *t)
__must_hold(&kcov_percpu_data.lock)
{
- struct kcov_percpu_data *data = this_cpu_ptr(&kcov_percpu_data);
unsigned int mode;
mode = READ_ONCE(t->kcov_mode);
barrier();
if (kcov_mode_enabled(mode)) {
- data->saved_mode = mode;
- data->saved_size = t->kcov_size;
- data->saved_area = t->kcov_area;
- data->saved_sequence = t->kcov_sequence;
- data->saved_kcov = t->kcov;
+ t->kcov_saved_mode = mode;
+ t->kcov_saved_size = t->kcov_size;
+ t->kcov_saved_area = t->kcov_area;
+ t->kcov_saved_sequence = t->kcov_sequence;
+ t->kcov_saved_kcov = t->kcov;
kcov_stop(t);
}
}
@@ -854,17 +854,15 @@ static void kcov_remote_softirq_start(struct task_struct *t)
static void kcov_remote_softirq_stop(struct task_struct *t)
__must_hold(&kcov_percpu_data.lock)
{
- struct kcov_percpu_data *data = this_cpu_ptr(&kcov_percpu_data);
-
- if (data->saved_kcov) {
- kcov_start(t, data->saved_kcov, data->saved_size,
- data->saved_area, data->saved_mode,
- data->saved_sequence);
- data->saved_mode = 0;
- data->saved_size = 0;
- data->saved_area = NULL;
- data->saved_sequence = 0;
- data->saved_kcov = NULL;
+ if (t->kcov_saved_kcov) {
+ kcov_start(t, t->kcov_saved_kcov, t->kcov_saved_size,
+ t->kcov_saved_area, t->kcov_saved_mode,
+ t->kcov_saved_sequence);
+ t->kcov_saved_mode = 0;
+ t->kcov_saved_size = 0;
+ t->kcov_saved_area = NULL;
+ t->kcov_saved_sequence = 0;
+ t->kcov_saved_kcov = NULL;
}
}
@@ -927,17 +925,17 @@ void kcov_remote_start(u64 handle)
sequence = kcov->sequence;
if (in_task()) {
size = kcov->remote_size;
- area = kcov_remote_area_get(size);
+ area = kcov_remote_area_get(size, false);
} else {
size = CONFIG_KCOV_IRQ_AREA_SIZE;
- area = this_cpu_ptr(&kcov_percpu_data)->irq_area;
+ area = kcov_remote_area_get(size, true);
}
spin_unlock(&kcov_remote_lock);
- /* Can only happen when in_task(). */
+ /* Allocate new buffer if we can sleep. */
if (!area) {
local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
- area = vmalloc(size * sizeof(unsigned long));
+ area = in_task() ? vmalloc(size * sizeof(unsigned long)) : NULL;
if (!area) {
kcov_put(kcov);
return;
@@ -1079,11 +1077,9 @@ void kcov_remote_stop(void)
kcov_move_area(kcov->mode, kcov->area, kcov->size, area);
spin_unlock(&kcov->lock);
- if (in_task()) {
- spin_lock(&kcov_remote_lock);
- kcov_remote_area_put(area, size);
- spin_unlock(&kcov_remote_lock);
- }
+ spin_lock(&kcov_remote_lock);
+ kcov_remote_area_put(area, size, !in_task());
+ spin_unlock(&kcov_remote_lock);
local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
@@ -1129,14 +1125,21 @@ static void __init selftest(void)
static int __init kcov_init(void)
{
- int cpu;
+ int cpu = num_possible_cpus();
+
+#ifdef CONFIG_PREEMPT_RT
+ /* Allocate some extra buffers in order to prepare for softirq preemption. */
+ cpu = cpu >= 4 ? cpu * 2 : cpu + 4;
+#endif
+ while (cpu--) {
+ void *area = vmalloc(CONFIG_KCOV_IRQ_AREA_SIZE * sizeof(unsigned long));
+ unsigned long flags;
- for_each_possible_cpu(cpu) {
- void *area = vmalloc_node(CONFIG_KCOV_IRQ_AREA_SIZE *
- sizeof(unsigned long), cpu_to_node(cpu));
if (!area)
return -ENOMEM;
- per_cpu_ptr(&kcov_percpu_data, cpu)->irq_area = area;
+ spin_lock_irqsave(&kcov_remote_lock, flags);
+ kcov_remote_area_put(area, CONFIG_KCOV_IRQ_AREA_SIZE, true);
+ spin_unlock_irqrestore(&kcov_remote_lock, flags);
}
/*
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 1244dcac2294..12786379bf1d 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -2247,10 +2247,11 @@ config KCOV_INSTRUMENT_ALL
config KCOV_IRQ_AREA_SIZE
hex "Size of interrupt coverage collection area in words"
depends on KCOV
+ range 0x80 0x1000000
default 0x40000
help
- KCOV uses preallocated per-cpu areas to collect coverage from
- soft interrupts. This specifies the size of those areas in the
+ KCOV uses preallocated areas to collect coverage from soft
+ interrupts. This specifies the size of those areas in the
number of unsigned long words.
config KCOV_SELFTEST
--
2.52.0
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v6] kcov: fix data corruption and race conditions on PREEMPT_RT by moving saved remote state to task_struct
2026-07-10 11:32 ` [PATCH v6] " Tetsuo Handa
@ 2026-07-10 12:16 ` Alexander Potapenko
2026-07-15 23:01 ` [PATCH v7] kcov: fix data corruption and race conditions on PREEMPT_RT Tetsuo Handa
2026-07-10 14:24 ` [PATCH v6] kcov: fix data corruption and race conditions on PREEMPT_RT by moving saved remote state to task_struct Sebastian Andrzej Siewior
1 sibling, 1 reply; 29+ messages in thread
From: Alexander Potapenko @ 2026-07-10 12:16 UTC (permalink / raw)
To: Tetsuo Handa
Cc: Sebastian Andrzej Siewior, Dmitry Vyukov, kasan-dev, LKML,
Alan Stern, Andrew Morton, Andrey Konovalov, Andrey Konovalov,
Clark Williams, Greg Kroah-Hartman, Marco Elver, Roman Gushchin
On Fri, Jul 10, 2026 at 1:32 PM Tetsuo Handa
<penguin-kernel@i-love.sakura.ne.jp> wrote:
>
> syzbot is reporting KCOV state corruption on PREEMPT_RT kernels, for the
> temporary storage used for saving/restoring remote KCOV state is currently
> allocated as the per-CPU area.
>
> On PREEMPT_RT kernels, softirq handlers run as preemptible task threads
> (e.g., ksoftirqd). If a softirq context preempts a task running a remote
> KCOV session, it safely saves the task's state into the per-CPU area.
> However, if that softirq thread is subsequently preempted by a higher-
> priority softirq thread on the same CPU, the second softirq will overwrite
> the same per-CPU area, permanently destroying the original task's KCOV
> state.
>
> Fix this data corruption by moving the temporary storage from the per-CPU
> area to the per-thread area. Since each softirq thread now owns its own
> task context, nested softirq preemption no longer causes data overwrites.
>
> Note that while the temporary storage is now on a per-thread basis, the
> per-CPU kcov_percpu_data.lock must be retained, for we need to ensure that
> kcov_remote_start() and kcov_remote_stop() operate atomically without
> racing against asynchronous interrupts that manipulate the current task's
> KCOV state.
>
> Reported-by: syzbot+3f51ad7ac3ae57a6fdcc@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=3f51ad7ac3ae57a6fdcc
> Reported-by: syzbot+47cf95ca1f9dcca872c8@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=47cf95ca1f9dcca872c8
> Reported-by: syzbot+8a173e13208949931dc7@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=8a173e13208949931dc7
> Reported-by: syzbot+90984d3713722683112e@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=90984d3713722683112e
> Analyzed-by: AI Mode in Google Search (no mail address)
> Fixes: 5ff3b30ab57d ("kcov: collect coverage from interrupts")
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Reviewed-by: Alexander Potapenko <glider@google.com>
See a nit below.
> kcov_task_reset(t);
> t->kcov_remote = NULL;
> t->kcov_handle = current->kcov_handle;
> + t->kcov_saved_mode = 0;
> + t->kcov_saved_size = 0;
> + t->kcov_saved_area = NULL;
> + t->kcov_saved_kcov = NULL;
> + t->kcov_saved_sequence = 0;
Thanks! For consistency, can you also initialize kcov_softirq?
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v6] kcov: fix data corruption and race conditions on PREEMPT_RT by moving saved remote state to task_struct
2026-07-10 11:32 ` [PATCH v6] " Tetsuo Handa
2026-07-10 12:16 ` Alexander Potapenko
@ 2026-07-10 14:24 ` Sebastian Andrzej Siewior
1 sibling, 0 replies; 29+ messages in thread
From: Sebastian Andrzej Siewior @ 2026-07-10 14:24 UTC (permalink / raw)
To: Tetsuo Handa
Cc: Alexander Potapenko, Dmitry Vyukov, kasan-dev, LKML, Alan Stern,
Andrew Morton, Andrey Konovalov, Andrey Konovalov,
Clark Williams, Greg Kroah-Hartman, Marco Elver, Roman Gushchin
On 2026-07-10 20:32:27 [+0900], Tetsuo Handa wrote:
> syzbot is reporting KCOV state corruption on PREEMPT_RT kernels, for the
> temporary storage used for saving/restoring remote KCOV state is currently
> allocated as the per-CPU area.
>
> On PREEMPT_RT kernels, softirq handlers run as preemptible task threads
> (e.g., ksoftirqd). If a softirq context preempts a task running a remote
> KCOV session, it safely saves the task's state into the per-CPU area.
> However, if that softirq thread is subsequently preempted by a higher-
> priority softirq thread on the same CPU, the second softirq will overwrite
> the same per-CPU area, permanently destroying the original task's KCOV
> state.
>
> Fix this data corruption by moving the temporary storage from the per-CPU
> area to the per-thread area. Since each softirq thread now owns its own
> task context, nested softirq preemption no longer causes data overwrites.
>
> Note that while the temporary storage is now on a per-thread basis, the
> per-CPU kcov_percpu_data.lock must be retained, for we need to ensure that
> kcov_remote_start() and kcov_remote_stop() operate atomically without
> racing against asynchronous interrupts that manipulate the current task's
> KCOV state.
>
> Reported-by: syzbot+3f51ad7ac3ae57a6fdcc@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=3f51ad7ac3ae57a6fdcc
> Reported-by: syzbot+47cf95ca1f9dcca872c8@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=47cf95ca1f9dcca872c8
> Reported-by: syzbot+8a173e13208949931dc7@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=8a173e13208949931dc7
> Reported-by: syzbot+90984d3713722683112e@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=90984d3713722683112e
> Analyzed-by: AI Mode in Google Search (no mail address)
> Fixes: 5ff3b30ab57d ("kcov: collect coverage from interrupts")
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
No complains from my side. Thank you.
Sebastian
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v7] kcov: fix data corruption and race conditions on PREEMPT_RT
2026-07-10 12:16 ` Alexander Potapenko
@ 2026-07-15 23:01 ` Tetsuo Handa
2026-07-16 9:12 ` Alexander Potapenko
0 siblings, 1 reply; 29+ messages in thread
From: Tetsuo Handa @ 2026-07-15 23:01 UTC (permalink / raw)
To: Alexander Potapenko, Sebastian Andrzej Siewior, Dmitry Vyukov
Cc: kasan-dev, LKML, Alan Stern, Andrew Morton, Andrey Konovalov,
Andrey Konovalov, Clark Williams, Greg Kroah-Hartman,
Marco Elver, Roman Gushchin, Christoph Hellwig, Mark Brown
syzbot is reporting KCOV state corruption on PREEMPT_RT kernels, for the
temporary storage used for saving/restoring remote KCOV state is currently
allocated as the per-CPU area.
On PREEMPT_RT kernels, softirq handlers run as preemptible task threads
(e.g., ksoftirqd). If a softirq context preempts a task running a remote
KCOV session, it safely saves the task's state into the per-CPU area.
However, if that softirq thread is subsequently preempted by a higher-
priority softirq thread on the same CPU, the second softirq will overwrite
the same per-CPU area, permanently destroying the original task's KCOV
state.
Fix this data corruption by moving the temporary storage from the per-CPU
area to the per-thread area. Since each softirq thread now owns its own
task context, nested softirq preemption no longer causes data overwrites.
Note that while the temporary storage is now on a per-thread basis, the
per-CPU kcov_percpu_data.lock must be retained, for we need to ensure that
kcov_remote_start() and kcov_remote_stop() operate atomically without
racing against asynchronous interrupts that manipulate the current task's
KCOV state.
It is likely that GFP_KERNEL allocation by vmalloc_node() in kcov_init()
has already called panic() before returning NULL, for there will be no
OOM-killable userspace processes when __init function of built-in module
runs. But this patch also fixes crashing the kernel when vmalloc_node()
in kcov_init() returned NULL, for kcov_init() left per-CPU irq_area == NULL
but kcov_remote_start() depends on per-CPU irq_area != NULL, resulting in
(1) doing vmalloc() in kcov_remote_start() despite !in_task() context
(2) out-of-array-bounds access if (1) succeeded but
kcov->remote_size < CONFIG_KCOV_IRQ_AREA_SIZE
(3) always leak memory allocated by (1), eventually killing all
OOM-killable userspace processes
problems.
Reported-by: syzbot+3f51ad7ac3ae57a6fdcc@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=3f51ad7ac3ae57a6fdcc
Reported-by: syzbot+47cf95ca1f9dcca872c8@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=47cf95ca1f9dcca872c8
Reported-by: syzbot+8a173e13208949931dc7@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=8a173e13208949931dc7
Reported-by: syzbot+90984d3713722683112e@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=90984d3713722683112e
Analyzed-by: AI Mode in Google Search (no mail address)
Fixes: 5ff3b30ab57d ("kcov: collect coverage from interrupts")
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Reviewed-by: Alexander Potapenko <glider@google.com>
---
syzbot has tested this patch using linux-next, and I confirmed that
this patch does not cause new problems. Please send to upstream.
By the way, I updated patch description because
if (!area)
return -ENOMEM;
was a dead code nobody had tried to test.
include/linux/sched.h | 8 ++++
kernel/kcov.c | 90 ++++++++++++++++++++++---------------------
lib/Kconfig.debug | 5 ++-
3 files changed, 58 insertions(+), 45 deletions(-)
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 373bcc0598d1..7a53c15cecb5 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1543,6 +1543,14 @@ struct task_struct {
/* Collect coverage from softirq context: */
unsigned int kcov_softirq;
+
+ /* Temporary storage for preempting remote coverage collection: */
+ unsigned int kcov_saved_mode;
+ unsigned int kcov_saved_size;
+ void *kcov_saved_area;
+ struct kcov *kcov_saved_kcov;
+ int kcov_saved_sequence;
+
#endif
#ifdef CONFIG_MEMCG_V1
diff --git a/kernel/kcov.c b/kernel/kcov.c
index 1df373fb562b..a7514303eff3 100644
--- a/kernel/kcov.c
+++ b/kernel/kcov.c
@@ -86,17 +86,12 @@ struct kcov_remote {
static DEFINE_SPINLOCK(kcov_remote_lock);
static DEFINE_HASHTABLE(kcov_remote_map, 4);
-static struct list_head kcov_remote_areas = LIST_HEAD_INIT(kcov_remote_areas);
+static struct list_head kcov_remote_areas[2] = {
+ LIST_HEAD_INIT(kcov_remote_areas[0]), LIST_HEAD_INIT(kcov_remote_areas[1])
+};
struct kcov_percpu_data {
- void *irq_area;
local_lock_t lock;
-
- unsigned int saved_mode;
- unsigned int saved_size;
- void *saved_area;
- struct kcov *saved_kcov;
- int saved_sequence;
};
static DEFINE_PER_CPU(struct kcov_percpu_data, kcov_percpu_data) = {
@@ -132,12 +127,13 @@ static struct kcov_remote *kcov_remote_add(struct kcov *kcov, u64 handle)
}
/* Must be called with kcov_remote_lock locked. */
-static struct kcov_remote_area *kcov_remote_area_get(unsigned int size)
+static struct kcov_remote_area *kcov_remote_area_get(unsigned int size, bool irq)
{
struct kcov_remote_area *area;
struct list_head *pos;
+ struct list_head *list = &kcov_remote_areas[irq];
- list_for_each(pos, &kcov_remote_areas) {
+ list_for_each(pos, list) {
area = list_entry(pos, struct kcov_remote_area, list);
if (area->size == size) {
list_del(&area->list);
@@ -149,11 +145,11 @@ static struct kcov_remote_area *kcov_remote_area_get(unsigned int size)
/* Must be called with kcov_remote_lock locked. */
static void kcov_remote_area_put(struct kcov_remote_area *area,
- unsigned int size)
+ unsigned int size, bool irq)
{
INIT_LIST_HEAD(&area->list);
area->size = size;
- list_add(&area->list, &kcov_remote_areas);
+ list_add(&area->list, &kcov_remote_areas[irq]);
/*
* KMSAN doesn't instrument this file, so it may not know area->list
* is initialized. Unpoison it explicitly to avoid reports in
@@ -390,6 +386,12 @@ void kcov_task_init(struct task_struct *t)
kcov_task_reset(t);
t->kcov_remote = NULL;
t->kcov_handle = current->kcov_handle;
+ t->kcov_softirq = 0;
+ t->kcov_saved_mode = 0;
+ t->kcov_saved_size = 0;
+ t->kcov_saved_area = NULL;
+ t->kcov_saved_kcov = NULL;
+ t->kcov_saved_sequence = 0;
}
static void kcov_reset(struct kcov *kcov)
@@ -836,17 +838,16 @@ static inline bool kcov_mode_enabled(unsigned int mode)
static void kcov_remote_softirq_start(struct task_struct *t)
__must_hold(&kcov_percpu_data.lock)
{
- struct kcov_percpu_data *data = this_cpu_ptr(&kcov_percpu_data);
unsigned int mode;
mode = READ_ONCE(t->kcov_mode);
barrier();
if (kcov_mode_enabled(mode)) {
- data->saved_mode = mode;
- data->saved_size = t->kcov_size;
- data->saved_area = t->kcov_area;
- data->saved_sequence = t->kcov_sequence;
- data->saved_kcov = t->kcov;
+ t->kcov_saved_mode = mode;
+ t->kcov_saved_size = t->kcov_size;
+ t->kcov_saved_area = t->kcov_area;
+ t->kcov_saved_sequence = t->kcov_sequence;
+ t->kcov_saved_kcov = t->kcov;
kcov_stop(t);
}
}
@@ -854,17 +855,15 @@ static void kcov_remote_softirq_start(struct task_struct *t)
static void kcov_remote_softirq_stop(struct task_struct *t)
__must_hold(&kcov_percpu_data.lock)
{
- struct kcov_percpu_data *data = this_cpu_ptr(&kcov_percpu_data);
-
- if (data->saved_kcov) {
- kcov_start(t, data->saved_kcov, data->saved_size,
- data->saved_area, data->saved_mode,
- data->saved_sequence);
- data->saved_mode = 0;
- data->saved_size = 0;
- data->saved_area = NULL;
- data->saved_sequence = 0;
- data->saved_kcov = NULL;
+ if (t->kcov_saved_kcov) {
+ kcov_start(t, t->kcov_saved_kcov, t->kcov_saved_size,
+ t->kcov_saved_area, t->kcov_saved_mode,
+ t->kcov_saved_sequence);
+ t->kcov_saved_mode = 0;
+ t->kcov_saved_size = 0;
+ t->kcov_saved_area = NULL;
+ t->kcov_saved_sequence = 0;
+ t->kcov_saved_kcov = NULL;
}
}
@@ -927,17 +926,17 @@ void kcov_remote_start(u64 handle)
sequence = kcov->sequence;
if (in_task()) {
size = kcov->remote_size;
- area = kcov_remote_area_get(size);
+ area = kcov_remote_area_get(size, false);
} else {
size = CONFIG_KCOV_IRQ_AREA_SIZE;
- area = this_cpu_ptr(&kcov_percpu_data)->irq_area;
+ area = kcov_remote_area_get(size, true);
}
spin_unlock(&kcov_remote_lock);
- /* Can only happen when in_task(). */
+ /* Allocate new buffer if we can sleep. */
if (!area) {
local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
- area = vmalloc(size * sizeof(unsigned long));
+ area = in_task() ? vmalloc(size * sizeof(unsigned long)) : NULL;
if (!area) {
kcov_put(kcov);
return;
@@ -1079,11 +1078,9 @@ void kcov_remote_stop(void)
kcov_move_area(kcov->mode, kcov->area, kcov->size, area);
spin_unlock(&kcov->lock);
- if (in_task()) {
- spin_lock(&kcov_remote_lock);
- kcov_remote_area_put(area, size);
- spin_unlock(&kcov_remote_lock);
- }
+ spin_lock(&kcov_remote_lock);
+ kcov_remote_area_put(area, size, !in_task());
+ spin_unlock(&kcov_remote_lock);
local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
@@ -1129,14 +1126,21 @@ static void __init selftest(void)
static int __init kcov_init(void)
{
- int cpu;
+ int cpu = num_possible_cpus();
+
+#ifdef CONFIG_PREEMPT_RT
+ /* Allocate some extra buffers in order to prepare for softirq preemption. */
+ cpu = cpu >= 4 ? cpu * 2 : cpu + 4;
+#endif
+ while (cpu--) {
+ void *area = vmalloc(CONFIG_KCOV_IRQ_AREA_SIZE * sizeof(unsigned long));
+ unsigned long flags;
- for_each_possible_cpu(cpu) {
- void *area = vmalloc_node(CONFIG_KCOV_IRQ_AREA_SIZE *
- sizeof(unsigned long), cpu_to_node(cpu));
if (!area)
return -ENOMEM;
- per_cpu_ptr(&kcov_percpu_data, cpu)->irq_area = area;
+ spin_lock_irqsave(&kcov_remote_lock, flags);
+ kcov_remote_area_put(area, CONFIG_KCOV_IRQ_AREA_SIZE, true);
+ spin_unlock_irqrestore(&kcov_remote_lock, flags);
}
/*
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 5e2eeacd2451..66e0f0a02345 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -2247,10 +2247,11 @@ config KCOV_INSTRUMENT_ALL
config KCOV_IRQ_AREA_SIZE
hex "Size of interrupt coverage collection area in words"
depends on KCOV
+ range 0x80 0x1000000
default 0x40000
help
- KCOV uses preallocated per-cpu areas to collect coverage from
- soft interrupts. This specifies the size of those areas in the
+ KCOV uses preallocated areas to collect coverage from soft
+ interrupts. This specifies the size of those areas in the
number of unsigned long words.
config KCOV_SELFTEST
--
2.52.0
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v7] kcov: fix data corruption and race conditions on PREEMPT_RT
2026-07-15 23:01 ` [PATCH v7] kcov: fix data corruption and race conditions on PREEMPT_RT Tetsuo Handa
@ 2026-07-16 9:12 ` Alexander Potapenko
0 siblings, 0 replies; 29+ messages in thread
From: Alexander Potapenko @ 2026-07-16 9:12 UTC (permalink / raw)
To: Andrew Morton
Cc: Sebastian Andrzej Siewior, Dmitry Vyukov, kasan-dev, LKML,
Alan Stern, Andrey Konovalov, Andrey Konovalov, Clark Williams,
Greg Kroah-Hartman, Marco Elver, Roman Gushchin,
Christoph Hellwig, Mark Brown, Tetsuo Handa
On Thu, Jul 16, 2026 at 1:01 AM Tetsuo Handa
<penguin-kernel@i-love.sakura.ne.jp> wrote:
> Reported-by: syzbot+3f51ad7ac3ae57a6fdcc@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=3f51ad7ac3ae57a6fdcc
> Reported-by: syzbot+47cf95ca1f9dcca872c8@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=47cf95ca1f9dcca872c8
> Reported-by: syzbot+8a173e13208949931dc7@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=8a173e13208949931dc7
> Reported-by: syzbot+90984d3713722683112e@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=90984d3713722683112e
> Analyzed-by: AI Mode in Google Search (no mail address)
> Fixes: 5ff3b30ab57d ("kcov: collect coverage from interrupts")
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Reviewed-by: Alexander Potapenko <glider@google.com>
Andrew, may I ask you to take this to the mm tree?
^ permalink raw reply [flat|nested] 29+ messages in thread
end of thread, other threads:[~2026-07-16 9:13 UTC | newest]
Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-05-04 15:30 [PATCH (draft)] kcov: fix potential kcov_mode corruption under CONFIG_PREEMPT_RT Tetsuo Handa
2026-05-04 15:39 ` Tetsuo Handa
2026-05-05 8:15 ` [PATCH] " Tetsuo Handa
2026-05-06 11:55 ` [PATCH v2] kcov: fix state corruption under CONFIG_PREEMPT_RT by eliminating per-cpu data Tetsuo Handa
2026-05-09 8:09 ` [PATCH v3] kcov: move kcov_remote_data to task_struct for RT and remove local_lock Tetsuo Handa
2026-05-15 7:10 ` Dmitry Vyukov
2026-05-20 10:33 ` Tetsuo Handa
2026-05-20 15:12 ` Sebastian Andrzej Siewior
2026-05-21 8:38 ` Sebastian Andrzej Siewior
2026-05-21 15:28 ` Tetsuo Handa
2026-05-21 18:14 ` Sebastian Andrzej Siewior
2026-05-22 2:39 ` Tetsuo Handa
2026-05-22 11:10 ` [PATCH v4] kcov: move kcov_remote_data to task_struct " Tetsuo Handa
2026-05-22 13:27 ` Sebastian Andrzej Siewior
2026-05-29 6:35 ` Tetsuo Handa
2026-06-27 9:58 ` [PATCH v5] kcov: fix data corruption and race conditions on PREEMPT_RT by moving saved remote state to task_struct Tetsuo Handa
2026-07-07 13:36 ` Tetsuo Handa
2026-07-07 15:00 ` Alexander Potapenko
2026-07-07 22:07 ` Tetsuo Handa
2026-07-08 15:28 ` Alexander Potapenko
2026-07-09 13:35 ` Tetsuo Handa
2026-07-10 10:43 ` Alexander Potapenko
2026-07-10 11:32 ` [PATCH v6] " Tetsuo Handa
2026-07-10 12:16 ` Alexander Potapenko
2026-07-15 23:01 ` [PATCH v7] kcov: fix data corruption and race conditions on PREEMPT_RT Tetsuo Handa
2026-07-16 9:12 ` Alexander Potapenko
2026-07-10 14:24 ` [PATCH v6] kcov: fix data corruption and race conditions on PREEMPT_RT by moving saved remote state to task_struct Sebastian Andrzej Siewior
2026-05-21 13:06 ` [PATCH v3] kcov: move kcov_remote_data to task_struct for RT and remove local_lock Alexander Potapenko
2026-05-21 14:39 ` Tetsuo Handa
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox