scsi: fix memory leak with scsi-mq
PATCH 1/1 scsi: fix memory leak with scsi-mq
Fix a memory leak with scsi-mq triggered by commands with large data transfer length.
__sg_alloc_table() sets both table->nents and table->orig_nents to the same value. When the scatterlist is DMA-mapped, table->nents is overwritten with the (possibly smaller) size of the DMA-mapped scatterlist, while table->orig_nents retains the original size of the allocated scatterlist. scsi_free_sgtable() should therefore check orig_nents instead of nents, and all code that initializes sdb->table without calling __sg_alloc_table() should set both nents and orig_nents.
---
For immediate inclusion.
drivers/scsi/scsi_error.c +1 -1
@@ -944,7 +944,7 @@ void scsi_eh_prep_cmnd(struct scsi_cmnd *scmd, struct scsi_eh_save *ses, scmd->sdb.length); scmd->sdb.table.sgl = &ses->sense_sgl; scmd->sc_data_direction =DMA_FROM_DEVICE ; - scmd->sdb.table.nents =1 ; + scmd->sdb.table.nents = scmd->sdb.table.orig_nents =1 ; scmd->cmnd[0 ] =REQUEST_SENSE ; scmd->cmnd[4 ] = scmd->sdb.length; scmd->cmd_len =COMMAND_SIZE (scmd->cmnd[0 ]);
drivers/scsi/scsi_lib.c +3 -3
@@ -583,7 +583,7 @@ static struct scatterlist *scsi_sg_alloc(unsigned int nents, gfp_t gfp_mask)static void scsi_free_sgtable (struct scsi_data_buffer *sdb,bool mq) { -if (mq && sdb->table.nents <=SCSI_MAX_SG_SEGMENTS ) +if (mq && sdb->table.orig_nents <=SCSI_MAX_SG_SEGMENTS )return ;__sg_free_table (&sdb->table,SCSI_MAX_SG_SEGMENTS , mq, scsi_sg_free); }
@@ -597,8 +597,8 @@ static int scsi_alloc_sgtable(struct scsi_data_buffer *sdb, int nents, bool mq)if (mq) {if (nents <=SCSI_MAX_SG_SEGMENTS ) { - sdb->table.nents = nents; -sg_init_table (sdb->table.sgl, sdb->table.nents); + sdb->table.nents = sdb->table.orig_nents = nents; +sg_init_table (sdb->table.sgl, nents);return 0 ; } first_chunk = sdb->table.sgl;
Thanks Tony,
this looks good to me. In the long run I'll see how we can hide these implementation details in the lib/scatterlist.c code instead of burdening it on the users.
Quoted 57 lines · Tony Battersby
Fix a memory leak with scsi-mq triggered by commands with large data transfer length.
__sg_alloc_table()sets bothtable->nentsandtable->orig_nentsto the same value. When the scatterlist is DMA-mapped,table->nentsis overwritten with the (possibly smaller) size of the DMA-mapped scatterlist, whiletable->orig_nentsretains the original size of the allocated scatterlist.scsi_free_sgtable()should therefore checkorig_nentsinstead of nents, and all code that initializessdb->tablewithout calling__sg_alloc_table()should set both nents andorig_nents.Fixes d285203cf647 ("scsi: add support for a blk-mq based I/O path.")Cc <[email protected]> # 3.17+Signed-off-by Tony Battersby <[email protected]>---
For immediate inclusion.
drivers/scsi/scsi_error.c +1
-1@@ -944,7 +944,7 @@ void scsi_eh_prep_cmnd(struct scsi_cmnd *scmd, struct scsi_eh_save *ses, scmd->sdb.length); scmd->sdb.table.sgl = &ses->sense_sgl; scmd->sc_data_direction =DMA_FROM_DEVICE ; - scmd->sdb.table.nents =1 ; + scmd->sdb.table.nents = scmd->sdb.table.orig_nents =1 ; scmd->cmnd[0 ] =REQUEST_SENSE ; scmd->cmnd[4 ] = scmd->sdb.length; scmd->cmd_len =COMMAND_SIZE (scmd->cmnd[0 ]);drivers/scsi/scsi_lib.c +3
-4@@ -583,7 +583,7 @@ static struct scatterlist *scsi_sg_alloc(unsigned int nents, gfp_t gfp_mask)static void scsi_free_sgtable (struct scsi_data_buffer *sdb,bool mq) { -if (mq && sdb->table.nents <=SCSI_MAX_SG_SEGMENTS ) +if (mq && sdb->table.orig_nents <=SCSI_MAX_SG_SEGMENTS )return ;__sg_free_table (&sdb->table,SCSI_MAX_SG_SEGMENTS , mq, scsi_sg_free); }@@ -597,8 +597,8 @@ static int scsi_alloc_sgtable(struct scsi_data_buffer *sdb, int nents, bool mq)if (mq) {if (nents <=SCSI_MAX_SG_SEGMENTS ) { - sdb->table.nents = nents; -sg_init_table (sdb->table.sgl, sdb->table.nents); + sdb->table.nents = sdb->table.orig_nents = nents; +sg_init_table (sdb->table.sgl, nents);return 0 ; } first_chunk = sdb->table.sgl; --
Quoted 3 lines
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Looks good. James, can we get this in please?