linux-kernel - Re: [PATCH 2/2] ipmi_ssif: Fix crash seen while ipmi_unregister

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 24 Aug 2018 08:08:47 -0500
From:   Corey Minyard <minyard@....org>
To:     George Cherian <george.cherian@...ium.com>,
        linux-kernel@...r.kernel.org,
        openipmi-developer@...ts.sourceforge.net
Cc:     arnd@...db.de, gregkh@...uxfoundation.org
Subject: Re: [PATCH 2/2] ipmi_ssif: Fix crash seen while ipmi_unregister_smi

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.

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@...ium.com>
> ---
>   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) {