ipmi:msghandler: Fix locking around users and interfaces
authorCorey Minyard <corey@minyard.net>
Fri, 21 Mar 2025 19:46:46 +0000 (14:46 -0500)
committerCorey Minyard <corey@minyard.net>
Wed, 7 May 2025 22:25:48 +0000 (17:25 -0500)
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@mvista.com>
drivers/char/ipmi/ipmi_msghandler.c

index f40f281b46b3394c022762c707b8dacdc74c9d2f..c7533a2846a64fd127c0b12de5c18f768da4ad79 100644 (file)
@@ -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 @@ out_err:
 
                smi_send(intf, intf->handlers, smi_msg, priority);
        }
-       mutex_unlock(&ipmi_interfaces_mutex);
+       mutex_unlock(&intf->users_mutex);
 
 out:
        if (rv && user)