@@ -3683,7 +3683,12 @@ static int sd_resume(struct device *dev)staticintsd_resume_runtime(structdevice*dev){structscsi_disk*sdkp=dev_get_drvdata(dev);-structscsi_device*sdp=sdkp->device;+structscsi_device*sdp;++if(!sdkp)/* E.g.: runtime resume at the start of sd_probe() */+return0;++sdp=sdkp->device;if(sdp->ignore_media_change){/* clear the device's sense data */
--
2.18.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
@@ -3683,7 +3683,12 @@ static int sd_resume(struct device *dev)
static int sd_resume_runtime(struct device *dev)
{
struct scsi_disk *sdkp = dev_get_drvdata(dev);
- struct scsi_device *sdp = sdkp->device;
+ struct scsi_device *sdp;
+
+ if (!sdkp) /* E.g.: runtime resume at the start of
sd_probe() */
+ return 0;
+
+ sdp = sdkp->device;
if (sdp->ignore_media_change) {
/* clear the device's sense data */
I'm fine with this, thank you.
Reviewed-by: Martin Kepplinger <redacted>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Stanley Chu <hidden> Date: 2021-10-15 13:33:14
Hi Miles,
On Fri, 2021-10-15 at 15:46 +0800, Miles Chen wrote:
After merging commit ed4246d37f3b ("scsi: sd: REQUEST SENSE for
BLIST_IGN_MEDIA_CHANGE devices in runtime_resume()"), I hit the
following crash on my device.
static int sd_resume_runtime(struct device *dev)
{
struct scsi_disk *sdkp = dev_get_drvdata(dev);
struct scsi_device *sdp = sdkp->device; // sdkp == NULL and
crash
if (sdp->ignore_media_change) {
...
}
I checked sd_resume() and found that sdkp is possbile to be NULL, and
there is a null pointer test in sd_resume() for this case.
To fix this crash, follow sd_resume() to test if sdkp is NULL
before dereferencing it.
@@ -3683,7 +3683,12 @@ static int sd_resume(struct device *dev)staticintsd_resume_runtime(structdevice*dev){structscsi_disk*sdkp=dev_get_drvdata(dev);-structscsi_device*sdp=sdkp->device;+structscsi_device*sdp;++if(!sdkp)/* E.g.: runtime resume at the start of sd_probe() */+return0;++sdp=sdkp->device;if(sdp->ignore_media_change){/* clear the device's sense data */
Fixing this crash by adding a check inside sd_resume_runtime() seems
wrong to me. sd_probe() namely calls dev_set_drvdata(dev, sdkp) before
sd_probe() has finished so even with the above patch applied sd_resume()
can be called before sd_probe() has finished.
With which kernel version has this crash been encountered? The
scsi_autopm_get_device() / scsi_autopm_put_device() pair added by commit
6fe8c1dbefd6 ("scsi: balance out autopm get/put calls in
scsi_sysfs_add_sdev()"; kernel v3.18) should be sufficient to prevent
the reported crash.
Thanks,
Bart.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Fixing this crash by adding a check inside sd_resume_runtime() seems wrong to me. sd_probe() namely calls dev_set_drvdata(dev, sdkp) before
sd_probe() has finished so even with the above patch applied sd_resume() can be called before sd_probe() has finished.
With which kernel version has this crash been encountered? The
scsi_autopm_get_device() / scsi_autopm_put_device() pair added by commit
6fe8c1dbefd6 ("scsi: balance out autopm get/put calls in scsi_sysfs_add_sdev()"; kernel v3.18) should be sufficient to prevent the reported crash.
Thanks,
Thanks for your comment.
I hit this in v5.15-rc1 merge, I can still reproduce this with v5.15-rc5.
I found two ways to avoid the crash:
1) revert commit ed4246d37f3b ("scsi: sd: REQUEST SENSE for
BLIST_IGN_MEDIA_CHANGE devices in runtime_resume()") works for me.
2) adding the NULL point check in this patch.
From the backtrace, dev_set_drvdata() is called after sd_resume_runtime()
is called.
sd_probe()
{
scsi_autopm_get_device()
pm_runtime_get_sync()
__pm_runtime_resume()
rpm_resume()
...
sd_resume_runtime() // crash here
dev_set_drvdata(dev, sdkp); // sdkp is set later
}
[ 4.861395][ T151] sd_resume_runtime+0x20/0x14c
[ 4.862025][ T151] scsi_runtime_resume+0x84/0xe4
[ 4.862667][ T151] __rpm_callback+0x1f4/0x8cc
[ 4.863275][ T151] rpm_resume+0x7e8/0xaa4
[ 4.863836][ T151] __pm_runtime_resume+0xa0/0x110
[ 4.864489][ T151] sd_probe+0x30/0x428
[ 4.865016][ T151] really_probe+0x14c/0x500
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Bart Van Assche <bvanassche@acm.org> Date: 2021-10-15 21:14:42
On 10/15/21 13:11, miles.chen@mediatek.com wrote:
I hit this in v5.15-rc1 merge, I can still reproduce this with v5.15-rc5.
I found two ways to avoid the crash:
1) revert commit ed4246d37f3b ("scsi: sd: REQUEST SENSE for
BLIST_IGN_MEDIA_CHANGE devices in runtime_resume()") works for me.
2) adding the NULL point check in this patch.
quoted
From the backtrace, dev_set_drvdata() is called after sd_resume_runtime()
is called.
sd_probe()
{
scsi_autopm_get_device()
pm_runtime_get_sync()
__pm_runtime_resume()
rpm_resume()
...
sd_resume_runtime() // crash here
dev_set_drvdata(dev, sdkp); // sdkp is set later
}
[ 4.861395][ T151] sd_resume_runtime+0x20/0x14c
[ 4.862025][ T151] scsi_runtime_resume+0x84/0xe4
[ 4.862667][ T151] __rpm_callback+0x1f4/0x8cc
[ 4.863275][ T151] rpm_resume+0x7e8/0xaa4
[ 4.863836][ T151] __pm_runtime_resume+0xa0/0x110
[ 4.864489][ T151] sd_probe+0x30/0x428
[ 4.865016][ T151] really_probe+0x14c/0x500
Thanks for the clarification. Given this clarification I'm fine with
your patch.
Bart.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: "Martin K. Petersen" <martin.petersen@oracle.com> Date: 2021-10-19 03:44:08
On Fri, 15 Oct 2021 15:46:54 +0800, Miles Chen wrote:
After merging commit ed4246d37f3b ("scsi: sd: REQUEST SENSE for
BLIST_IGN_MEDIA_CHANGE devices in runtime_resume()"), I hit the
following crash on my device.
static int sd_resume_runtime(struct device *dev)
{
struct scsi_disk *sdkp = dev_get_drvdata(dev);
struct scsi_device *sdp = sdkp->device; // sdkp == NULL and crash
[...]