Re: [PATCH resend] kcov: allow simultaneous KCOV_ENABLE/KCOV_REMOTE_ENABLE

[Jann Horn]

On Sat, May 9, 2026 at 3:24 AM Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote:
> On Tue, 05 May 2026 11:00:46 +0200 Jann Horn <jannh@xxxxxxxxxx> wrote:
> > Allow the same userspace thread to simultaneously collect normal coverage
> > in syscall context (KCOV_ENABLE) and remote coverage of asynchronous work
> > created by the thread (KCOV_REMOTE_ENABLE).
> > With this, remote KCOV coverage becomes useful for generic fuzzing and not
> > just fuzzing of specific data injection interfaces.
> >
> > This requires that the task_struct::kcov_* fields are separated into ones
> > that are used by the task that generates coverage, and ones that are used
> > by the task that requested remote coverage. To split this up:
> >
> >  - Split task_struct::kcov into kcov and kcov_remote. kcov_task_exit() now
> >    has to clean up both separately.
> >  - Only use task_struct::kcov_mode on the task that generates coverage.
> >  - Only reset task_struct::kcov_handle on the task that requested remote
> >    coverage.
> >
> > After this change, fields used by the task that generates coverage are:
> >
> >  - kcov_mode
> >  - kcov_size
> >  - kcov_area
> >  - kcov
> >  - kcov_sequence
> >  - kcov_softirq
> >
> > Fields used by the task that requested remote coverage are:
> >
> >  - kcov_remote
> >  - kcov_handle
>
> Thanks, I queued this for testing while we await reviewer input.
>
> checkpatch asked:
>
> WARNING: Using vsprintf specifier '%px' potentially exposes the kernel memory layout, if you don't really need the address please consider using '%p'.
> #141: FILE: kernel/kcov.c:463:
> +               kcov_debug("t = %px, kcov->t = %px\n", t, kcov->t);

Yep, I saw that when running checkpatch but decided to leave it this
way. I didn't introduce that line, I just moved it around; and other
parts of kcov use %px to identify the task as well, so changing just
this spot to %p would be inconsistent. I guess I should have noted
that below the patch...

(And kcov_debug() maps to pr_debug(), so these things will not
normally show up in the kernel log.)

> and Sashiko found a femtobug:
>         https://sashiko.dev/#/patchset/20260505-kcov-simultaneous-remote-v1-1-a670ba7cefd2@xxxxxxxxxx

Hm. In my view, this isn't different from the previous behavior - if
the state is broken, WARN_ON() will print a warning and the function
will bail out immediately without dropping references.

I think if we hit a WARN_ON(), generally this means some state has
been corrupted and/or programmer assumptions have been broken, and it
doesn't really make sense to try to go for full correctness (dropping
references properly and such) because the system is already in a state
the programmer didn't consider. Instead, I think it's best to not
touch the broken state, and prefer leaking memory rather than
potentially making things worse by dropping references (which could
make the situation worse and cause UAF or similar).