Re: [PATCH] SCSI: Fix NULL pointer dereference in runtime PM

[Johannes Thumshirn <jthumshirn@suse.de>]

Alan Stern <stern@rowland.harvard.edu> writes:

> The routines in scsi_rpm.c assume that if a runtime-PM callback is
> invoked for a SCSI device, it can only mean that the device's driver 
> has asked the block layer to handle the runtime power management (by
> calling blk_pm_runtime_init(), which among other things sets q->dev).
>
> However, this assumption turns out to be wrong for things like the ses
> driver.  Normally ses devices are not allowed to do runtime PM, but
> userspace can override this setting.  If this happens, the kernel gets
> a NULL pointer dereference when blk_post_runtime_resume() tries to use
> the uninitialized q->dev pointer.
>
> This patch fixes the problem by calling the block layer's runtime-PM
> routines only if the device's driver really does have a runtime-PM
> callback routine.  Since ses doesn't define any such callbacks, the
> crash won't occur.
>
> This fixes Bugzilla #101371.
>
> Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
> Reported-by: Stanisław Pitucha <viraptor@gmail.com>
> Reported-by: Ilan Cohen <ilanco@gmail.com>
> Tested-by: Ilan Cohen <ilanco@gmail.com>
>
> ---
>
>
> [as1784]
>
>
>  drivers/scsi/scsi_pm.c |   22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
>
> Index: usb-4.0/drivers/scsi/scsi_pm.c
> ===================================================================
> --- usb-4.0.orig/drivers/scsi/scsi_pm.c
> +++ usb-4.0/drivers/scsi/scsi_pm.c
> @@ -217,15 +217,15 @@ static int sdev_runtime_suspend(struct d
>  {
>  	const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
>  	struct scsi_device *sdev = to_scsi_device(dev);
> -	int err;
> +	int err = 0;
>  
> -	err = blk_pre_runtime_suspend(sdev->request_queue);
> -	if (err)
> -		return err;
> -	if (pm && pm->runtime_suspend)
> +	if (pm && pm->runtime_suspend) {
> +		err = blk_pre_runtime_suspend(sdev->request_queue);
> +		if (err)
> +			return err;
>  		err = pm->runtime_suspend(dev);
> -	blk_post_runtime_suspend(sdev->request_queue, err);
> -
> +		blk_post_runtime_suspend(sdev->request_queue, err);
> +	}
>  	return err;
>  }
>  
> @@ -248,11 +248,11 @@ static int sdev_runtime_resume(struct de
>  	const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
>  	int err = 0;
>  
> -	blk_pre_runtime_resume(sdev->request_queue);
> -	if (pm && pm->runtime_resume)
> +	if (pm && pm->runtime_resume) {
> +		blk_pre_runtime_resume(sdev->request_queue);
>  		err = pm->runtime_resume(dev);
> -	blk_post_runtime_resume(sdev->request_queue, err);
> -
> +		blk_post_runtime_resume(sdev->request_queue, err);
> +	}
>  	return err;
>  }
>  
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>

-- 
Johannes Thumshirn                                           Storage
jthumshirn@suse.de                                 +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600  D0D0 0393 969D 2D76 0850
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html