scsi: Fix sg memory leak building 2 scatter-gather lists
Subsystems (2)
PATCH 1/1 scsi: Fix sg memory leak building 2 scatter-gather lists
Hello,
There is a memory leak in sg that is hit in the case when direct_io is in
use and the length of the data is longer than we can fit into a single scatter-gather list. Two scatter-gather lists are allocated for each io that meets the criteria, and only 1 ever gets released.
The following fix releases the scatter-gather list created by the
attempted sg_build_direct when abandoning the effort in favor of sg_build_indirect. sg_build_indirect creates a new scatter-gather list.
drivers/scsi/sg.c +1 -0
@@ -1648,6 +1648,7 @@ res =sg_build_direct (srp, sfp, dxfer_len);if (res <=0 )/* -ve -> error, 0 -> done, 1 -> try indirect */ return res; +sg_remove_scat (req_schp); }if ((!sg_res_in_use (sfp)) && (dxfer_len <= rsv_schp->bufflen))sg_link_reserve (sfp, srp, dxfer_len);
Signature · 2 lines
-- Bryan Holty
Quoted 9 lines · Bryan Holty
Hello,
There is a memory leak in sg that is hit in the case whendirect_iois in
use and the length of the data is longer than we can fit into a single scatter-gather list. Two scatter-gather lists are allocated for each io that meets the criteria, and only 1 ever gets released.The following fix releases the scatter-gather list created by the
attemptedsg_build_directwhen abandoning the effort in favor ofsg_build_indirect.sg_build_indirectcreates a new scatter-gather list.
Wow, sg leak fixes are coming thick and fast. I agree there is a leak there but I'm not sure about the placement of sg_remove_scat().
Bryan, what do you think about the variant attached to this post? It is untested.
Doug Gilbert
drivers/scsi/sg.c line 1654
Quoted 12 lines
Signed-off-by Bryan Holty <[email protected]>drivers/scsi/sg.c +1
-0@@ -1648,6 +1648,7 @@ res =sg_build_direct (srp, sfp, dxfer_len);if (res <=0 )/* -ve -> error, 0 -> done, 1 -> try indirect */ return res; +sg_remove_scat (req_schp); }if ((!sg_res_in_use (sfp)) && (dxfer_len <= rsv_schp->bufflen))sg_link_reserve (sfp, srp, dxfer_len);
drivers/scsi/sg.c2616dio_lk renamed from drivers/scsi/sg.c +3 -1
@@ -1834,8 +1834,10 @@ res = st_map_user_pages(schp->buffer, mx_sc_elems, (unsigned long)hp->dxferp, dxfer_len, (SG_DXFER_TO_DEV == hp->dxfer_direction) ? 1 : 0); - if (res <= 0) + if (res <= 0) { + sg_remove_scat(schp); return 1; + } schp->k_use_sg = res; schp->dio_in_use = 1; hp->info |= SG_INFO_DIRECT_IO;
Quoted 22 lines · Douglas Gilbert
Quoted 9 lines · Bryan Holty
Hello,
There is a memory leak in sg that is hit in the case whendirect_io
is in use and the length of the data is longer than we can fit into a single scatter-gather list. Two scatter-gather lists are allocated for each io that meets the criteria, and only 1 ever gets released.The following fix releases the scatter-gather list created by the
attemptedsg_build_directwhen abandoning the effort in favor ofsg_build_indirect.sg_build_indirectcreates a new scatter-gather list.Wow, sg leak fixes are coming thick and fast. I agree there is a leak there but I'm not sure about the placement of
sg_remove_scat().Bryan, what do you think about the variant attached to this post? It is untested.
Doug Gilbert
Quoted 2 lines
Signed-off-by Bryan Holty <[email protected]>
Quoted 10 lines
drivers/scsi/sg.c line 1654
Quoted 10 lines
drivers/scsi/sg.c +1
-0@@ -1648,6 +1648,7 @@ res =sg_build_direct (srp, sfp, dxfer_len);if (res <=0 )/* -ve -> error, 0 -> done, 1 -> try indirect */ return res; +sg_remove_scat (req_schp); }if ((!sg_res_in_use (sfp)) && (dxfer_len <= rsv_schp->bufflen))sg_link_reserve (sfp, srp, dxfer_len);
I think the variant solves the same problem. I had no other reason for releasing the scatter-gather list outside of sg_build_direct other than that is how the the sg_build_indirect is done and that due to the checks in sg_remove_scat it could be called from there.
I agree that the variant placement makes more sense. --
Bryan Holty