linux-kernel - Re: [PATCH] ipmi: ipmi_ipmb: Fix null-ptr-deref in 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] [day] [month] [year] [list]

Message-ID: <20220421115332.GZ426325@minyard.net>
Date:   Thu, 21 Apr 2022 06:53:32 -0500
From:   Corey Minyard <minyard@....org>
To:     Wei Yongjun <weiyongjun1@...wei.com>
Cc:     openipmi-developer@...ts.sourceforge.net,
        linux-kernel@...r.kernel.org, Hulk Robot <hulkci@...wei.com>
Subject: Re: [PATCH] ipmi: ipmi_ipmb: Fix null-ptr-deref in
 ipmi_unregister_smi()

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@...wei.com>
> Signed-off-by: Wei Yongjun <weiyongjun1@...wei.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
>