linux-kernel - [PATCH 12/23] ipmi:msghandler: Fix locking around users and interfaces

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-13-corey@minyard.net>
Date: Thu, 24 Apr 2025 11:49:49 -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 12/23] ipmi:msghandler: Fix locking around users and interfaces

Now that SRCU is gone from IPMI, it can no longer be sloppy about
locking.  Use the users mutex now when sending a message, not the big
ipmi_interfaces mutex, because it can result in a recursive lock.  The
users mutex will work because the interface destroy code claims it after
setting the interface in shutdown mode.

Also, due to the same changes, rework the refcounting on users and
interfaces.  Remove the refcount to an interface when the user is
freed, not when it is destroyed.  If the interface is destroyed
while the user still exists, the user will still point to the
interface to test that it is valid if the user tries to do anything
but delete the user.

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

diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
index e7bed764b4bb..d05032dc1f69 100644
--- a/drivers/char/ipmi/ipmi_msghandler.c
+++ b/drivers/char/ipmi/ipmi_msghandler.c
@@ -46,6 +46,7 @@ static void handle_new_recv_msgs(struct ipmi_smi *intf);
 static void need_waiter(struct ipmi_smi *intf);
 static int handle_one_recv_msg(struct ipmi_smi *intf,
 			       struct ipmi_smi_msg *msg);
+static void intf_free(struct kref *ref);
 
 static bool initialized;
 static bool drvregistered;
@@ -196,25 +197,6 @@ struct ipmi_user {
 	atomic_t nr_msgs;
 };
 
-static void free_ipmi_user(struct kref *ref)
-{
-	struct ipmi_user *user = container_of(ref, struct ipmi_user, refcount);
-
-	vfree(user);
-}
-
-static void release_ipmi_user(struct ipmi_user *user)
-{
-	kref_put(&user->refcount, free_ipmi_user);
-}
-
-static struct ipmi_user *acquire_ipmi_user(struct ipmi_user *user)
-{
-	if (!kref_get_unless_zero(&user->refcount))
-		return NULL;
-	return user;
-}
-
 struct cmd_rcvr {
 	struct list_head link;
 
@@ -611,6 +593,28 @@ static int __ipmi_bmc_register(struct ipmi_smi *intf,
 			       bool guid_set, guid_t *guid, int intf_num);
 static int __scan_channels(struct ipmi_smi *intf, struct ipmi_device_id *id);
 
+static void free_ipmi_user(struct kref *ref)
+{
+	struct ipmi_user *user = container_of(ref, struct ipmi_user, refcount);
+	struct module *owner;
+
+	owner = user->intf->owner;
+	kref_put(&user->intf->refcount, intf_free);
+	module_put(owner);
+	vfree(user);
+}
+
+static void release_ipmi_user(struct ipmi_user *user)
+{
+	kref_put(&user->refcount, free_ipmi_user);
+}
+
+static struct ipmi_user *acquire_ipmi_user(struct ipmi_user *user)
+{
+	if (!kref_get_unless_zero(&user->refcount))
+		return NULL;
+	return user;
+}
 
 /*
  * The driver model view of the IPMI messaging driver.
@@ -1330,7 +1334,6 @@ static void _ipmi_destroy_user(struct ipmi_user *user)
 	unsigned long    flags;
 	struct cmd_rcvr  *rcvr;
 	struct cmd_rcvr  *rcvrs = NULL;
-	struct module    *owner;
 
 	if (!refcount_dec_if_one(&user->destroyed))
 		return;
@@ -1382,10 +1385,6 @@ static void _ipmi_destroy_user(struct ipmi_user *user)
 	}
 
 	release_ipmi_user(user);
-
-	owner = intf->owner;
-	kref_put(&intf->refcount, intf_free);
-	module_put(owner);
 }
 
 void ipmi_destroy_user(struct ipmi_user *user)
@@ -2315,7 +2314,7 @@ static int i_ipmi_request(struct ipmi_user     *user,
 		}
 	}
 
-	mutex_lock(&ipmi_interfaces_mutex);
+	mutex_lock(&intf->users_mutex);
 	if (intf->in_shutdown) {
 		rv = -ENODEV;
 		goto out_err;
@@ -2361,7 +2360,7 @@ static int i_ipmi_request(struct ipmi_user     *user,
 
 		smi_send(intf, intf->handlers, smi_msg, priority);
 	}
-	mutex_unlock(&ipmi_interfaces_mutex);
+	mutex_unlock(&intf->users_mutex);
 
 out:
 	if (rv && user)
-- 
2.43.0