On 6/8/26 10:58 PM, Corey Minyard wrote:
> 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 <[email protected]>
>
> 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
>
Hi Corey,
Thanks for the review and the Acked-by.
Regarding backporting the SRCU removal series: I agree that removing
SRCU entirely would be the cleaner long-term solution. However, as
you noted, that series involves significant refactoring across
multiple functions and would be a relatively large change for a
stable branch. The one-line fix is minimal and addresses the
immediate SRCU imbalance without risking regressions, which seems
more appropriate for a stable backport.
With your Acked-by, I'll ask the stable team to pick this up.
Thanks again,
Rui
>>
>> No changes since v2. The patch is reproduced below for convenience.
>>
>> From: Rui Qi <[email protected]>
>> 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: [email protected]
>> Signed-off-by: Rui Qi <[email protected]>
>>
>> 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);
_______________________________________________
Openipmi-developer mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openipmi-developer
