linux-kernel - [PATCH 13/23] ipmi:msghandler: Don't acquire a user refcount for queued messages

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250424165020.627193-14-corey@minyard.net>
Date: Thu, 24 Apr 2025 11:49:50 -0500
From: Corey Minyard <corey@...yard.net>
To: linux-kernel@...r.kernel.org,
	openipmi-developer@...ts.sourceforge.net,
	Rik van Riel <riel@...riel.com>
Cc: "Paul E . McKenney" <paulmck@...nel.org>,
	Breno Leitao <leitao@...ian.org>,
	Corey Minyard <corey@...yard.net>,
	Corey Minyard <cminyard@...sta.com>
Subject: [PATCH 13/23] ipmi:msghandler: Don't acquire a user refcount for queued messages

Messages already have a refcount for the user, so there's no need to
account for a new one.

As part of this, grab a refcount to the interface when processing
received messages.  The messages can be freed there, cause the user
then the interface to be freed.

Signed-off-by: Corey Minyard <cminyard@...sta.com>
---
 drivers/char/ipmi/ipmi_msghandler.c | 32 +++++++++++++++--------------
 1 file changed, 17 insertions(+), 15 deletions(-)

diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
index d05032dc1f69..edc53c22ca84 100644
--- a/drivers/char/ipmi/ipmi_msghandler.c
+++ b/drivers/char/ipmi/ipmi_msghandler.c
@@ -958,20 +958,14 @@ static int deliver_response(struct ipmi_smi *intf, struct ipmi_recv_msg *msg)
 		ipmi_free_recv_msg(msg);
 		atomic_dec(&msg->user->nr_msgs);
 	} else {
-		struct ipmi_user *user = acquire_ipmi_user(msg->user);
-
-		if (user) {
-			/* Deliver it in smi_work. */
-			mutex_lock(&intf->user_msgs_mutex);
-			list_add_tail(&msg->link, &intf->user_msgs);
-			mutex_unlock(&intf->user_msgs_mutex);
-			queue_work(system_wq, &intf->smi_work);
-			/* User release will happen in the work queue. */
-		} else {
-			/* User went away, give up. */
-			ipmi_free_recv_msg(msg);
-			rv = -EINVAL;
-		}
+		/*
+		 * Deliver it in smi_work.  The message will hold a
+		 * refcount to the user.
+		 */
+		mutex_lock(&intf->user_msgs_mutex);
+		list_add_tail(&msg->link, &intf->user_msgs);
+		mutex_unlock(&intf->user_msgs_mutex);
+		queue_work(system_wq, &intf->smi_work);
 	}
 
 	return rv;
@@ -4827,6 +4821,13 @@ static void smi_work(struct work_struct *t)
 		mutex_unlock(&intf->users_mutex);
 	}
 
+	/*
+	 * Freeing the message can cause a user to be released, which
+	 * can then cause the interface to be freed.  Make sure that
+	 * doesn't happen until we are ready.
+	 */
+	kref_get(&intf->refcount);
+
 	mutex_lock(&intf->user_msgs_mutex);
 	list_for_each_entry_safe(msg, msg2, &intf->user_msgs, link) {
 		struct ipmi_user *user = msg->user;
@@ -4834,9 +4835,10 @@ static void smi_work(struct work_struct *t)
 		list_del(&msg->link);
 		atomic_dec(&user->nr_msgs);
 		user->handler->ipmi_recv_hndl(msg, user->handler_data);
-		release_ipmi_user(user);
 	}
 	mutex_unlock(&intf->user_msgs_mutex);
+
+	kref_put(&intf->refcount, intf_free);
 }
 
 /* Handle a new message from the lower layer. */
-- 
2.43.0