On Wed, Jul 01, 2026 at 02:43:48AM +0900, Seiji Nishikawa wrote: > try_smi_init() allocates new_smi->si_sm and later calls > ipmi_register_smi_mod(), which maps to ipmi_add_smi(). > > During ipmi_add_smi(), the upper IPMI message handler obtains the > initial BMC device information through __bmc_get_device_id(). This can > fail if the BMC does not return a successful response to the Get Device > ID command. > > When the BMC returns a nonzero completion code, the device-id helper > retries the command and eventually returns -EIO if the device ID still > cannot be fetched. > > On this failure path, ipmi_add_smi() logs "Unable to get the device id" > and goes to out_err_started, where it invokes the lower driver's > shutdown callback. try_smi_init() then logs the returned registration > failure: > > ipmi_si IPI0001:00: IPMI message handler: Unable to get the device id: -5 > ipmi_si IPI0001:00: Unable to register device: error -5 > > For ipmi_si, the shutdown callback is shutdown_smi(), which cleans up > the SI state machine data, frees smi_info->si_sm, and sets > smi_info->si_sm and smi_info->intf to NULL. > > However, intf->in_shutdown is not set on this failed-registration > rollback path. Therefore, the asynchronous redo_bmc_reg work item can > still retry BMC device-id probing after the lower driver has already > cleared its SI state machine data. In the observed case, that retry path > reached start_next_msg(), which passed the NULL smi_info->si_sm pointer > to the selected KCS state machine handler: > > BUG: unable to handle kernel NULL pointer dereference at 0000000000000000 > Workqueue: events redo_bmc_reg [ipmi_msghandler] > RIP: start_kcs_transaction+0x2c/0x190 [ipmi_si] > Call Trace: > start_next_msg+0x50/0x80 [ipmi_si] > check_start_timer_thread.part.9+0x3b/0x50 [ipmi_si] > sender+0x69/0x80 [ipmi_si] > i_ipmi_request+0x2ac/0x9d0 [ipmi_msghandler] > __get_device_id.isra.29+0xaa/0x180 [ipmi_msghandler] > __bmc_get_device_id+0xef/0x950 [ipmi_msghandler] > redo_bmc_reg+0x52/0x60 [ipmi_msghandler] > process_one_work+0x1a7/0x360 > > Set intf->in_shutdown on the out_err_started path before invoking the > lower driver's shutdown callback. This prevents later redo_bmc_reg > retries from using an interface whose lower driver state has been > cleaned up, and applies the same shutdown state to other IPMI interfaces > as well. > > Signed-off-by: Seiji Nishikawa <[email protected]> > --- > Thanks, I agree. I moved the fix to the ipmi_add_smi() rollback path in v2 > and dropped the SI-side NULL checks.
Thanks, this is in my for-next tree. -corey > > Changes in v2: > - Move the fix to ipmi_add_smi() by setting intf->in_shutdown on the > out_err_started path, as suggested by Corey. > - Drop the NULL checks in ipmi_si_intf.c. > > drivers/char/ipmi/ipmi_msghandler.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/char/ipmi/ipmi_msghandler.c > b/drivers/char/ipmi/ipmi_msghandler.c > index ab4c85f3d6fe..8d9f2e647d9b 100644 > --- a/drivers/char/ipmi/ipmi_msghandler.c > +++ b/drivers/char/ipmi/ipmi_msghandler.c > @@ -3757,6 +3757,7 @@ 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: > -- > 2.54.0 > _______________________________________________ Openipmi-developer mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/openipmi-developer
