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] [day] [month] [year] [list]

Message-ID: <acd0a040-18ce-05f5-4896-ad24dda6fb00@huaweicloud.com>
Date: Mon, 18 Aug 2025 08:56:01 +0800
From: Yu Kuai <yukuai1@...weicloud.com>
To: Yunseong Kim <ysk@...lloc.com>, Song Liu <song@...nel.org>
Cc: 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,

在 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.

> 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);
>   	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.

Thanks,
Kuai

>   	if (!list_empty(&ppl_conf->no_mem_stripes)) {
>   		struct stripe_head *sh;
>   
> @@ -571,9 +569,7 @@ static void ppl_io_unit_finished(struct ppl_io_unit *io)
>   		set_bit(STRIPE_HANDLE, &sh->state);
>   		raid5_release_stripe(sh);
>   	}
> -	spin_unlock(&ppl_conf->no_mem_stripes_lock);
> -
> -	local_irq_restore(flags);
> +	spin_unlock_irqrestore(&ppl_conf->no_mem_stripes_lock, flags);
>   
>   	wake_up(&conf->wait_for_quiescent);
>   }
>