Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Sender: Corey Minyard <tcminyard@gmail.com>
Date:   Thu, 21 Apr 2022 06:53:32 -0500
From:   Corey Minyard <minyard@acm.org>
To:     Wei Yongjun <weiyongjun1@huawei.com>
Cc:     openipmi-developer@lists.sourceforge.net,
        linux-kernel@vger.kernel.org, Hulk Robot <hulkci@huawei.com>
Subject: Re: [PATCH] ipmi: ipmi_ipmb: Fix null-ptr-deref in
 ipmi_unregister_smi()
Message-ID: <20220421115332.GZ426325@minyard.net>
Reply-To: minyard@acm.org
References: <20220421100835.1942677-1-weiyongjun1@huawei.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220421100835.1942677-1-weiyongjun1@huawei.com>
Precedence: bulk

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
>