Re: [PATCH] scsi: ips: Fix NULL pointer dereference in ips_release()

On 8/25/26 10:25, Yang Zi wrote:
> When ips_insert_device() fails before calling pci_set_drvdata(), the
> PCI device's drvdata is NULL.  The .remove callback ips_remove_device()
> then calls ips_release(NULL), which immediately calls
> scsi_remove_host(NULL).  That dereferences shost->scan_mutex at a fixed
> offset of the NULL pointer, triggering a NULL pointer dereference.
> 
> KASAN report:
> 
>      BUG: KASAN: null-ptr-deref in __mutex_lock_common kernel/locking/mutex.c:625 [inline]
>      BUG: KASAN: null-ptr-deref in __mutex_lock+0x8b/0x1010 kernel/locking/mutex.c:820
>      Read of size 8 at addr 00000000000000d0 by task syz.0.671/5574
> 
>      __mutex_lock+0x8b/0x1010  kernel/locking/mutex.c:820
>      scsi_remove_host+0x24/0x300  drivers/scsi/hosts.c:169
>      ips_release  drivers/scsi/ips.c:654 [inline] [ips]
>      ips_remove_device+0x155/0x2840  drivers/scsi/ips.c:6701 [ips]

You have sent many fixes for similar crashes in other drivers today. How 
did you recreate this? You have this ips HW, right?

> 
> Make ips_release() handle a NULL Scsi_Host by returning early, so the
> remove path is safe when probe never established drvdata.

ips_release() is only called from ips_remove_device(), and this is in 
the driver removal path. This would not be called if the probe fails 
(and pci_set_drvdata() was not called). So there seems to be something 
wrong in ips_insert_device().

> 
> Signed-off-by: Yang Zi <[email protected]>
> ---
>   drivers/scsi/ips.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/scsi/ips.c b/drivers/scsi/ips.c
> index 41ed73966a48..39568a7effb3 100644
> --- a/drivers/scsi/ips.c
> +++ b/drivers/scsi/ips.c
> @@ -649,6 +649,9 @@ static void ips_release(struct Scsi_Host *sh)
>       ips_ha_t *ha;
>       int i;
>   
> +    if (!sh)
> +        return;
> +
>       METHOD_TRACE("ips_release", 1);
>   
>       scsi_remove_host(sh);