From: Johannes Thumshirn <johannes.thumsh...@wdc.com>

[ Upstream commit 20a66f2bf280277ab5bb22e27445153b4eb0ac88 ]

In case scsi_setup_fs_cmnd() fails we're not freeing the sgtables allocated
by scsi_init_io(), thus we leak the allocated memory.

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
Link: https://lore.kernel.org/r/20200428104605.8143-2-johannes.thumsh...@wdc.com
Reviewed-by: Christoph Hellwig <h...@lst.de>
Reviewed-by: Daniel Wagner <dwag...@suse.de>
Reviewed-by: Hannes Reinecke <h...@suse.de>
Signed-off-by: Johannes Thumshirn <johannes.thumsh...@wdc.com>
Signed-off-by: Martin K. Petersen <martin.peter...@oracle.com>
Signed-off-by: Sasha Levin <sas...@kernel.org>
---
 drivers/scsi/scsi_lib.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 91c007d26c1ef..206c9f53e9e7a 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -551,7 +551,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,
@@ -563,7 +563,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);
        scsi_del_cmd_from_list(cmd);
 }
@@ -1063,7 +1063,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);
@@ -1214,6 +1214,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;
@@ -1223,9 +1224,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
-- 
2.25.1



Reply via email to