| Message ID | 20190720030637.14447-1-ming.lei@redhat.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | block/scsi/dm-rq: fix leak of request private data in dm-mpath | expand |
Hey Ming Lei, On Sat, Jul 20, 2019 at 11:06:35AM +0800, Ming Lei wrote: > Hi, > > When one request is dispatched to LLD via dm-rq, if the result is > BLK_STS_*RESOURCE, dm-rq will free the request. However, LLD may allocate > private data for this request, so this way will cause memory leak. I am confused about this. Probably because I am not up-to-date with all of blk-mq. But if you free the LLD private data before the request is finished, what is the LLD doing if the request finishes afterwards? Would that not be an automatic use-after-free? > > Add .cleanup_rq() callback and implement it in SCSI for fixing the issue, > since SCSI is the only driver which allocates private requst data in > .queue_rq() path. > > Another use case of this callback is to free the request and re-submit > bios during cpu hotplug when the hctx is dead, see the following link: > > https://lore.kernel.org/linux-block/f122e8f2-5ede-2d83-9ca0-bc713ce66d01@huawei.com/T/#t > > V2: > - run .cleanup_rq() in blk_mq_free_request(), as suggested by Mike
On Fri, Jul 26, 2019 at 06:20:46PM +0200, Benjamin Block wrote: > Hey Ming Lei, > > On Sat, Jul 20, 2019 at 11:06:35AM +0800, Ming Lei wrote: > > Hi, > > > > When one request is dispatched to LLD via dm-rq, if the result is > > BLK_STS_*RESOURCE, dm-rq will free the request. However, LLD may allocate > > private data for this request, so this way will cause memory leak. > > I am confused about this. Probably because I am not up-to-date with > all of blk-mq. But if you free the LLD private data before the request > is finished, what is the LLD doing if the request finishes afterwards? > Would that not be an automatic use-after-free? Wrt. this special use case, the underlying request is totally covered by dm-rq after .queue_rq() returns BLK_STS_*RESOURCE. So the request won't be re-dispatched by blk-mq at all. thanks, Ming
Hi, When one request is dispatched to LLD via dm-rq, if the result is BLK_STS_*RESOURCE, dm-rq will free the request. However, LLD may allocate private data for this request, so this way will cause memory leak. Add .cleanup_rq() callback and implement it in SCSI for fixing the issue, since SCSI is the only driver which allocates private requst data in .queue_rq() path. Another use case of this callback is to free the request and re-submit bios during cpu hotplug when the hctx is dead, see the following link: https://lore.kernel.org/linux-block/f122e8f2-5ede-2d83-9ca0-bc713ce66d01@huawei.com/T/#t V2: - run .cleanup_rq() in blk_mq_free_request(), as suggested by Mike Ming Lei (2): blk-mq: add callback of .cleanup_rq scsi: implement .cleanup_rq callback block/blk-mq.c | 3 +++ drivers/scsi/scsi_lib.c | 28 ++++++++++++++++++++-------- include/linux/blk-mq.h | 7 +++++++ 3 files changed, 30 insertions(+), 8 deletions(-) Cc: Ewan D. Milne <emilne@redhat.com> Cc: Bart Van Assche <bvanassche@acm.org> Cc: Hannes Reinecke <hare@suse.com> Cc: Christoph Hellwig <hch@lst.de> Cc: Mike Snitzer <snitzer@redhat.com> Cc: dm-devel@redhat.com Cc: <stable@vger.kernel.org> Fixes: 396eaf21ee17 ("blk-mq: improve DM's blk-mq IO merging via blk_insert_cloned_request feedback")