* [PATCH] ipmi: Fix handling of messages with provided receive message pointer
@ 2025-10-06 20:18 Guenter Roeck
2025-10-07 11:45 ` Corey Minyard
0 siblings, 1 reply; 4+ messages in thread
From: Guenter Roeck @ 2025-10-06 20:18 UTC (permalink / raw)
To: Corey Minyard
Cc: openipmi-developer, linux-kernel, Guenter Roeck, Eric Dumazet,
Greg Thelen
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
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] ipmi: Fix handling of messages with provided receive message pointer
2025-10-06 20:18 [PATCH] ipmi: Fix handling of messages with provided receive message pointer Guenter Roeck
@ 2025-10-07 11:45 ` Corey Minyard
2025-10-07 11:46 ` Corey Minyard
2025-10-07 13:50 ` Guenter Roeck
0 siblings, 2 replies; 4+ messages in thread
From: Corey Minyard @ 2025-10-07 11:45 UTC (permalink / raw)
To: Guenter Roeck; +Cc: openipmi-developer, linux-kernel, Eric Dumazet, Greg Thelen
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
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] ipmi: Fix handling of messages with provided receive message pointer
2025-10-07 11:45 ` Corey Minyard
@ 2025-10-07 11:46 ` Corey Minyard
2025-10-07 13:50 ` Guenter Roeck
1 sibling, 0 replies; 4+ messages in thread
From: Corey Minyard @ 2025-10-07 11:46 UTC (permalink / raw)
To: Guenter Roeck, openipmi-developer, linux-kernel, Eric Dumazet,
Greg Thelen
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
> >
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] ipmi: Fix handling of messages with provided receive message pointer
2025-10-07 11:45 ` Corey Minyard
2025-10-07 11:46 ` Corey Minyard
@ 2025-10-07 13:50 ` Guenter Roeck
1 sibling, 0 replies; 4+ messages in thread
From: Guenter Roeck @ 2025-10-07 13:50 UTC (permalink / raw)
To: corey; +Cc: openipmi-developer, linux-kernel, Eric Dumazet, Greg Thelen
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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-10-07 13:50 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-10-06 20:18 [PATCH] ipmi: Fix handling of messages with provided receive message pointer Guenter Roeck
2025-10-07 11:45 ` Corey Minyard
2025-10-07 11:46 ` Corey Minyard
2025-10-07 13:50 ` Guenter Roeck
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®