Please consider mainline commit 9393c8de628c for stable

Please consider mainline commit 9393c8de628c for stable

[Michael Schmitz]

Dear stable kernel maintainers,

please consider including commit 9393c8de628c ("scsi: core: Handle 
drivers which set sg_tablesize to zero") for inclusion in stable.

The commit fixes a long standing bug that affects all SCSI low-level 
drivers setting sg_tablesize to zero, introduced in commit d285203c 
("scsi: add support for a blk-mq based I/O path.") around kernel version 
3.16.

Use of the option use_blk_mq=y in kernel versions prior to 5.1, and any 
use of such drivers in later kernels, will result in a null pointer 
dereference from the block layer.

I hadn't expected Martin Petersen to pick my fix over another one 
submitted by Finn Thain, so I never added CC: or Fixes: tags.

Cheers,

	Michael

Re: Please consider mainline commit 9393c8de628c for stable

[Sasha Levin]

On Tue, Nov 12, 2019 at 03:52:47PM +1300, Michael Schmitz wrote:
>Dear stable kernel maintainers,
>
>please consider including commit 9393c8de628c ("scsi: core: Handle 
>drivers which set sg_tablesize to zero") for inclusion in stable.
>
>The commit fixes a long standing bug that affects all SCSI low-level 
>drivers setting sg_tablesize to zero, introduced in commit d285203c 
>("scsi: add support for a blk-mq based I/O path.") around kernel 
>version 3.16.
>
>Use of the option use_blk_mq=y in kernel versions prior to 5.1, and 
>any use of such drivers in later kernels, will result in a null 
>pointer dereference from the block layer.
>
>I hadn't expected Martin Petersen to pick my fix over another one 
>submitted by Finn Thain, so I never added CC: or Fixes: tags.

Could you provide a backport for 4.19 and older?

We would need to work around not having 3dccdf53c2f3 ("scsi: core: avoid
preallocating big SGL for data") in older kernels, and I'm not confident
about what I ended up as a backport without ability to test it.

-- 
Thanks,
Sasha

Re: Please consider mainline commit 9393c8de628c for stable

[Michael Schmitz]

Hi Sasha,

Am 13.11.2019 um 14:27 schrieb Sasha Levin:
> On Tue, Nov 12, 2019 at 03:52:47PM +1300, Michael Schmitz wrote:
>> Dear stable kernel maintainers,
>>
>> please consider including commit 9393c8de628c ("scsi: core: Handle
>> drivers which set sg_tablesize to zero") for inclusion in stable.
>>
>> The commit fixes a long standing bug that affects all SCSI low-level
>> drivers setting sg_tablesize to zero, introduced in commit d285203c
>> ("scsi: add support for a blk-mq based I/O path.") around kernel
>> version 3.16.
>>
>> Use of the option use_blk_mq=y in kernel versions prior to 5.1, and
>> any use of such drivers in later kernels, will result in a null
>> pointer dereference from the block layer.
>>
>> I hadn't expected Martin Petersen to pick my fix over another one
>> submitted by Finn Thain, so I never added CC: or Fixes: tags.
>
> Could you provide a backport for 4.19 and older?
>
> We would need to work around not having 3dccdf53c2f3 ("scsi: core: avoid
> preallocating big SGL for data") in older kernels, and I'm not confident
> about what I ended up as a backport without ability to test it.

I hadn't spotted that. From  what I've seen, that commit renamed 
scsi_mq_inline_sgl_size() by scsi_mq_sgl_size() (among all the other 
changes) so modifying my patch with that in mind should suffice.

For safety, I'll test that version though.

Cheers,

	Michael

[PATCH] scsi: core: Handle drivers which set sg_tablesize to zero

[Michael Schmitz]

commit 9393c8de628c upstream

In scsi_mq_setup_tags(), cmd_size is calculated based on zero size
for the scatter-gather list in case the low level driver uses SG_NONE
in its host template.d, and an empty message aborts the commit.

cmd_size is passed on to the block layer for calculation of the request
size, and we've seen NULL pointer dereference errors from the block
layer in drivers where SG_NONE is used and a mq IO scheduler is active,
apparently as a consequence of this (see commit 68ab2d76e4be for the
cxflash driver, and a recent patch by Finn Thain converting the three
m68k NFR5380 drivers to avoid setting SG_NONE).

Try to avoid these errors by accounting for at least one sg list entry
when caculating cmd_size, regardless of whether the low level driver
set a zero sg_tablesize.

Tested on 030 m68k with the atari_scsi driver - setting sg_tablesize to
SG_NONE no longer results in a crash when loading this driver.

Backport of commit 9393c8de628c to stable kernels before 4.19 which lack
commit 3dccdf53c2f3 ("scsi: core: avoid preallocating big SGL for data"),
as requestef by Sasha Levin.

Signed-off-by: Michael Schmitz <schmitzmic@gmail.com>
Cc: Finn Thain <fthain@telegraphics.com.au>
Cc: Sasha Levin <sashal@kernel.org>
Cc: Martin K. Petersen <martin.petersen@oracle.com>
---
 drivers/scsi/scsi_lib.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 0adfb3b..e0a4ad9 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2356,7 +2356,8 @@ int scsi_mq_setup_tags(struct Scsi_Host *shost)
 {
 	unsigned int cmd_size, sgl_size;
 
-	sgl_size = scsi_mq_sgl_size(shost);
+	sgl_size = max_t(unsigned int, sizeof(struct scatterlist),
+			scsi_mq_sgl_size(shost));
 	cmd_size = sizeof(struct scsi_cmnd) + shost->hostt->cmd_size + sgl_size;
 	if (scsi_host_get_prot(shost))
 		cmd_size += sizeof(struct scsi_data_buffer) + sgl_size;
-- 
1.7.0.4

Re: [PATCH] scsi: core: Handle drivers which set sg_tablesize to zero

[Greg KH]

On Wed, Nov 13, 2019 at 07:39:41PM +1300, Michael Schmitz wrote:
> commit 9393c8de628c upstream

<snip>

Thanks for this, now queued up.

greg k-h

Re: [PATCH] scsi: core: Handle drivers which set sg_tablesize to zero

[Michael Schmitz]

Thanks Greg!

On further thought - the bug will only be triggered prior to 5.x if 
use_blk_mq=y is set for the SCSI midlevel, Quite unlikely actually.

Cheers,

	Michael

Am 14.11.2019 um 19:00 schrieb Greg KH:
> On Wed, Nov 13, 2019 at 07:39:41PM +1300, Michael Schmitz wrote:
>> commit 9393c8de628c upstream
>
> <snip>
>
> Thanks for this, now queued up.
>
> greg k-h
>