1. Patchew
  2. linux
  3. View series

[PATCH] ipmi: Fix handling of messages with provided receive message pointer

Guenter Roeck posted 1 patch 1 month, 1 week ago 
drivers/char/ipmi/ipmi_msghandler.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
[PATCH] ipmi: Fix handling of messages with provided receive message pointer
Posted by Guenter Roeck 1 month, 1 week ago 
Prior to commit b52da4054ee0 ("ipmi: Rework user message limit handling"),
i_ipmi_request() used to increase the user reference counter if the receive
message is provided by the caller of IPMI API functions. This is no longer
the case. However, ipmi_free_recv_msg() is still called and decreases the
reference counter. This results in the reference counter reaching zero,
the user data pointer is released, and all kinds of interesting crashes are
seen.

Fix the problem by increasing user reference counter if the receive message
has been provided by the caller.

Fixes: b52da4054ee0 ("ipmi: Rework user message limit handling")
Reported-by: Eric Dumazet <edumazet@google.com>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Greg Thelen <gthelen@google.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/char/ipmi/ipmi_msghandler.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
index a0b67a35a5f0..3700ab4eba3e 100644
--- a/drivers/char/ipmi/ipmi_msghandler.c
+++ b/drivers/char/ipmi/ipmi_msghandler.c
@@ -2301,8 +2301,11 @@ static int i_ipmi_request(struct ipmi_user     *user,
 	if (supplied_recv) {
 		recv_msg = supplied_recv;
 		recv_msg->user = user;
-		if (user)
+		if (user) {
 			atomic_inc(&user->nr_msgs);
+			/* The put happens when the message is freed. */
+			kref_get(&user->refcount);
+		}
 	} else {
 		recv_msg = ipmi_alloc_recv_msg(user);
 		if (IS_ERR(recv_msg))
-- 
2.45.2
Re: [PATCH] ipmi: Fix handling of messages with provided receive message pointer
Posted by Corey Minyard 1 month, 1 week ago 
On Mon, Oct 06, 2025 at 01:18:57PM -0700, Guenter Roeck wrote:
> Prior to commit b52da4054ee0 ("ipmi: Rework user message limit handling"),
> i_ipmi_request() used to increase the user reference counter if the receive
> message is provided by the caller of IPMI API functions. This is no longer
> the case. However, ipmi_free_recv_msg() is still called and decreases the
> reference counter. This results in the reference counter reaching zero,
> the user data pointer is released, and all kinds of interesting crashes are
> seen.
> 
> Fix the problem by increasing user reference counter if the receive message
> has been provided by the caller.

Yes, the only interface that uses this that would matter is the watchdog
timer, which my tests don't currently cover.  I guess I need to add some
tests.

Sorry, and thanks for the fix.  It's queued for next release.

-corey

> 
> Fixes: b52da4054ee0 ("ipmi: Rework user message limit handling")
> Reported-by: Eric Dumazet <edumazet@google.com>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Greg Thelen <gthelen@google.com>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
>  drivers/char/ipmi/ipmi_msghandler.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
> index a0b67a35a5f0..3700ab4eba3e 100644
> --- a/drivers/char/ipmi/ipmi_msghandler.c
> +++ b/drivers/char/ipmi/ipmi_msghandler.c
> @@ -2301,8 +2301,11 @@ static int i_ipmi_request(struct ipmi_user     *user,
>  	if (supplied_recv) {
>  		recv_msg = supplied_recv;
>  		recv_msg->user = user;
> -		if (user)
> +		if (user) {
>  			atomic_inc(&user->nr_msgs);
> +			/* The put happens when the message is freed. */
> +			kref_get(&user->refcount);
> +		}
>  	} else {
>  		recv_msg = ipmi_alloc_recv_msg(user);
>  		if (IS_ERR(recv_msg))
> -- 
> 2.45.2
>
Re: [PATCH] ipmi: Fix handling of messages with provided receive message pointer
Posted by Guenter Roeck 1 month, 1 week ago 
On 10/7/25 04:45, Corey Minyard wrote:
> On Mon, Oct 06, 2025 at 01:18:57PM -0700, Guenter Roeck wrote:
>> Prior to commit b52da4054ee0 ("ipmi: Rework user message limit handling"),
>> i_ipmi_request() used to increase the user reference counter if the receive
>> message is provided by the caller of IPMI API functions. This is no longer
>> the case. However, ipmi_free_recv_msg() is still called and decreases the
>> reference counter. This results in the reference counter reaching zero,
>> the user data pointer is released, and all kinds of interesting crashes are
>> seen.
>>
>> Fix the problem by increasing user reference counter if the receive message
>> has been provided by the caller.
> 
> Yes, the only interface that uses this that would matter is the watchdog
> timer, which my tests don't currently cover.  I guess I need to add some
> tests.
> 

Yes, that is the one that is crashing. Sorry, I should have mentioned that.

> Sorry, and thanks for the fix.  It's queued for next release.
> 

Thanks!

Guenter
Re: [PATCH] ipmi: Fix handling of messages with provided receive message pointer
Posted by Corey Minyard 1 month, 1 week ago 
On Tue, Oct 07, 2025 at 06:46:01AM -0500, Corey Minyard wrote:
> On Mon, Oct 06, 2025 at 01:18:57PM -0700, Guenter Roeck wrote:
> > Prior to commit b52da4054ee0 ("ipmi: Rework user message limit handling"),
> > i_ipmi_request() used to increase the user reference counter if the receive
> > message is provided by the caller of IPMI API functions. This is no longer
> > the case. However, ipmi_free_recv_msg() is still called and decreases the
> > reference counter. This results in the reference counter reaching zero,
> > the user data pointer is released, and all kinds of interesting crashes are
> > seen.
> > 
> > Fix the problem by increasing user reference counter if the receive message
> > has been provided by the caller.
> 
> Yes, the only interface that uses this that would matter is the watchdog
> timer, which my tests don't currently cover.  I guess I need to add some
> tests.
> 
> Sorry, and thanks for the fix.  It's queued for next release.

Sorry, it's queued for this release, 6.18.  I'll send it to Linus
in a few days.

-corey

> 
> -corey
> 
> > 
> > Fixes: b52da4054ee0 ("ipmi: Rework user message limit handling")
> > Reported-by: Eric Dumazet <edumazet@google.com>
> > Cc: Eric Dumazet <edumazet@google.com>
> > Cc: Greg Thelen <gthelen@google.com>
> > Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> > ---
> >  drivers/char/ipmi/ipmi_msghandler.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
> > index a0b67a35a5f0..3700ab4eba3e 100644
> > --- a/drivers/char/ipmi/ipmi_msghandler.c
> > +++ b/drivers/char/ipmi/ipmi_msghandler.c
> > @@ -2301,8 +2301,11 @@ static int i_ipmi_request(struct ipmi_user     *user,
> >  	if (supplied_recv) {
> >  		recv_msg = supplied_recv;
> >  		recv_msg->user = user;
> > -		if (user)
> > +		if (user) {
> >  			atomic_inc(&user->nr_msgs);
> > +			/* The put happens when the message is freed. */
> > +			kref_get(&user->refcount);
> > +		}
> >  	} else {
> >  		recv_msg = ipmi_alloc_recv_msg(user);
> >  		if (IS_ERR(recv_msg))
> > -- 
> > 2.45.2
> >

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.