[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aOT9o-IkilgQXktF@mail.minyard.net>
Date: Tue, 7 Oct 2025 06:46:43 -0500
From: Corey Minyard <corey@...yard.net>
To: Guenter Roeck <linux@...ck-us.net>,
openipmi-developer@...ts.sourceforge.net,
linux-kernel@...r.kernel.org, Eric Dumazet <edumazet@...gle.com>,
Greg Thelen <gthelen@...gle.com>
Subject: Re: [PATCH] ipmi: Fix handling of messages with provided receive
message pointer
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@...gle.com>
> > Cc: Eric Dumazet <edumazet@...gle.com>
> > Cc: Greg Thelen <gthelen@...gle.com>
> > Signed-off-by: Guenter Roeck <linux@...ck-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
> >
Powered by blists - more mailing lists