Merge tag 'for-linus-7.3-1' of https://github.com/cminyard/linux-ipmi

Pull ipmi updates from Corey Minyard:
 "Several cleanup on error fixes and a missing RCU wait and proper
  validation on a received message in one place.

  The biggest change is the initialization of the driver can be done
  asynchronously on a work queue. That saves significant boot time"

* tag 'for-linus-7.3-1' of https://github.com/cminyard/linux-ipmi:
  ipmi: Fix use-after-free of cmd_rcvr in _ipmi_destroy_user()
  ipmi:msghandler: Cancel work cleanly on an error
  ipmi:si: Add async init to ipmi_si
  char: ipmi: use named initializers for acpi_device_id
  ipmi: Fix leak in __ipmi_bmc_register
  ipmi: Remove all sysfs files on registration failure
  ipmi: si: Fix NULL pointer dereference after failed registration
  ipmi: ipmb: validate write message length

drivers/char/ipmi/Kconfig

@@ -67,6 +67,15 @@ config IPMI_SI
 	  Currently, only KCS and SMIC are supported.  If
 	  you are using IPMI, you should probably say "y" here.
 
+config IPMI_SI_ASYNC_INIT
+	bool 'Asynchronous initialization of IPMI System Interface'
+	depends on IPMI_SI
+	default n
+	help
+	  Offloads individual SMI inits. It speeds up the boot time.
+	  It also introduces a very small risk that something else might fail
+	  if it depends on synchronous IPMI init.
+
 config IPMI_SSIF
 	tristate 'IPMI SMBus handler (SSIF)'
 	depends on I2C

drivers/char/ipmi/ipmb_dev_int.c

@@ -141,13 +141,14 @@ static ssize_t ipmb_write(struct file *file, const char __user *buf,
 	u8 msg[MAX_MSG_LEN];
 	ssize_t ret;
 
-	if (count > sizeof(msg))
+	if (!count || count > sizeof(msg))
 		return -EINVAL;
 
 	if (copy_from_user(&msg, buf, count))
 		return -EFAULT;
 
-	if (count < msg[0])
+	if (msg[IPMB_MSG_LEN_IDX] < IPMB_REQUEST_LEN_MIN ||
+	    count < (size_t)msg[IPMB_MSG_LEN_IDX] + 1)
 		return -EINVAL;
 
 	rq_sa = GET_7BIT_ADDR(msg[RQ_SA_8BIT_IDX]);
@@ -360,8 +361,8 @@ MODULE_DEVICE_TABLE(i2c, ipmb_id);
 
 #ifdef CONFIG_ACPI
 static const struct acpi_device_id acpi_ipmb_id[] = {
-	{ "IPMB0001", 0 },
-	{},
+	{ .id = "IPMB0001" },
+	{ }
 };
 MODULE_DEVICE_TABLE(acpi, acpi_ipmb_id);
 #endif

drivers/char/ipmi/ipmi_msghandler.c

@@ -1391,6 +1391,7 @@ static void _ipmi_destroy_user(struct ipmi_user *user)
 		}
 	}
 	mutex_unlock(&intf->cmd_rcvrs_mutex);
+	synchronize_rcu();
 	while (rcvrs) {
 		rcvr = rcvrs;
 		rcvrs = rcvr->next;
@@ -3301,6 +3302,7 @@ static int __ipmi_bmc_register(struct ipmi_smi *intf,
 	list_del(&intf->bmc_link);
 	mutex_unlock(&bmc->dyn_mutex);
 	intf->bmc = &intf->tmp_bmc;
+	ida_free(&ipmi_bmc_ida, bmc->pdev.id);
 	put_device(&bmc->pdev.dev);
 	goto out;
 }
@@ -3740,6 +3742,7 @@ int ipmi_add_smi(struct module         *owner,
 	sysfs_attr_init(&intf->maintenance_mode_devattr.attr);
 	rv = device_create_file(intf->si_dev, &intf->maintenance_mode_devattr);
 	if (rv) {
+		device_remove_file(intf->si_dev, &intf->nr_msgs_devattr);
 		device_remove_file(intf->si_dev, &intf->nr_users_devattr);
 		goto out_err_bmc_reg;
 	}
@@ -3757,12 +3760,14 @@ int ipmi_add_smi(struct module         *owner,
  out_err_bmc_reg:
 	ipmi_bmc_unregister(intf);
  out_err_started:
+	intf->in_shutdown = true;
 	if (intf->handlers->shutdown)
 		intf->handlers->shutdown(intf->send_info);
  out_err:
 	list_del(&intf->link);
 	mutex_unlock(&ipmi_interfaces_mutex);
 	mutex_unlock(&smi_watchers_mutex);
+	cancel_work_sync(&intf->smi_work);
 	kref_put(&intf->refcount, intf_free);
 
 	return rv;

drivers/char/ipmi/ipmi_si_intf.c

@@ -39,6 +39,7 @@
 #include <linux/rcupdate.h>
 #include <linux/ipmi.h>
 #include <linux/ipmi_smi.h>
+#include <linux/workqueue.h>
 #include "ipmi_si.h"
 #include "ipmi_si_sm.h"
 #include <linux/string.h>
@@ -252,6 +253,8 @@ struct smi_info {
 
 	struct task_struct *thread;
 
+	struct work_struct init_work;
+
 	struct list_head link;
 };
 
@@ -272,6 +275,7 @@ static bool unload_when_empty = true;
 static int try_smi_init(struct smi_info *smi);
 static void cleanup_one_si(struct smi_info *smi_info);
 static void cleanup_ipmi_si(void);
+static void smi_init_work_fn(struct work_struct *work);
 
 #ifdef DEBUG_TIMING
 void debug_timestamp(struct smi_info *smi_info, char *msg)
@@ -1970,6 +1974,7 @@ int ipmi_si_add_smi(struct si_sm_io *io)
 	if (!new_smi)
 		return -ENOMEM;
 	spin_lock_init(&new_smi->si_lock);
+	INIT_WORK(&new_smi->init_work, smi_init_work_fn);
 
 	new_smi->io = *io;
 
@@ -1982,7 +1987,12 @@ int ipmi_si_add_smi(struct si_sm_io *io)
 			dev_info(dup->io.dev,
 				 "Removing SMBIOS-specified %s state machine in favor of ACPI\n",
 				 si_to_str[new_smi->io.si_info->type]);
+			list_del(&dup->link);
+			mutex_unlock(&smi_infos_lock);
+
 			cleanup_one_si(dup);
+
+			mutex_lock(&smi_infos_lock);
 		} else {
 			dev_info(new_smi->io.dev,
 				 "%s-specified %s state machine: duplicate\n",
@@ -2000,8 +2010,12 @@ int ipmi_si_add_smi(struct si_sm_io *io)
 
 	list_add_tail(&new_smi->link, &smi_infos);
 
-	if (initialized)
-		rv = try_smi_init(new_smi);
+	if (initialized) {
+		if (IS_ENABLED(CONFIG_IPMI_SI_ASYNC_INIT))
+			queue_work(system_dfl_wq, &new_smi->init_work);
+		else
+			rv = try_smi_init(new_smi);
+	}
 out_err:
 	mutex_unlock(&smi_infos_lock);
 	return rv;
@@ -2174,6 +2188,15 @@ static bool __init ipmi_smi_info_same(struct smi_info *e1, struct smi_info *e2)
 		e1->io.addr_data == e2->io.addr_data);
 }
 
+static void smi_init_work_fn(struct work_struct *work)
+{
+	struct smi_info *smi = container_of(work, struct smi_info, init_work);
+
+	mutex_lock(&smi_infos_lock);
+	try_smi_init(smi);
+	mutex_unlock(&smi_infos_lock);
+}
+
 static int __init init_ipmi_si(void)
 {
 	struct smi_info *e, *e2;
@@ -2219,8 +2242,12 @@ static int __init init_ipmi_si(void)
 				break;
 			}
 		}
-		if (!dup)
-			try_smi_init(e);
+		if (!dup) {
+			if (IS_ENABLED(CONFIG_IPMI_SI_ASYNC_INIT))
+				queue_work(system_unbound_wq, &e->init_work);
+			else
+				try_smi_init(e);
+		}
 	}
 
 	/*
@@ -2253,8 +2280,12 @@ static int __init init_ipmi_si(void)
 				break;
 			}
 		}
-		if (!dup)
-			try_smi_init(e);
+		if (!dup) {
+			if (IS_ENABLED(CONFIG_IPMI_SI_ASYNC_INIT))
+				queue_work(system_unbound_wq, &e->init_work);
+			else
+				try_smi_init(e);
+		}
 	}
 
 	initialized = true;
@@ -2344,31 +2375,36 @@ static void shutdown_smi(void *send_info)
 }
 
 /*
- * Must be called with smi_infos_lock held, to serialize the
- * smi_info->intf check.
+ * Must be called with smi_info unlinked from smi_infos and smi_infos_lock released.
  */
 static void cleanup_one_si(struct smi_info *smi_info)
 {
 	if (!smi_info)
 		return;
 
-	list_del(&smi_info->link);
+	if (IS_ENABLED(CONFIG_IPMI_SI_ASYNC_INIT))
+		cancel_work_sync(&smi_info->init_work);
+
 	ipmi_unregister_smi(smi_info->intf);
 	kfree(smi_info);
 }
 
 void ipmi_si_remove_by_dev(struct device *dev)
 {
-	struct smi_info *e;
+	struct smi_info *e = NULL, *tmp;
 
 	mutex_lock(&smi_infos_lock);
-	list_for_each_entry(e, &smi_infos, link) {
-		if (e->io.dev == dev) {
-			cleanup_one_si(e);
+	list_for_each_entry(tmp, &smi_infos, link) {
+		if (tmp->io.dev == dev) {
+			e = tmp;
+			list_del(&e->link);
 			break;
 		}
 	}
 	mutex_unlock(&smi_infos_lock);
+
+	if (e)
+		cleanup_one_si(e);
 }
 
 struct device *ipmi_si_remove_by_data(int addr_space, enum si_type si_type,
@@ -2377,6 +2413,7 @@ struct device *ipmi_si_remove_by_data(int addr_space, enum si_type si_type,
 	/* remove */
 	struct smi_info *e, *tmp_e;
 	struct device *dev = NULL;
+	LIST_HEAD(to_clean);
 
 	mutex_lock(&smi_infos_lock);
 	list_for_each_entry_safe(e, tmp_e, &smi_infos, link) {
@@ -2386,17 +2423,23 @@ struct device *ipmi_si_remove_by_data(int addr_space, enum si_type si_type,
 			continue;
 		if (e->io.addr_data == addr) {
 			dev = get_device(e->io.dev);
-			cleanup_one_si(e);
+			list_move_tail(&e->link, &to_clean);
 		}
 	}
 	mutex_unlock(&smi_infos_lock);
 
+	list_for_each_entry_safe(e, tmp_e, &to_clean, link) {
+		list_del(&e->link);
+		cleanup_one_si(e);
+	}
+
 	return dev;
 }
 
 static void cleanup_ipmi_si(void)
 {
 	struct smi_info *e, *tmp_e;
+	LIST_HEAD(to_clean);
 
 	if (!initialized)
 		return;
@@ -2410,10 +2453,14 @@ static void cleanup_ipmi_si(void)
 	ipmi_si_platform_shutdown();
 
 	mutex_lock(&smi_infos_lock);
-	list_for_each_entry_safe(e, tmp_e, &smi_infos, link)
-		cleanup_one_si(e);
+	list_splice_init(&smi_infos, &to_clean);
 	mutex_unlock(&smi_infos_lock);
 
+	list_for_each_entry_safe(e, tmp_e, &to_clean, link) {
+		list_del(&e->link);
+		cleanup_one_si(e);
+	}
+
 	ipmi_si_hardcode_exit();
 	ipmi_si_hotmod_exit();
 }

drivers/char/ipmi/ipmi_si_platform.c

@@ -387,8 +387,8 @@ static int acpi_ipmi_probe(struct platform_device *pdev)
 }
 
 static const struct acpi_device_id acpi_ipmi_match[] = {
-	{ "IPI0001", 0 },
-	{ },
+	{ .id = "IPI0001" },
+	{ }
 };
 MODULE_DEVICE_TABLE(acpi, acpi_ipmi_match);
 #else

drivers/char/ipmi/ipmi_ssif.c

@@ -2057,8 +2057,8 @@ static unsigned short *ssif_address_list(void)
 
 #ifdef CONFIG_ACPI
 static const struct acpi_device_id ssif_acpi_match[] = {
-	{ "IPI0001", 0 },
-	{ },
+	{ .id = "IPI0001" },
+	{ }
 };
 MODULE_DEVICE_TABLE(acpi, ssif_acpi_match);
 #endif