Re: [PATCH v2] kcov, usb: Fix invalid context sleep in softirq path on PREEMPT_RT

[Hillf Danton <hdanton@sina.com>]

On Sun, 3 Aug 2025 12:07:11 +0900 Yunseong Kim wrote:
> On 8/3/25 11:34 오전, Hillf Danton wrote:
> > On Sat,  2 Aug 2025 14:26:49 +0000 Yunseong Kim wrote:
> >> +	raw_spin_unlock(&kcov_remote_lock);
> >>  
> >>  	/* Can only happen when in_task(). */
> >>  	if (!area) {
> 
> 		/* 1. Interrupts are temporarily re-enabled here. */
> 
> >> -		local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
> >> +		local_irq_restore(flags);
> 
> 		/* 2. vmalloc() is called safely in a non-atomic context. */
> 
> >>  		area = vmalloc(size * sizeof(unsigned long));
> > 
> > Given irq disabled for the duration of the coverage collection section [1],
> > vmalloc does not work here [2].
> 
> 
> Thank you for the detailed review and for pointing out this critical
> interaction. You are absolutely correct that vmalloc() cannot be called
> from an atomic context with interrupts disabled, as it is a sleeping function.
> 
> However, upon closer inspection of the kcov_remote_start() function's
> control flow, it appears the original author anticipated this issue and
> implemented a safeguard. The vmalloc() call is explicitly wrapped by
> local_irq_restore() and local_irq_save():
> 
> >>  		if (!area) {
> >>  			kcov_put(kcov);
> >>  			return;
> >>  		}
> 
> 		/* 3. Interrupts are disabled again to protect the rest of the function. */
> 
> >> -		local_lock_irqsave(&kcov_percpu_data.lock, flags);
> >> +		local_irq_save(flags);
> >>  	}
> 
> This sequence ensures that the vmalloc() call itself does not happen in an
> IRQ-disabled context.
> 
> My patch reverts the per-CPU locking back to the local_irq_save/restore
> primitives but preserves this essential bracketing around the vmalloc() call.
> Therefore, the sleeping function bug should not occur.
> 
What you missed is the local_irq_save in kcov_remote_start_usb_softirq().
See comment [1] for detail.

> > [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/kcov.h#n73
> > [2] Subject: [RFC 0/7] vmallloc and non-blocking GFPs
> > https://lore.kernel.org/all/20250704152537.55724-1-urezki@gmail.com/
> 
> Additionally, I have tested this implementation by running syzkaller
> for a full day, and no issues were reported.
> 
> Perhaps a comment could be added here (I can volunteer to do so) to
> improve readability where the control flow isn’t obvious to future developers.
> 
> Thanks,
> 
> Yunseong Kim
> 
>