* [PATCH] scsi: ufs: ufshpb: Fix possible memory leak [not found] <CGME20210820014624epcms2p6724e146ca1f93ba6eac5e7cf95d4cfd2@epcms2p6> @ 2021-08-20 1:46 ` Keoseong Park 2021-08-20 20:28 ` Bart Van Assche 0 siblings, 1 reply; 3+ messages in thread From: Keoseong Park @ 2021-08-20 1:46 UTC (permalink / raw) To: ALIM AKHTAR, avri.altman@wdc.com, jejb@linux.ibm.com, martin.petersen@oracle.com, Daejun Park, beanhuo@micron.com, gregkh@linuxfoundation.org, linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org When HPB pinned region exists and mctx allocation for this region fails, memory leak is possible because memory is not released for the subregion table of the current region. So, change to free memory for the subregion table of the current region. Signed-off-by: Keoseong Park <keosung.park@samsung.com> --- drivers/scsi/ufs/ufshpb.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/ufs/ufshpb.c b/drivers/scsi/ufs/ufshpb.c index 9acce92a356b..052f584c789a 100644 --- a/drivers/scsi/ufs/ufshpb.c +++ b/drivers/scsi/ufs/ufshpb.c @@ -1933,7 +1933,7 @@ static int ufshpb_alloc_region_tbl(struct ufs_hba *hba, struct ufshpb_lu *hpb) if (ufshpb_is_pinned_region(hpb, rgn_idx)) { ret = ufshpb_init_pinned_active_region(hba, hpb, rgn); if (ret) - goto release_srgn_table; + goto release_current_srgn_table; } else { rgn->rgn_state = HPB_RGN_INACTIVE; } @@ -1944,6 +1944,9 @@ static int ufshpb_alloc_region_tbl(struct ufs_hba *hba, struct ufshpb_lu *hpb) return 0; +release_current_srgn_table: + kvfree(rgn_table[rgn_idx].srgn_tbl); + release_srgn_table: for (i = 0; i < rgn_idx; i++) kvfree(rgn_table[i].srgn_tbl); -- 2.17.1 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] scsi: ufs: ufshpb: Fix possible memory leak 2021-08-20 1:46 ` [PATCH] scsi: ufs: ufshpb: Fix possible memory leak Keoseong Park @ 2021-08-20 20:28 ` Bart Van Assche 2021-08-23 1:09 ` Keoseong Park 0 siblings, 1 reply; 3+ messages in thread From: Bart Van Assche @ 2021-08-20 20:28 UTC (permalink / raw) To: keosung.park, ALIM AKHTAR, avri.altman@wdc.com, jejb@linux.ibm.com, martin.petersen@oracle.com, Daejun Park, beanhuo@micron.com, gregkh@linuxfoundation.org, linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org On 8/19/21 6:46 PM, Keoseong Park wrote: > When HPB pinned region exists and mctx allocation for this region fails, > memory leak is possible because memory is not released for the subregion > table of the current region. > > So, change to free memory for the subregion table of the current region. > > Signed-off-by: Keoseong Park <keosung.park@samsung.com> > --- > drivers/scsi/ufs/ufshpb.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/scsi/ufs/ufshpb.c b/drivers/scsi/ufs/ufshpb.c > index 9acce92a356b..052f584c789a 100644 > --- a/drivers/scsi/ufs/ufshpb.c > +++ b/drivers/scsi/ufs/ufshpb.c > @@ -1933,7 +1933,7 @@ static int ufshpb_alloc_region_tbl(struct ufs_hba *hba, struct ufshpb_lu *hpb) > if (ufshpb_is_pinned_region(hpb, rgn_idx)) { > ret = ufshpb_init_pinned_active_region(hba, hpb, rgn); > if (ret) > - goto release_srgn_table; > + goto release_current_srgn_table; > } else { > rgn->rgn_state = HPB_RGN_INACTIVE; > } > @@ -1944,6 +1944,9 @@ static int ufshpb_alloc_region_tbl(struct ufs_hba *hba, struct ufshpb_lu *hpb) > > return 0; > > +release_current_srgn_table: > + kvfree(rgn_table[rgn_idx].srgn_tbl); > + > release_srgn_table: > for (i = 0; i < rgn_idx; i++) > kvfree(rgn_table[i].srgn_tbl); 'rgn_table' is allocated with kvcalloc() so please merge the new kvfree() statement with the for-loop below it. There is another improvement that can be made in this function: hpb->rgn_tbl is not cleared in the error path. I propose to move the "hpb->rgn_tbl = rgn_table" assignment from the start of the function to just above the "return 0" statement. Thanks, Bart. ^ permalink raw reply [flat|nested] 3+ messages in thread
* RE: Re: [PATCH] scsi: ufs: ufshpb: Fix possible memory leak 2021-08-20 20:28 ` Bart Van Assche @ 2021-08-23 1:09 ` Keoseong Park 0 siblings, 0 replies; 3+ messages in thread From: Keoseong Park @ 2021-08-23 1:09 UTC (permalink / raw) To: Bart Van Assche, Keoseong Park, ALIM AKHTAR, avri.altman@wdc.com, jejb@linux.ibm.com, martin.petersen@oracle.com, Daejun Park, beanhuo@micron.com, gregkh@linuxfoundation.org, linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org Hi Bart, > On 8/19/21 6:46 PM, Keoseong Park wrote: >> When HPB pinned region exists and mctx allocation for this region fails, >> memory leak is possible because memory is not released for the subregion >> table of the current region. >> >> So, change to free memory for the subregion table of the current region. >> >> Signed-off-by: Keoseong Park <keosung.park@samsung.com> >> --- >> drivers/scsi/ufs/ufshpb.c | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/scsi/ufs/ufshpb.c b/drivers/scsi/ufs/ufshpb.c >> index 9acce92a356b..052f584c789a 100644 >> --- a/drivers/scsi/ufs/ufshpb.c >> +++ b/drivers/scsi/ufs/ufshpb.c >> @@ -1933,7 +1933,7 @@ static int ufshpb_alloc_region_tbl(struct ufs_hba *hba, struct ufshpb_lu *hpb) >> if (ufshpb_is_pinned_region(hpb, rgn_idx)) { >> ret = ufshpb_init_pinned_active_region(hba, hpb, rgn); >> if (ret) >> - goto release_srgn_table; >> + goto release_current_srgn_table; >> } else { >> rgn->rgn_state = HPB_RGN_INACTIVE; >> } >> @@ -1944,6 +1944,9 @@ static int ufshpb_alloc_region_tbl(struct ufs_hba *hba, struct ufshpb_lu *hpb) >> >> return 0; >> >> +release_current_srgn_table: >> + kvfree(rgn_table[rgn_idx].srgn_tbl); >> + >> release_srgn_table: >> for (i = 0; i < rgn_idx; i++) >> kvfree(rgn_table[i].srgn_tbl); > >'rgn_table' is allocated with kvcalloc() so please merge the new kvfree() statement >with the for-loop below it. > >There is another improvement that can be made in this function: hpb->rgn_tbl >is not cleared in the error path. I propose to move the "hpb->rgn_tbl = rgn_table" >assignment from the start of the function to just above the "return 0" statement. > >Thanks, > >Bart. > Thank you for your feedback. I will fix it. Best Regards, Keoseong ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-08-23 1:38 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <CGME20210820014624epcms2p6724e146ca1f93ba6eac5e7cf95d4cfd2@epcms2p6>
2021-08-20 1:46 ` [PATCH] scsi: ufs: ufshpb: Fix possible memory leak Keoseong Park
2021-08-20 20:28 ` Bart Van Assche
2021-08-23 1:09 ` Keoseong Park
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox