| Message ID | 1572922150-4358-1-git-send-email-schmitzmic@gmail.com (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | 9393c8de628cf0968d81a17cc11841e42191e041 |
| Headers | show |
| Series | scsi: scsi-lib.c: increase cmd_size by sizeof(struct scatterlist) | expand |
On Tue, 5 Nov 2019, Michael Schmitz wrote: > 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. > > 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. > > Signed-off-by: Michael Schmitz <schmitzmic@gmail.com> > CC: Finn Thain <fthain@telegraphics.com.au> No objections from me. I still like the patch series I sent but it's a matter of taste. I'm happy to leave it to the scsi maintainers to figure out the best fix for this regression.
Michael, > 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. Applied to 5.4/scsi-fixes. I picked your version since it is small. I still intend to queue Finn's SG_NONE cleanups for 5.5. In a way these are orthogonal. Thanks!
Thanks Martin, Am 06.11.2019 um 18:46 schrieb Martin K. Petersen: > > Michael, > >> 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. > > Applied to 5.4/scsi-fixes. I picked your version since it is small. I realize it's more of a band-aid and doesn't address the real issue perhaps. Maybe it gives someone with a better clue about the block layer some idea though (I got out of my depth rather quickly). > I still intend to queue Finn's SG_NONE cleanups for 5.5. In a way these > are orthogonal. Either zero or one work for sg_tablesize in my case, so that's perfectly fine by me. Cheers, Michael > > Thanks! >
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index d47d637..d38b0e1 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -1867,7 +1867,8 @@ int scsi_mq_setup_tags(struct Scsi_Host *shost) { unsigned int cmd_size, sgl_size; - sgl_size = scsi_mq_inline_sgl_size(shost); + sgl_size = max_t(unsigned int, sizeof(struct scatterlist), + scsi_mq_inline_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) +
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. 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. Signed-off-by: Michael Schmitz <schmitzmic@gmail.com> CC: Finn Thain <fthain@telegraphics.com.au> --- drivers/scsi/scsi_lib.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)