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

[Yunseong Kim <ysk@kzalloc.com>]

Hi Hillf,

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.

> [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