Re: [PATCH] ipmi: ipmi_ipmb: Fix null-ptr-deref in ipmi_unregister_smi()

[Corey Minyard <minyard@acm.org>]

On Thu, Apr 21, 2022 at 10:08:35AM +0000, Wei Yongjun wrote:
> KASAN report null-ptr-deref as follows:
> 
> KASAN: null-ptr-deref in range [0x0000000000000008-0x000000000000000f]
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014
> RIP: 0010:ipmi_unregister_smi+0x7d/0xd50 drivers/char/ipmi/ipmi_msghandler.c:3680
> Call Trace:
>  ipmi_ipmb_remove+0x138/0x1a0 drivers/char/ipmi/ipmi_ipmb.c:443
>  ipmi_ipmb_probe+0x409/0xda1 drivers/char/ipmi/ipmi_ipmb.c:548
>  i2c_device_probe+0x959/0xac0 drivers/i2c/i2c-core-base.c:563
>  really_probe+0x3f3/0xa70 drivers/base/dd.c:541
> 
> In ipmi_ipmb_probe(), 'iidev->intf' is not set before ipmi_register_smi() success.
> And in the error handling case, ipmi_ipmb_remove() is called to release resources,
> ipmi_unregister_smi() is called without check 'iidev->intf', this will cause KASAN
> null-ptr-deref issue.
> 
> Fix by adding NULL check prior to calling ipmi_unregister_smi().

This bug is valid, but I'd like to fix it another way.  General kernel
style is to allow NULL to be passed into these sorts of things and just
return if it's NULL.  So I've fixed it that way.  Fix is in linux-next.

Thanks,

-corey

> 
> Fixes: 57c9e3c9a374 ("ipmi:ipmi_ipmb: Unregister the SMI on remove")
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
> ---
>  drivers/char/ipmi/ipmi_ipmb.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/char/ipmi/ipmi_ipmb.c b/drivers/char/ipmi/ipmi_ipmb.c
> index b81b862532fb..ea8fdb5ecfc9 100644
> --- a/drivers/char/ipmi/ipmi_ipmb.c
> +++ b/drivers/char/ipmi/ipmi_ipmb.c
> @@ -437,7 +437,8 @@ static int ipmi_ipmb_remove(struct i2c_client *client)
>  	iidev->client = NULL;
>  	ipmi_ipmb_stop_thread(iidev);
>  
> -	ipmi_unregister_smi(iidev->intf);
> +	if (iidev->intf)
> +		ipmi_unregister_smi(iidev->intf);
>  
>  	return 0;
>  }
> -- 
> 2.25.1
>