Re: [PATCH v2] ipmi: Fix rcu_read_unlock to srcu_read_unlock in handle_read_event_rsp

[Corey Minyard]

On Mon, Jun 08, 2026 at 11:27:54AM +0800, Rui Qi wrote:
> Hi Corey,
> 
> I'm following up on this patch which was originally submitted on
> March 25 and resubmitted as v2 on May 25. I haven't received any
> feedback so far, so I wanted to bring it back to your attention.
> 
> To recap, this is a one-line fix for handle_read_event_rsp() where
> rcu_read_unlock() is incorrectly called instead of srcu_read_unlock()
> on the error path, leaving the SRCU read-side lock held.
> 
> This patch is specifically targeted at stable branches (v6.12 and
> earlier) that still carry the original SRCU-based locking. In
> mainline, commit 3be997d5a64a ("ipmi:msghandler: Remove srcu from
> the ipmi user structure") has already restructured this function to
> use a mutex, effectively eliminating the bug. However, that commit
> is part of a larger SRCU removal series that is not suitable for
> stable backport.
> 
> Since the affected code no longer exists in mainline or your
> for-next tree, this patch cannot follow the usual path of being
> applied there first and then cherry-picked by stable. Could you
> please review and provide an Acked-by so the stable team can pick
> it up directly?

I can give an:

Acked-by: Corey Minyard <corey@xxxxxxxxxxx>

on this, as it is obviously correct.  However, it might be better to
backport the changes removing SRCU.  Using SRCU here was a mistake to
begin with.  But that might be too big a change.

-corey

> 
> No changes since v2. The patch is reproduced below for convenience.
> 
> From: Rui Qi <qirui.001@xxxxxxxxxxxxx>
> Subject: [PATCH v2] ipmi: Fix rcu_read_unlock to srcu_read_unlock in
>  handle_read_event_rsp
> 
> Fix a bug where rcu_read_unlock() was used instead of srcu_read_unlock()
> in handle_read_event_rsp() when ipmi_alloc_recv_msg() fails.
> 
> This mismatch leads to an SRCU read-side critical section imbalance: the
> entry uses srcu_read_lock(&intf->users_srcu) but the error path
> incorrectly calls rcu_read_unlock(), which is a no-op for SRCU and
> leaves the SRCU lock held.
> 
> The offending code was restructured in mainline by commit 3be997d5a64a
> ("ipmi:msghandler: Remove srcu from the ipmi user structure"), which
> replaced the SRCU locking with a mutex in this function, effectively
> eliminating the mismatch. However, that commit is part of a larger
> SRCU removal series that is not suitable for stable backport. This
> minimal fix addresses the SRCU imbalance for 6.12 and earlier stable
> branches that still carry the original locking scheme.
> 
> Fixes: e86ee2d44b44 ("ipmi: Rework locking and shutdown for hot remove")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Rui Qi <qirui.001@xxxxxxxxxxxxx>
> 
>  drivers/char/ipmi/ipmi_msghandler.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
> index 188722ec0337..41ae4dac4eeb 100644
> --- a/drivers/char/ipmi/ipmi_msghandler.c
> +++ b/drivers/char/ipmi/ipmi_msghandler.c
> @@ -4395,7 +4395,7 @@ static int handle_read_event_rsp(struct ipmi_smi *intf,
> 
>  		recv_msg = ipmi_alloc_recv_msg(user);
>  		if (IS_ERR(recv_msg)) {
> -			rcu_read_unlock();
> +			srcu_read_unlock(&intf->users_srcu, index);
>  			list_for_each_entry_safe(recv_msg, recv_msg2, &msgs,
>  						 link) {
>  				list_del(&recv_msg->link);