Re: [PATCH v3] kcov: move kcov_remote_data to task_struct for RT and remove local_lock

[Alexander Potapenko]

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.