Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 2512e40e authored by Corey Minyard's avatar Corey Minyard
Browse files

ipmi: Rework SMI registration failure 



There were certain situations where ipmi_register_smi() would
return a failure, but the interface would still be registered
and would need to be unregistered.  This is obviously a bad
design and resulted in an oops in certain failure cases.

If the interface is started up in ipmi_register_smi(), then
an error occurs, shut down the interface there so the
cleanup can be done properly.

Fix the various smi users, too.

Signed-off-by: default avatarCorey Minyard <cminyard@mvista.com>
Reported-by: default avatarJustin Ernst <justin.ernst@hpe.com>
Tested-by: default avatarJustin Ernst <justin.ernst@hpe.com>
Cc: Andrew Banman <abanman@hpe.com>
Cc: Russ Anderson <russ.anderson@hpe.com>
Cc: <stable@vger.kernel.org> # 4.18.x
parent cd2315d4

drivers/char/ipmi/ipmi_msghandler.c

+30 −23
Original line number Diff line number Diff line
@@ -3381,27 +3381,20 @@ int ipmi_register_smi(const struct ipmi_smi_handlers *handlers, 


	rv = handlers->start_processing(send_info, intf);

	if (rv)

		goto out;

		goto out_err;



	rv = __bmc_get_device_id(intf, NULL, &id, NULL, NULL, i);

	if (rv) {

		dev_err(si_dev, "Unable to get the device id: %d\n", rv);

		goto out;

		goto out_err_started;

	}



	mutex_lock(&intf->bmc_reg_mutex);

	rv = __scan_channels(intf, &id);

	mutex_unlock(&intf->bmc_reg_mutex);

	if (rv)

		goto out_err_bmc_reg;



 out:

	if (rv) {

		ipmi_bmc_unregister(intf);

		list_del_rcu(&intf->link);

		mutex_unlock(&ipmi_interfaces_mutex);

		synchronize_srcu(&ipmi_interfaces_srcu);

		cleanup_srcu_struct(&intf->users_srcu);

		kref_put(&intf->refcount, intf_free);

	} else {

	/*

	 * Keep memory order straight for RCU readers.  Make

	 * sure everything else is committed to memory before
@@ -3413,7 +3406,20 @@ int ipmi_register_smi(const struct ipmi_smi_handlers *handlers, 


	/* After this point the interface is legal to use. */

	call_smi_watchers(i, intf->si_dev);

	}



	return 0;



 out_err_bmc_reg:

	ipmi_bmc_unregister(intf);

 out_err_started:

	if (intf->handlers->shutdown)

		intf->handlers->shutdown(intf->send_info);

 out_err:

	list_del_rcu(&intf->link);

	mutex_unlock(&ipmi_interfaces_mutex);

	synchronize_srcu(&ipmi_interfaces_srcu);

	cleanup_srcu_struct(&intf->users_srcu);

	kref_put(&intf->refcount, intf_free);



	return rv;

}
@@ -3504,6 +3510,7 @@ void ipmi_unregister_smi(struct ipmi_smi *intf) 
	}

	srcu_read_unlock(&intf->users_srcu, index);



	if (intf->handlers->shutdown)

		intf->handlers->shutdown(intf->send_info);



	cleanup_smi_msgs(intf);

drivers/char/ipmi/ipmi_si_intf.c

+4 −13
Original line number Diff line number Diff line
@@ -2083,18 +2083,9 @@ static int try_smi_init(struct smi_info *new_smi) 
		 si_to_str[new_smi->io.si_type]);



	WARN_ON(new_smi->io.dev->init_name != NULL);

	kfree(init_name);



	return 0;



 out_err:

	if (new_smi->intf) {

		ipmi_unregister_smi(new_smi->intf);

		new_smi->intf = NULL;

	}



	kfree(init_name);



	return rv;

}
@@ -2227,6 +2218,8 @@ static void shutdown_smi(void *send_info) 


	kfree(smi_info->si_sm);

	smi_info->si_sm = NULL;



	smi_info->intf = NULL;

}



/*
@@ -2240,10 +2233,8 @@ static void cleanup_one_si(struct smi_info *smi_info) 


	list_del(&smi_info->link);



	if (smi_info->intf) {

	if (smi_info->intf)

		ipmi_unregister_smi(smi_info->intf);

		smi_info->intf = NULL;

	}



	if (smi_info->pdev) {

		if (smi_info->pdev_registered)

drivers/char/ipmi/ipmi_ssif.c

+3 −10
Original line number Diff line number Diff line
@@ -1214,18 +1214,11 @@ static void shutdown_ssif(void *send_info) 
		complete(&ssif_info->wake_thread);

		kthread_stop(ssif_info->thread);

	}



	/*

	 * No message can be outstanding now, we have removed the

	 * upper layer and it permitted us to do so.

	 */

	kfree(ssif_info);

}



static int ssif_remove(struct i2c_client *client)

{

	struct ssif_info *ssif_info = i2c_get_clientdata(client);

	struct ipmi_smi *intf;

	struct ssif_addr_info *addr_info;



	if (!ssif_info)
@@ -1235,9 +1228,7 @@ static int ssif_remove(struct i2c_client *client) 
	 * After this point, we won't deliver anything asychronously

	 * to the message handler.  We can unregister ourself.

	 */

	intf = ssif_info->intf;

	ssif_info->intf = NULL;

	ipmi_unregister_smi(intf);

	ipmi_unregister_smi(ssif_info->intf);



	list_for_each_entry(addr_info, &ssif_infos, link) {

		if (addr_info->client == client) {
@@ -1246,6 +1237,8 @@ static int ssif_remove(struct i2c_client *client) 
		}

	}



	kfree(ssif_info);



	return 0;

}