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

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

[Rui Qi]

From: "Rui Qi" <qirui.001@bytedance.com>

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@vger.kernel.org
Signed-off-by: Rui Qi <qirui.001@bytedance.com>
---
 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);
--
2.20.1

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

[Rui Qi]

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?

No changes since v2. The patch is reproduced below for convenience.

From: Rui Qi <qirui.001@bytedance.com>
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@vger.kernel.org
Signed-off-by: Rui Qi <qirui.001@bytedance.com>

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

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@minyard.net>

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@bytedance.com>
> 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@vger.kernel.org
> Signed-off-by: Rui Qi <qirui.001@bytedance.com>
> 
>  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);

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

[Rui Qi]

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 <corey@minyard.net>
> 
> 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 <qirui.001@bytedance.com>
>> 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@vger.kernel.org
>> Signed-off-by: Rui Qi <qirui.001@bytedance.com>
>>
>>  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);

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

[Greg Kroah-Hartman]

On Mon, May 25, 2026 at 02:32:35PM +0800, Rui Qi wrote:
> From: "Rui Qi" <qirui.001@bytedance.com>
> 
> 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@vger.kernel.org
> Signed-off-by: Rui Qi <qirui.001@bytedance.com>
> ---
>  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);
> --
> 2.20.1
> 

Does not apply to the tree, please rebase and resend.

thanks,

greg k-h