[PATCH] firmware: arm_scmi: Fix NULL pointer dereference in mailbox_chan_free

[Cristian Marussi]

On Tue, Sep 08, 2020 at 12:26:11PM +0100, Sudeep Holla wrote:
> scmi_mailbox is obtained from cinfo->transport_info and the first
> call to mailbox_chan_free frees the channel and sets cinfo->transport_info
> to NULL. Care is taken to check for non NULL smbox->chan but smbox can
> itself be NULL. Fix it by checking for it without which, kernel crashes
> with below NULL pointer dereference and eventually kernel panic.
> 

LGTM.

Reviewed-by: Cristian Marussi <cristian.marussi at arm.com>
Tested-by: Cristian Marussi <cristian.marussi at arm.com>

Cheers

Cristian

>    Unable to handle kernel NULL pointer dereference at
>    		virtual address 0000000000000038
>    Modules linked in: scmi_module(-)
>    Hardware name: ARM LTD ARM Juno Development Platform/ARM Juno
>    		Development Platform, BIOS EDK II Sep  2 2020
>    pstate: 80000005 (Nzcv daif -PAN -UAO BTYPE=--)
>    pc : mailbox_chan_free+0x2c/0x70 [scmi_module]
>    lr : idr_for_each+0x6c/0xf8
>    Call trace:
>     mailbox_chan_free+0x2c/0x70 [scmi_module]
>     idr_for_each+0x6c/0xf8
>     scmi_remove+0xa8/0xf0 [scmi_module]
>     platform_drv_remove+0x34/0x58
>     device_release_driver_internal+0x118/0x1f0
>     driver_detach+0x58/0xe8
>     bus_remove_driver+0x64/0xe0
>     driver_unregister+0x38/0x68
>     platform_driver_unregister+0x1c/0x28
>     scmi_driver_exit+0x38/0x44 [scmi_module]
>    ---[ end trace 17bde19f50436de9 ]---
>    Kernel panic - not syncing: Fatal exception
>    SMP: stopping secondary CPUs
>    Kernel Offset: 0x1d0000 from 0xffff800010000000
>    PHYS_OFFSET: 0x80000000
>    CPU features: 0x0240022,25806004
>    Memory Limit: none
>    ---[ end Kernel panic - not syncing: Fatal exception ]---
> 
> Cc: Cristian Marussi <cristian.marussi at arm.com>
> Cc: Viresh Kumar <viresh.kumar at linaro.org>
> Fixes: 5c8a47a5a91d ("firmware: arm_scmi: Make scmi core independent of the transport type")
> Signed-off-by: Sudeep Holla <sudeep.holla at arm.com>
> ---
>  drivers/firmware/arm_scmi/mailbox.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/firmware/arm_scmi/mailbox.c b/drivers/firmware/arm_scmi/mailbox.c
> index cc2de207fe10..4626404be541 100644
> --- a/drivers/firmware/arm_scmi/mailbox.c
> +++ b/drivers/firmware/arm_scmi/mailbox.c
> @@ -110,7 +110,7 @@ static int mailbox_chan_free(int id, void *p, void *data)
>  	struct scmi_chan_info *cinfo = p;
>  	struct scmi_mailbox *smbox = cinfo->transport_info;
>  
> -	if (!IS_ERR(smbox->chan)) {
> +	if (smbox && !IS_ERR(smbox->chan)) {
>  		mbox_free_channel(smbox->chan);
>  		cinfo->transport_info = NULL;
>  		smbox->chan = NULL;
> -- 
> 2.17.1
>