Messages in this thread |  | | From | Alexander Potapenko <> | | Date | Thu, 21 May 2026 15:06:31 +0200 | | Subject | Re: [PATCH v3] kcov: move kcov_remote_data to task_struct for RT and remove local_lock |
| |
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.
|  |