I missed the merge window, which wasn't really important at the time

as there was nothing that critical that I had for 5.0.
 
 However, I say that,and then a number of critical fixes come in:
 
   ipmi: fix use-after-free of user->release_barrier.rda
   ipmi: Prevent use-after-free in deliver_response
   ipmi: msghandler: Fix potential Spectre v1 vulnerabilities
 
 which are obvious candidates for 5.0.  Then there is:
 
   ipmi:ssif: Fix handling of multi-part return messages
 
 which is less critical, but it still has some off-by-one things that
 are not great, so it seemed appropriate.  Some machines are broken
 without it.  Then:
 
   ipmi: Don't initialize anything in the core until something uses it
 
 It turns out that using SRCU causes large chunks of memory to be used
 on big iron machines, even if IPMI is never used.  This was causing
 some issues for people on those machines.
 
 Everything here is destined for stable.
 -----BEGIN PGP SIGNATURE-----
 
 iQIzBAABCgAdFiEE/Q1c5nzg9ZpmiCaGYfOMkJGb/4EFAlxIojsACgkQYfOMkJGb
 /4H+pg/+PCgbRVGujXyQsc73zmO4Mfo987bJV/G4U5Xvk4rNFczZ58iyY33LDTjy
 itfFbQQgJSEiVH5rnWOIjwMizhwKbLcXwzhdIBc9qpyPthNZxyU+GcuaAuueV9Hh
 9n5OpwmsHPUfHpaaJbZfUJgQXTqUd3L5PacYfM9kQp4cIHobUzbeM0C1Sl4L3m5g
 XQviOHClDj4bkrGGgXEnpLk+4rstDECX6el1VIF8tmOS+rAOc6SZmTCtaO+eMs7h
 Q1snbW/lW1/92nWiiBYnsKlHb34wumb7Ym3KZl4kR6HFexjkKMK4cRlG2ylddfjV
 vdauaOBz4h3BiujeOiaEAl+l0dB6zBBL7AtDHFsvEOhtB0veX+3y/puWYa6gS7ra
 829d4i5a/Dpci3FnDB9PnxvVbWnU5j56mKpxl7BsILJfw7kahUf5rfoePTfOrOUH
 yZB4e/zYRDT7bX4ZBlVfEzMjb2QT+RVRuMIdZX2ltLlfz8pUT35he0//9INuOedg
 ETm7lnlo6MuUQwbADosAJzVEDf2Z7VKUE495wZBtKa/IlTOZrZqNV9b4lpLQeVLQ
 EnDPz3XxVnGu/hOkv+c6VRJxgXdQExzdR90eHb8yRa9ZbFHghl4biWDbWnqkHIHN
 L4QjFgEMQkdKLg0xyxMJPFs0bljxD2IUyUnTlbN/mhOKm4zZJ4c=
 =qPvR
 -----END PGP SIGNATURE-----

Merge tag 'for-linus-5.0' of git://github.com/cminyard/linux-ipmi

Pull IPMI fixes from Corey Minyard:
 "I missed the merge window, which wasn't really important at the time
  as there was nothing that critical that I had for 5.0.

  However, I say that,and then a number of critical fixes come in:

   - ipmi: fix use-after-free of user->release_barrier.rda
   - ipmi: Prevent use-after-free in deliver_response
   - ipmi: msghandler: Fix potential Spectre v1 vulnerabilities

  which are obvious candidates for 5.0.  Then there is:

   - ipmi:ssif: Fix handling of multi-part return messages

  which is less critical, but it still has some off-by-one things that
  are not great, so it seemed appropriate. Some machines are broken
  without it. Then:

   - ipmi: Don't initialize anything in the core until something uses it

  It turns out that using SRCU causes large chunks of memory to be used
  on big iron machines, even if IPMI is never used. This was causing
  some issues for people on those machines.

  Everything here is destined for stable"

* tag 'for-linus-5.0' of git://github.com/cminyard/linux-ipmi:
  ipmi: Don't initialize anything in the core until something uses it
  ipmi: fix use-after-free of user->release_barrier.rda
  ipmi: Prevent use-after-free in deliver_response
  ipmi: msghandler: Fix potential Spectre v1 vulnerabilities
  ipmi:ssif: Fix handling of multi-part return messages

drivers/char/ipmi/ipmi_msghandler.c

@@ -32,6 +32,7 @@
 #include <linux/moduleparam.h>
 #include <linux/workqueue.h>
 #include <linux/uuid.h>
+#include <linux/nospec.h>
 
 #define IPMI_DRIVER_VERSION "39.2"
 
@@ -62,7 +63,8 @@ static void ipmi_debug_msg(const char *title, unsigned char *data,
 { }
 #endif
 
-static int initialized;
+static bool initialized;
+static bool drvregistered;
 
 enum ipmi_panic_event_op {
 	IPMI_SEND_PANIC_EVENT_NONE,
@@ -612,7 +614,7 @@ static DEFINE_MUTEX(ipmidriver_mutex);
 
 static LIST_HEAD(ipmi_interfaces);
 static DEFINE_MUTEX(ipmi_interfaces_mutex);
-DEFINE_STATIC_SRCU(ipmi_interfaces_srcu);
+struct srcu_struct ipmi_interfaces_srcu;
 
 /*
  * List of watchers that want to know when smi's are added and deleted.
@@ -720,7 +722,15 @@ struct watcher_entry {
 int ipmi_smi_watcher_register(struct ipmi_smi_watcher *watcher)
 {
 	struct ipmi_smi *intf;
-	int index;
+	int index, rv;
+
+	/*
+	 * Make sure the driver is actually initialized, this handles
+	 * problems with initialization order.
+	 */
+	rv = ipmi_init_msghandler();
+	if (rv)
+		return rv;
 
 	mutex_lock(&smi_watchers_mutex);
 
@@ -884,7 +894,7 @@ static int deliver_response(struct ipmi_smi *intf, struct ipmi_recv_msg *msg)
 
 		if (user) {
 			user->handler->ipmi_recv_hndl(msg, user->handler_data);
-			release_ipmi_user(msg->user, index);
+			release_ipmi_user(user, index);
 		} else {
 			/* User went away, give up. */
 			ipmi_free_recv_msg(msg);
@@ -1076,7 +1086,7 @@ int ipmi_create_user(unsigned int          if_num,
 {
 	unsigned long flags;
 	struct ipmi_user *new_user;
-	int           rv = 0, index;
+	int           rv, index;
 	struct ipmi_smi *intf;
 
 	/*
@@ -1094,18 +1104,9 @@ int ipmi_create_user(unsigned int          if_num,
 	 * Make sure the driver is actually initialized, this handles
 	 * problems with initialization order.
 	 */
-	if (!initialized) {
+	rv = ipmi_init_msghandler();
-		rv = ipmi_init_msghandler();
+	if (rv)
-		if (rv)
+		return rv;
-			return rv;
-
-		/*
-		 * The init code doesn't return an error if it was turned
-		 * off, but it won't initialize.  Check that.
-		 */
-		if (!initialized)
-			return -ENODEV;
-	}
 
 	new_user = kmalloc(sizeof(*new_user), GFP_KERNEL);
 	if (!new_user)
@@ -1183,6 +1184,7 @@ EXPORT_SYMBOL(ipmi_get_smi_info);
 static void free_user(struct kref *ref)
 {
 	struct ipmi_user *user = container_of(ref, struct ipmi_user, refcount);
+	cleanup_srcu_struct(&user->release_barrier);
 	kfree(user);
 }
 
@@ -1259,7 +1261,6 @@ int ipmi_destroy_user(struct ipmi_user *user)
 {
 	_ipmi_destroy_user(user);
 
-	cleanup_srcu_struct(&user->release_barrier);
 	kref_put(&user->refcount, free_user);
 
 	return 0;
@@ -1298,10 +1299,12 @@ int ipmi_set_my_address(struct ipmi_user *user,
 	if (!user)
 		return -ENODEV;
 
-	if (channel >= IPMI_MAX_CHANNELS)
+	if (channel >= IPMI_MAX_CHANNELS) {
 		rv = -EINVAL;
-	else
+	} else {
+		channel = array_index_nospec(channel, IPMI_MAX_CHANNELS);
 		user->intf->addrinfo[channel].address = address;
+	}
 	release_ipmi_user(user, index);
 
 	return rv;
@@ -1318,10 +1321,12 @@ int ipmi_get_my_address(struct ipmi_user *user,
 	if (!user)
 		return -ENODEV;
 
-	if (channel >= IPMI_MAX_CHANNELS)
+	if (channel >= IPMI_MAX_CHANNELS) {
 		rv = -EINVAL;
-	else
+	} else {
+		channel = array_index_nospec(channel, IPMI_MAX_CHANNELS);
 		*address = user->intf->addrinfo[channel].address;
+	}
 	release_ipmi_user(user, index);
 
 	return rv;
@@ -1338,10 +1343,12 @@ int ipmi_set_my_LUN(struct ipmi_user *user,
 	if (!user)
 		return -ENODEV;
 
-	if (channel >= IPMI_MAX_CHANNELS)
+	if (channel >= IPMI_MAX_CHANNELS) {
 		rv = -EINVAL;
-	else
+	} else {
+		channel = array_index_nospec(channel, IPMI_MAX_CHANNELS);
 		user->intf->addrinfo[channel].lun = LUN & 0x3;
+	}
 	release_ipmi_user(user, index);
 
 	return rv;
@@ -1358,10 +1365,12 @@ int ipmi_get_my_LUN(struct ipmi_user *user,
 	if (!user)
 		return -ENODEV;
 
-	if (channel >= IPMI_MAX_CHANNELS)
+	if (channel >= IPMI_MAX_CHANNELS) {
 		rv = -EINVAL;
-	else
+	} else {
+		channel = array_index_nospec(channel, IPMI_MAX_CHANNELS);
 		*address = user->intf->addrinfo[channel].lun;
+	}
 	release_ipmi_user(user, index);
 
 	return rv;
@@ -2184,6 +2193,7 @@ static int check_addr(struct ipmi_smi  *intf,
 {
 	if (addr->channel >= IPMI_MAX_CHANNELS)
 		return -EINVAL;
+	addr->channel = array_index_nospec(addr->channel, IPMI_MAX_CHANNELS);
 	*lun = intf->addrinfo[addr->channel].lun;
 	*saddr = intf->addrinfo[addr->channel].address;
 	return 0;
@@ -3291,17 +3301,9 @@ int ipmi_register_smi(const struct ipmi_smi_handlers *handlers,
 	 * Make sure the driver is actually initialized, this handles
 	 * problems with initialization order.
 	 */
-	if (!initialized) {
+	rv = ipmi_init_msghandler();
-		rv = ipmi_init_msghandler();
+	if (rv)
-		if (rv)
+		return rv;
-			return rv;
-		/*
-		 * The init code doesn't return an error if it was turned
-		 * off, but it won't initialize.  Check that.
-		 */
-		if (!initialized)
-			return -ENODEV;
-	}
 
 	intf = kzalloc(sizeof(*intf), GFP_KERNEL);
 	if (!intf)
@@ -5017,6 +5019,22 @@ static int panic_event(struct notifier_block *this,
 	return NOTIFY_DONE;
 }
 
+/* Must be called with ipmi_interfaces_mutex held. */
+static int ipmi_register_driver(void)
+{
+	int rv;
+
+	if (drvregistered)
+		return 0;
+
+	rv = driver_register(&ipmidriver.driver);
+	if (rv)
+		pr_err("Could not register IPMI driver\n");
+	else
+		drvregistered = true;
+	return rv;
+}
+
 static struct notifier_block panic_block = {
 	.notifier_call	= panic_event,
 	.next		= NULL,
@@ -5027,66 +5045,75 @@ static int ipmi_init_msghandler(void)
 {
 	int rv;
 
+	mutex_lock(&ipmi_interfaces_mutex);
+	rv = ipmi_register_driver();
+	if (rv)
+		goto out;
 	if (initialized)
-		return 0;
+		goto out;
 
-	rv = driver_register(&ipmidriver.driver);
+	init_srcu_struct(&ipmi_interfaces_srcu);
-	if (rv) {
-		pr_err("Could not register IPMI driver\n");
-		return rv;
-	}
-
-	pr_info("version " IPMI_DRIVER_VERSION "\n");
 
 	timer_setup(&ipmi_timer, ipmi_timeout, 0);
 	mod_timer(&ipmi_timer, jiffies + IPMI_TIMEOUT_JIFFIES);
 
 	atomic_notifier_chain_register(&panic_notifier_list, &panic_block);
 
-	initialized = 1;
+	initialized = true;
 
-	return 0;
+out:
+	mutex_unlock(&ipmi_interfaces_mutex);
+	return rv;
 }
 
 static int __init ipmi_init_msghandler_mod(void)
 {
-	ipmi_init_msghandler();
+	int rv;
-	return 0;
+
+	pr_info("version " IPMI_DRIVER_VERSION "\n");
+
+	mutex_lock(&ipmi_interfaces_mutex);
+	rv = ipmi_register_driver();
+	mutex_unlock(&ipmi_interfaces_mutex);
+
+	return rv;
 }
 
 static void __exit cleanup_ipmi(void)
 {
 	int count;
 
-	if (!initialized)
+	if (initialized) {
-		return;
+		atomic_notifier_chain_unregister(&panic_notifier_list,
+						 &panic_block);
 
-	atomic_notifier_chain_unregister(&panic_notifier_list, &panic_block);
+		/*
+		 * This can't be called if any interfaces exist, so no worry
+		 * about shutting down the interfaces.
+		 */
 
-	/*
+		/*
-	 * This can't be called if any interfaces exist, so no worry
+		 * Tell the timer to stop, then wait for it to stop.  This
-	 * about shutting down the interfaces.
+		 * avoids problems with race conditions removing the timer
-	 */
+		 * here.
+		 */
+		atomic_inc(&stop_operation);
+		del_timer_sync(&ipmi_timer);
 
-	/*
+		initialized = false;
-	 * Tell the timer to stop, then wait for it to stop.  This
-	 * avoids problems with race conditions removing the timer
-	 * here.
-	 */
-	atomic_inc(&stop_operation);
-	del_timer_sync(&ipmi_timer);
 
-	driver_unregister(&ipmidriver.driver);
+		/* Check for buffer leaks. */
+		count = atomic_read(&smi_msg_inuse_count);
+		if (count != 0)
+			pr_warn("SMI message count %d at exit\n", count);
+		count = atomic_read(&recv_msg_inuse_count);
+		if (count != 0)
+			pr_warn("recv message count %d at exit\n", count);
 
-	initialized = 0;
+		cleanup_srcu_struct(&ipmi_interfaces_srcu);
-
+	}
-	/* Check for buffer leaks. */
+	if (drvregistered)
-	count = atomic_read(&smi_msg_inuse_count);
+		driver_unregister(&ipmidriver.driver);
-	if (count != 0)
-		pr_warn("SMI message count %d at exit\n", count);
-	count = atomic_read(&recv_msg_inuse_count);
-	if (count != 0)
-		pr_warn("recv message count %d at exit\n", count);
 }
 module_exit(cleanup_ipmi);
 

drivers/char/ipmi/ipmi_ssif.c

@@ -632,8 +632,9 @@ static void msg_done_handler(struct ssif_info *ssif_info, int result,
 
 		/* Remove the multi-part read marker. */
 		len -= 2;
+		data += 2;
 		for (i = 0; i < len; i++)
-			ssif_info->data[i] = data[i+2];
+			ssif_info->data[i] = data[i];
 		ssif_info->multi_len = len;
 		ssif_info->multi_pos = 1;
 
@@ -661,8 +662,19 @@ static void msg_done_handler(struct ssif_info *ssif_info, int result,
 		}
 
 		blocknum = data[0];
+		len--;
+		data++;
 
-		if (ssif_info->multi_len + len - 1 > IPMI_MAX_MSG_LENGTH) {
+		if (blocknum != 0xff && len != 31) {
+		    /* All blocks but the last must have 31 data bytes. */
+			result = -EIO;
+			if (ssif_info->ssif_debug & SSIF_DEBUG_MSG)
+				pr_info("Received middle message <31\n");
+
+			goto continue_op;
+		}
+
+		if (ssif_info->multi_len + len > IPMI_MAX_MSG_LENGTH) {
 			/* Received message too big, abort the operation. */
 			result = -E2BIG;
 			if (ssif_info->ssif_debug & SSIF_DEBUG_MSG)
@@ -671,16 +683,14 @@ static void msg_done_handler(struct ssif_info *ssif_info, int result,
 			goto continue_op;
 		}
 
-		/* Remove the blocknum from the data. */
-		len--;
 		for (i = 0; i < len; i++)
-			ssif_info->data[i + ssif_info->multi_len] = data[i + 1];
+			ssif_info->data[i + ssif_info->multi_len] = data[i];
 		ssif_info->multi_len += len;
 		if (blocknum == 0xff) {
 			/* End of read */
 			len = ssif_info->multi_len;
 			data = ssif_info->data;
-		} else if (blocknum + 1 != ssif_info->multi_pos) {
+		} else if (blocknum != ssif_info->multi_pos) {
 			/*
 			 * Out of sequence block, just abort.  Block
 			 * numbers start at zero for the second block,
@@ -707,6 +717,7 @@ static void msg_done_handler(struct ssif_info *ssif_info, int result,
 		}
 	}
 
+ continue_op:
 	if (result < 0) {
 		ssif_inc_stat(ssif_info, receive_errors);
 	} else {
@@ -714,8 +725,6 @@ static void msg_done_handler(struct ssif_info *ssif_info, int result,
 		ssif_inc_stat(ssif_info, received_message_parts);
 	}
 
-
- continue_op:
 	if (ssif_info->ssif_debug & SSIF_DEBUG_STATE)
 		pr_info("DONE 1: state = %d, result=%d\n",
 			ssif_info->ssif_state, result);