| Message ID | 20200427113153.31246-2-johannes.thumshirn@wdc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Introduce Zone Append for writing to zoned block devices | expand |
On 4/27/20 1:31 PM, Johannes Thumshirn wrote: > In case scsi_setup_fs_cmnd() fails we're not freeing the sgtables > allocated by scsi_init_io(), thus we leak the allocated memory. > > So free the sgtables allocated by scsi_init_io() in case > scsi_setup_fs_cmnd() fails. > > Technically scsi_setup_scsi_cmnd() does not suffer from this problem, as > it can only fail if scsi_init_io() fails, so it does not have sgtables > allocated. But to maintain symmetry and as a measure of defensive > programming, free the sgtables on scsi_setup_scsi_cmnd() failure as well. > scsi_mq_free_sgtables() has safeguards against double-freeing of memory so > this is safe to do. > > While we're at it, rename scsi_mq_free_sgtables() to scsi_free_sgtables(). > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=205595 > Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com> > Reviewed-by: Christoph Hellwig <hch@lst.de> > Reviewed-by: Daniel Wagner <dwagner@suse.de> > --- > drivers/scsi/scsi_lib.c | 16 +++++++++++----- > 1 file changed, 11 insertions(+), 5 deletions(-) > Reviewed-by: Hannes Reinecke <hare@suse.de> Cheers, Hannes
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index 0a73230a8f16..f0cb26b3da6a 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -548,7 +548,7 @@ static void scsi_uninit_cmd(struct scsi_cmnd *cmd) } } -static void scsi_mq_free_sgtables(struct scsi_cmnd *cmd) +static void scsi_free_sgtables(struct scsi_cmnd *cmd) { if (cmd->sdb.table.nents) sg_free_table_chained(&cmd->sdb.table, @@ -560,7 +560,7 @@ static void scsi_mq_free_sgtables(struct scsi_cmnd *cmd) static void scsi_mq_uninit_cmd(struct scsi_cmnd *cmd) { - scsi_mq_free_sgtables(cmd); + scsi_free_sgtables(cmd); scsi_uninit_cmd(cmd); } @@ -1086,7 +1086,7 @@ blk_status_t scsi_init_io(struct scsi_cmnd *cmd) return BLK_STS_OK; out_free_sgtables: - scsi_mq_free_sgtables(cmd); + scsi_free_sgtables(cmd); return ret; } EXPORT_SYMBOL(scsi_init_io); @@ -1217,6 +1217,7 @@ static blk_status_t scsi_setup_cmnd(struct scsi_device *sdev, struct request *req) { struct scsi_cmnd *cmd = blk_mq_rq_to_pdu(req); + blk_status_t ret; if (!blk_rq_bytes(req)) cmd->sc_data_direction = DMA_NONE; @@ -1226,9 +1227,14 @@ static blk_status_t scsi_setup_cmnd(struct scsi_device *sdev, cmd->sc_data_direction = DMA_FROM_DEVICE; if (blk_rq_is_scsi(req)) - return scsi_setup_scsi_cmnd(sdev, req); + ret = scsi_setup_scsi_cmnd(sdev, req); else - return scsi_setup_fs_cmnd(sdev, req); + ret = scsi_setup_fs_cmnd(sdev, req); + + if (ret != BLK_STS_OK) + scsi_free_sgtables(cmd); + + return ret; } static blk_status_t