From 9ca15af3164f3bb84db101fc7843fde25be3288c Mon Sep 17 00:00:00 2001
From: Corey Minyard <cminyard@mvista.com>
Date: Fri, 1 Sep 2017 12:52:20 -0500
Subject: [PATCH] ipmi: Fix issues with BMC refcounts

BMC device refcounts were not being decremented after fetching from
driver_find_device().  Also, document the use of ipmidriver_mutex
and tighten it's span some by incrementing the BMC's usecount in
the BMC find routines and not later.  This will be important for
future changes where a long mutex hold area will complicate things.

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

diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
index efa5581c2f8b..42532f296e93 100644
--- a/drivers/char/ipmi/ipmi_msghandler.c
+++ b/drivers/char/ipmi/ipmi_msghandler.c
@@ -526,6 +526,11 @@ static struct platform_driver ipmidriver = {
 		.bus = &platform_bus_type
 	}
 };
+/*
+ * This mutex protects adding/removing BMCs on the ipmidriver's device
+ * list.  This way we can pull items out of the driver's list and reuse
+ * them.
+ */
 static DEFINE_MUTEX(ipmidriver_mutex);
 
 static LIST_HEAD(ipmi_interfaces);
@@ -2440,16 +2445,23 @@ static int __find_bmc_guid(struct device *dev, void *data)
 	return memcmp(to_bmc_device(dev)->guid, id, 16) == 0;
 }
 
+/*
+ * Must be called with ipmidriver_mutex held.  Returns with the
+ * bmc's usecount incremented, if it is non-NULL.
+ */
 static struct bmc_device *ipmi_find_bmc_guid(struct device_driver *drv,
 					     unsigned char *guid)
 {
 	struct device *dev;
+	struct bmc_device *bmc = NULL;
 
 	dev = driver_find_device(drv, NULL, guid, __find_bmc_guid);
-	if (dev)
-		return to_bmc_device(dev);
-	else
-		return NULL;
+	if (dev) {
+		bmc = to_bmc_device(dev);
+		kref_get(&bmc->usecount);
+		put_device(dev);
+	}
+	return bmc;
 }
 
 struct prod_dev_id {
@@ -2470,6 +2482,10 @@ static int __find_bmc_prod_dev_id(struct device *dev, void *data)
 		&& bmc->id.device_id == id->device_id);
 }
 
+/*
+ * Must be called with ipmidriver_mutex held.  Returns with the
+ * bmc's usecount incremented, if it is non-NULL.
+ */
 static struct bmc_device *ipmi_find_bmc_prod_dev_id(
 	struct device_driver *drv,
 	unsigned int product_id, unsigned char device_id)
@@ -2479,12 +2495,15 @@ static struct bmc_device *ipmi_find_bmc_prod_dev_id(
 		.device_id = device_id,
 	};
 	struct device *dev;
+	struct bmc_device *bmc = NULL;
 
 	dev = driver_find_device(drv, NULL, &id, __find_bmc_prod_dev_id);
-	if (dev)
-		return to_bmc_device(dev);
-	else
-		return NULL;
+	if (dev) {
+		bmc = to_bmc_device(dev);
+		kref_get(&bmc->usecount);
+		put_device(dev);
+	}
+	return bmc;
 }
 
 static void
@@ -2514,8 +2533,8 @@ static void ipmi_bmc_unregister(ipmi_smi_t intf)
 
 	mutex_lock(&ipmidriver_mutex);
 	kref_put(&bmc->usecount, cleanup_bmc_device);
-	intf->bmc = NULL;
 	mutex_unlock(&ipmidriver_mutex);
+	intf->bmc = NULL;
 }
 
 static int ipmi_bmc_register(ipmi_smi_t intf, int ifnum)
@@ -2524,18 +2543,18 @@ static int ipmi_bmc_register(ipmi_smi_t intf, int ifnum)
 	struct bmc_device *bmc = intf->bmc;
 	struct bmc_device *old_bmc;
 
-	mutex_lock(&ipmidriver_mutex);
-
 	/*
 	 * Try to find if there is an bmc_device struct
 	 * representing the interfaced BMC already
 	 */
+	mutex_lock(&ipmidriver_mutex);
 	if (bmc->guid_set)
 		old_bmc = ipmi_find_bmc_guid(&ipmidriver.driver, bmc->guid);
 	else
 		old_bmc = ipmi_find_bmc_prod_dev_id(&ipmidriver.driver,
 						    bmc->id.product_id,
 						    bmc->id.device_id);
+	mutex_unlock(&ipmidriver_mutex);
 
 	/*
 	 * If there is already an bmc_device, free the new one,
@@ -2546,9 +2565,6 @@ static int ipmi_bmc_register(ipmi_smi_t intf, int ifnum)
 		intf->bmc = old_bmc;
 		bmc = old_bmc;
 
-		kref_get(&bmc->usecount);
-		mutex_unlock(&ipmidriver_mutex);
-
 		printk(KERN_INFO
 		       "ipmi: interfacing existing BMC (man_id: 0x%6.6x,"
 		       " prod_id: 0x%4.4x, dev_id: 0x%2.2x)\n",
@@ -2558,14 +2574,17 @@ static int ipmi_bmc_register(ipmi_smi_t intf, int ifnum)
 	} else {
 		unsigned char orig_dev_id = bmc->id.device_id;
 		int warn_printed = 0;
+		struct bmc_device *tmp_bmc;
 
 		snprintf(bmc->name, sizeof(bmc->name),
 			 "ipmi_bmc.%4.4x", bmc->id.product_id);
 		bmc->pdev.name = bmc->name;
 
-		while (ipmi_find_bmc_prod_dev_id(&ipmidriver.driver,
+		mutex_lock(&ipmidriver_mutex);
+		while ((tmp_bmc = ipmi_find_bmc_prod_dev_id(&ipmidriver.driver,
 						 bmc->id.product_id,
-						 bmc->id.device_id)) {
+						 bmc->id.device_id))) {
+			kref_put(&tmp_bmc->usecount, cleanup_bmc_device);
 			if (!warn_printed) {
 				printk(KERN_WARNING PFX
 				       "This machine has two different BMCs"
-- 
GitLab