Re: [PATCH 2/2] ipmi_ssif: Fix crash seen while ipmi_unregister_smi

[George Cherian]

Hi Corey,


On 08/24/2018 06:38 PM, Corey Minyard wrote:
> On 08/24/2018 06:10 AM, George Cherian wrote:
> > Dont set ssif_info->intf to NULL before ipmi_unresgiter_smi.
> > shutdown_ssif will anyways free ssif_info.
>
> This is correct, but it goes a little deeper. I just sent out a
> patch yesterday that included this.

Yes I saw the patch now, 
https://sourceforge.net/p/openipmi/mailman/message/36397896/
I will test and update in that thread.

> Thanks,
>
> -corey
>
> > Following crash is obsearved if ssif_info->intf is set to NULL
> > before ipmi_unregister_smi.
> >
> > Â CPU: 119 PID: 7317 Comm: kssif000e Not tainted 4.18.0+ #80
> > Â Hardware name: Cavium Inc. Saber/Saber, BIOS Cavium reference 
> > firmware version 7.0 08/04/2018
> > Â pstate: 20400009 (nzCv daif +PAN -UAO)
> > Â pc : ipmi_smi_msg_received+0x44/0x3bc [ipmi_msghandler]
> > Â lr : deliver_recv_msg+0x30/0x5c [ipmi_ssif]
> > Â sp : ffff000037a0fd20
> > Â x29: ffff000037a0fd20 x28: 0000000000000000
> > Â x27: ffff0000047e08f0 x26: ffff800ed9375800
> > Â x25: ffff000037a0fe00 x24: ffff000009073000
> > Â x23: 0000000000000013 x22: 0000000000000000
> > Â x21: 0000000000007000 x20: ffff800adce18400
> > Â x19: 0000000000000000 x18: ffff00003742fd38
> > Â x17: ffff0000089960f0 x16: 000000000000000e
> > Â x15: 0000000000000007 x14: 0000000000000000
> > Â x13: 0000000000000000 x12: 0000000000000033
> > Â x11: 0000000000000381 x10: 0000000000000ba0
> > Â x9 : 0000000000000000 x8 : ffff800ac001fc00
> > Â x7 : ffff7fe003b4d800 x6 : ffff800adce1854b
> > Â x5 : 0000000000000014 x4 : 0000000000000004
> > Â x3 : 0000000000000000 x2 : 0000000000000002
> > Â x1 : 567cb12f8b916b00 x0 : 0000000000000002
> > Â Process kssif000e (pid: 7317, stack limit = 0x0000000041077d8a)
> > Â Call trace:
> > ÂÂ ipmi_smi_msg_received+0x44/0x3bc [ipmi_msghandler]
> > ÂÂ deliver_recv_msg+0x30/0x5c [ipmi_ssif]
> > ÂÂ msg_done_handler+0x2f0/0x66c [ipmi_ssif]
> > ÂÂ ipmi_ssif_thread+0x108/0x124 [ipmi_ssif]
> > ÂÂ kthread+0x108/0x134
> > ÂÂ ret_from_fork+0x10/0x18
> > Â Code: b9402280 91401e75 f90037a1 7100041f (b945bab6)
> > Â ---[ end trace fb7d748bc7b17490 ]---
> > Â Kernel panic - not syncing: Fatal exception
> > Â SMP: stopping secondary CPUs
> > Â Kernel Offset: disabled
> > Â CPU features: 0x23800c38
> > Â Memory Limit: none
> > Â ---[ end Kernel panic - not syncing: Fatal exception ]---
> >
> > Signed-off-by: George Cherian <george.cherian@xxxxxxxxxx>
> > ---
> > Â drivers/char/ipmi/ipmi_ssif.c | 5 +----
> > Â 1 file changed, 1 insertion(+), 4 deletions(-)
> >
> > diff --git a/drivers/char/ipmi/ipmi_ssif.c 
> > b/drivers/char/ipmi/ipmi_ssif.c
> > index ccdf6b1..1490636 100644
> > --- a/drivers/char/ipmi/ipmi_ssif.c
> > +++ b/drivers/char/ipmi/ipmi_ssif.c
> > @@ -1226,7 +1226,6 @@ static void shutdown_ssif(void *send_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)
> > @@ -1236,9 +1235,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) {