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

[Dmitry Vyukov]

On Tue, 5 May 2026 at 11:01, 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
>
> Signed-off-by: Jann Horn <jannh@xxxxxxxxxx>
> ---
> resending because the previous recipient list was incomplete
> ---
>  include/linux/sched.h |  3 ++
>  kernel/kcov.c         | 94 ++++++++++++++++++++++++++++-----------------------
>  2 files changed, 55 insertions(+), 42 deletions(-)
>
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index 368c7b4d7cb5..c3bc248a0d27 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -1514,6 +1514,9 @@ struct task_struct {
>         /* KCOV descriptor wired with this task or NULL: */
>         struct kcov                     *kcov;
>
> +       /* KCOV descriptor for remote coverage collection from other tasks: */
> +       struct kcov                     *kcov_remote;
> +
>         /* KCOV common handle for remote coverage collection: */
>         u64                             kcov_handle;
>
> diff --git a/kernel/kcov.c b/kernel/kcov.c
> index 0b369e88c7c9..bcbde917d4df 100644
> --- a/kernel/kcov.c
> +++ b/kernel/kcov.c
> @@ -368,6 +368,7 @@ static void kcov_start(struct task_struct *t, struct kcov *kcov,
>         WRITE_ONCE(t->kcov_mode, mode);
>  }
>
> +/* operates on coverage-generator-owned fields */
>  static void kcov_stop(struct task_struct *t)
>  {
>         WRITE_ONCE(t->kcov_mode, KCOV_MODE_DISABLED);
> @@ -377,16 +378,17 @@ static void kcov_stop(struct task_struct *t)
>         t->kcov_area = NULL;
>  }
>
> +/* operates on coverage-generator-owned fields */
>  static void kcov_task_reset(struct task_struct *t)
>  {
>         kcov_stop(t);
>         t->kcov_sequence = 0;
> -       t->kcov_handle = 0;
>  }
>
>  void kcov_task_init(struct task_struct *t)
>  {
>         kcov_task_reset(t);
> +       t->kcov_remote = NULL;
>         t->kcov_handle = current->kcov_handle;
>  }
>
> @@ -423,11 +425,14 @@ static void kcov_remote_reset(struct kcov *kcov)
>  static void kcov_disable(struct task_struct *t, struct kcov *kcov)
>         __must_hold(&kcov->lock)
>  {
> -       kcov_task_reset(t);
> -       if (kcov->remote)
> +       if (kcov->remote) {
> +               t->kcov_handle = 0;
> +               t->kcov_remote = NULL;
>                 kcov_remote_reset(kcov);
> -       else
> +       } else {
> +               kcov_task_reset(t);
>                 kcov_reset(kcov);
> +       }
>  }
>
>  static void kcov_get(struct kcov *kcov)
> @@ -453,41 +458,47 @@ void kcov_task_exit(struct task_struct *t)
>         unsigned long flags;
>
>         kcov = t->kcov;
> -       if (kcov == NULL)
> -               return;
> -
> -       spin_lock_irqsave(&kcov->lock, flags);
> -       kcov_debug("t = %px, kcov->t = %px\n", t, kcov->t);
> -       /*
> -        * For KCOV_ENABLE devices we want to make sure that t->kcov->t == t,
> -        * which comes down to:
> -        *        WARN_ON(!kcov->remote && kcov->t != t);
> -        *
> -        * For KCOV_REMOTE_ENABLE devices, the exiting task is either:
> -        *
> -        * 1. A remote task between kcov_remote_start() and kcov_remote_stop().
> -        *    In this case we should print a warning right away, since a task
> -        *    shouldn't be exiting when it's in a kcov coverage collection
> -        *    section. Here t points to the task that is collecting remote
> -        *    coverage, and t->kcov->t points to the thread that created the
> -        *    kcov device. Which means that to detect this case we need to
> -        *    check that t != t->kcov->t, and this gives us the following:
> -        *        WARN_ON(kcov->remote && kcov->t != t);
> -        *
> -        * 2. The task that created kcov exiting without calling KCOV_DISABLE,
> -        *    and then again we make sure that t->kcov->t == t:
> -        *        WARN_ON(kcov->remote && kcov->t != t);
> -        *
> -        * By combining all three checks into one we get:
> -        */
> -       if (WARN_ON(kcov->t != t)) {
> +       if (kcov) {
> +               spin_lock_irqsave(&kcov->lock, flags);
> +               kcov_debug("t = %px, kcov->t = %px\n", t, kcov->t);
> +               /*
> +                * This could be a remote task between kcov_remote_start() and
> +                * kcov_remote_stop().
> +                * In this case we should print a warning right away, since a
> +                * task shouldn't be exiting when it's in a kcov coverage
> +                * collection section.
> +                *
> +                * Otherwise, this should be a task that created a local
> +                * kcov instance and hasn't called KCOV_DISABLE.
> +                * Make sure that t->kcov->t is consistent.
> +                */
> +               if (WARN_ON(kcov->remote) || WARN_ON(kcov->t != t)) {
> +                       spin_unlock_irqrestore(&kcov->lock, flags);
> +                       return;
> +               }
> +               /* Just to not leave dangling references behind. */
> +               kcov_disable(t, kcov);
>                 spin_unlock_irqrestore(&kcov->lock, flags);
> -               return;
> +               kcov_put(kcov);
> +       }
> +       kcov = t->kcov_remote;
> +       if (kcov) {
> +               spin_lock_irqsave(&kcov->lock, flags);
> +               kcov_debug("t = %px, kcov->t = %px\n", t, kcov->t);
> +               /*
> +                * This is a KCOV_REMOTE_ENABLE device, and the task is the
> +                * user task which has requested remote coverage collection.
> +                * Make sure that t->kcov->t is consistent.
> +                */
> +               if (WARN_ON(!kcov->remote) || WARN_ON(kcov->t != t)) {
> +                       spin_unlock_irqrestore(&kcov->lock, flags);
> +                       return;
> +               }
> +               /* Just to not leave dangling references behind. */
> +               kcov_disable(t, kcov);
> +               spin_unlock_irqrestore(&kcov->lock, flags);
> +               kcov_put(kcov);
>         }
> -       /* Just to not leave dangling references behind. */
> -       kcov_disable(t, kcov);
> -       spin_unlock_irqrestore(&kcov->lock, flags);
> -       kcov_put(kcov);
>  }
>
>  static int kcov_mmap(struct file *filep, struct vm_area_struct *vma)
> @@ -629,9 +640,9 @@ static int kcov_ioctl_locked(struct kcov *kcov, unsigned int cmd,
>         case KCOV_DISABLE:
>                 /* Disable coverage for the current task. */
>                 unused = arg;
> -               if (unused != 0 || current->kcov != kcov)
> -                       return -EINVAL;
>                 t = current;
> +               if (unused != 0 || (kcov != t->kcov && kcov != t->kcov_remote))
> +                       return -EINVAL;
>                 if (WARN_ON(kcov->t != t))
>                         return -EINVAL;
>                 kcov_disable(t, kcov);
> @@ -641,7 +652,7 @@ static int kcov_ioctl_locked(struct kcov *kcov, unsigned int cmd,
>                 if (kcov->mode != KCOV_MODE_INIT || !kcov->area)
>                         return -EINVAL;
>                 t = current;
> -               if (kcov->t != NULL || t->kcov != NULL)
> +               if (kcov->t != NULL || t->kcov_remote != NULL)
>                         return -EBUSY;
>                 remote_arg = (struct kcov_remote_arg *)arg;
>                 mode = kcov_get_mode(remote_arg->trace_mode);
> @@ -651,8 +662,7 @@ static int kcov_ioctl_locked(struct kcov *kcov, unsigned int cmd,
>                     LONG_MAX / sizeof(unsigned long))
>                         return -EINVAL;
>                 kcov->mode = mode;
> -               t->kcov = kcov;
> -               t->kcov_mode = KCOV_MODE_REMOTE;

KCOV_MODE_REMOTE does not seem to be used anywhere else now.
Please remove it.

Otherwise, this looks good to me (though, all the logic here is pretty
involved).

Reviewed-by: Dmitry Vyukov <dvyukov@xxxxxxxxxx>

FTR, I've uploaded the change to gerrit for easier side-by-side and
full context review:
https://linux-review.googlesource.com/c/linux/kernel/git/torvalds/linux/+/26742


> +               t->kcov_remote = kcov;
>                 kcov->t = t;
>                 kcov->remote = true;
>                 kcov->remote_size = remote_arg->area_size;
>
> ---
> base-commit: 57b8e2d666a31fa201432d58f5fe3469a0dd83ba
> change-id: 20260429-kcov-simultaneous-remote-a1bc6cf0a01b
>
> --
> Jann Horn <jannh@xxxxxxxxxx>
>