linux-kernel - Re: [PATCH] md/raid5-ppl: Fix invalid context sleep in ppl_io_unit_finished() 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: <6450c5b4-b3cf-490f-ba7e-6201669829d9@kzalloc.com>
Date: Mon, 18 Aug 2025 20:54:21 +0900
From: Yunseong Kim <ysk@...lloc.com>
To: Yu Kuai <yukuai1@...weicloud.com>, Song Liu <song@...nel.org>
Cc: Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
 Clark Williams <clrkwllms@...nel.org>, Steven Rostedt <rostedt@...dmis.org>,
 "Luis Claudio R. Goncalves" <lgoncalv@...hat.com>,
 linux-raid@...r.kernel.org, linux-rt-devel@...ts.linux.dev,
 linux-kernel@...r.kernel.org, "yukuai (C)" <yukuai3@...wei.com>
Subject: Re: [PATCH] md/raid5-ppl: Fix invalid context sleep in
 ppl_io_unit_finished() on PREEMPT_RT

Hi Yu,

On 8/18/25 9:56 AM, Yu Kuai wrote:
> Hi,
> 
> 在 2025/08/17 19:31, Yunseong Kim 写道:
>> The function ppl_io_unit_finished() uses a local_irq_save()/spin_lock()
>> sequence. On a PREEMPT_RT enabled kernel, spin_lock() can sleep. Calling it
>> with interrupts disabled creates an atomic context where sleeping is
>> forbidden.
>>
> What? I believe spin_lock can never sleep.

I think you might have been a bit surprised by me sending a patch out of
the blue. It would be helpful to refer to the references below:

 On PREEMPT_RT kernels, these lock types are converted to sleeping locks:
  local_lock
  spinlock_t
  rwlock_t

Link: https://docs.kernel.org/locking/locktypes.html#sleeping-locks

>> Ensuring that the interrupt state is managed atomically with the lock
>> itself. The change is applied to both the 'log->io_list_lock' and
>> 'ppl_conf->no_mem_stripes_lock' critical sections within the function.
>>
>> Signed-off-by: Yunseong Kim <ysk@...lloc.com>
>> ---
>>   drivers/md/raid5-ppl.c | 12 ++++--------
>>   1 file changed, 4 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/md/raid5-ppl.c b/drivers/md/raid5-ppl.c
>> index 56b234683ee6..650bd59ead72 100644
>> --- a/drivers/md/raid5-ppl.c
>> +++ b/drivers/md/raid5-ppl.c
>> @@ -553,15 +553,13 @@ static void ppl_io_unit_finished(struct ppl_io_unit *io)
>>         pr_debug("%s: seq: %llu\n", __func__, io->seq);
>>   -    local_irq_save(flags);
>> -
>> -    spin_lock(&log->io_list_lock);
>> +    spin_lock_irqsave(&log->io_list_lock, flags);

 The changes in spinlock_t and rwlock_t semantics on PREEMPT_RT kernels
 have a few implications. For example, on a non-PREEMPT_RT kernel the
 following code sequence works as expected:

 local_irq_disable();
 spin_lock(&lock);

 and is fully equivalent to:

 spin_lock_irq(&lock);

 Same applies to rwlock_t and the _irqsave() suffix variants.

Link: https://docs.kernel.org/locking/locktypes.html#spinlock-t-and-rwlock-t

>>       list_del(&io->log_sibling);
>> -    spin_unlock(&log->io_list_lock);
>> +    spin_unlock_irqrestore(&log->io_list_lock, flags);
>>         mempool_free(io, &ppl_conf->io_pool);
>>   -    spin_lock(&ppl_conf->no_mem_stripes_lock);
>> +    spin_lock_irqsave(&ppl_conf->no_mem_stripes_lock, flags);
> 
> Please notice, local_irq_save + spin_lock is the same as
> spin_lock_irqsave, I don't think your changes have any functonal
> chagnes.

This issue has also been a problem in other subsystems, such as USB:

[BUG] usb: gadget: dummy_hcd: Sleeping function called from invalid
context in dummy_dequeue on PREEMPT_RT

Link: https://lore.kernel.org/lkml/20250816065933.EPwBJ0Sd@linutronix.de/t/#u

I am currently contributing to the kernel to address the areas that need to
adapt to this paradigm shift so that the PREEMPT_RT Linux kernel can be
well supported. I have CC’d other RT people so they can also review
this part.

> Thanks,
> Kuai

Thank you!

Yunseong