linux-kernel - Re: [PATCH v2] kcov, usb: Fix invalid context sleep in softirq path on PREEMPT

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <18003f21-f83a-4bad-93b2-70273c03974f@kzalloc.com>
Date: Sun, 3 Aug 2025 12:07:11 +0900
From: Yunseong Kim <ysk@...lloc.com>
To: Hillf Danton <hdanton@...a.com>
Cc: Dmitry Vyukov <dvyukov@...gle.com>,
 Andrey Konovalov <andreyknvl@...il.com>, linux-kernel@...r.kernel.org,
 Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>,
 Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
 Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
 Uladzislau Rezki <urezki@...il.com>
Subject: Re: [PATCH v2] kcov, usb: Fix invalid context sleep in softirq path
 on PREEMPT_RT

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