* [PATCH 1/2] libfc: fix memory leakage in local port
@ 2010-10-27 12:40 Hillf Danton
2010-10-27 12:43 ` [PATCH 2/2] " Hillf Danton
2010-10-28 23:12 ` [Open-FCoE] [PATCH 1/2] " Robert Love
0 siblings, 2 replies; 6+ messages in thread
From: Hillf Danton @ 2010-10-27 12:40 UTC (permalink / raw)
To: devel; +Cc: linux-scsi
There seems info should get freed when error encountered.
Signed-off-by: Hillf Danton <dhillf@gmail.com>
---
--- a/drivers/scsi/libfc/fc_lport.c 2010-09-13 07:07:38.000000000 +0800
+++ b/drivers/scsi/libfc/fc_lport.c 2010-10-27 20:33:36.000000000 +0800
@@ -1766,8 +1766,10 @@ static int fc_lport_ct_request(struct fc
info->sg = job->reply_payload.sg_list;
if (!lport->tt.exch_seq_send(lport, fp, fc_lport_bsg_resp,
- NULL, info, tov))
+ NULL, info, tov)) {
+ kfree(info);
return -ECOMM;
+ }
return 0;
}
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 2/2] libfc: fix memory leakage in local port
2010-10-27 12:40 [PATCH 1/2] libfc: fix memory leakage in local port Hillf Danton
@ 2010-10-27 12:43 ` Hillf Danton
2010-10-28 23:13 ` [Open-FCoE] " Robert Love
2010-10-28 23:12 ` [Open-FCoE] [PATCH 1/2] " Robert Love
1 sibling, 1 reply; 6+ messages in thread
From: Hillf Danton @ 2010-10-27 12:43 UTC (permalink / raw)
To: devel; +Cc: linux-scsi
There seems info should get freed when error encountered.
Signed-off-by: Hillf Danton <dhillf@gmail.com>
---
--- a/drivers/scsi/libfc/fc_lport.c 2010-09-13 07:07:38.000000000 +0800
+++ b/drivers/scsi/libfc/fc_lport.c 2010-10-27 20:35:10.000000000 +0800
@@ -1707,8 +1707,10 @@ static int fc_lport_els_request(struct f
info->sg = job->reply_payload.sg_list;
if (!lport->tt.exch_seq_send(lport, fp, fc_lport_bsg_resp,
- NULL, info, tov))
+ NULL, info, tov)) {
+ kfree(info);
return -ECOMM;
+ }
return 0;
}
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Open-FCoE] [PATCH 2/2] libfc: fix memory leakage in local port
2010-10-27 12:43 ` [PATCH 2/2] " Hillf Danton
@ 2010-10-28 23:13 ` Robert Love
0 siblings, 0 replies; 6+ messages in thread
From: Robert Love @ 2010-10-28 23:13 UTC (permalink / raw)
To: Hillf Danton; +Cc: devel, linux-scsi
On Wed, 2010-10-27 at 20:43 +0800, Hillf Danton wrote:
> There seems info should get freed when error encountered.
>
> Signed-off-by: Hillf Danton <dhillf@gmail.com>
> ---
>
> --- a/drivers/scsi/libfc/fc_lport.c 2010-09-13 07:07:38.000000000 +0800
> +++ b/drivers/scsi/libfc/fc_lport.c 2010-10-27 20:35:10.000000000 +0800
> @@ -1707,8 +1707,10 @@ static int fc_lport_els_request(struct f
> info->sg = job->reply_payload.sg_list;
>
> if (!lport->tt.exch_seq_send(lport, fp, fc_lport_bsg_resp,
> - NULL, info, tov))
> + NULL, info, tov)) {
> + kfree(info);
The same comment I made about patch [1/2] applies here. We need the BSG
info in the fc_lport_bsg_resp response handler.
Thanks, //Rob
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Open-FCoE] [PATCH 1/2] libfc: fix memory leakage in local port
2010-10-27 12:40 [PATCH 1/2] libfc: fix memory leakage in local port Hillf Danton
2010-10-27 12:43 ` [PATCH 2/2] " Hillf Danton
@ 2010-10-28 23:12 ` Robert Love
2010-10-28 23:31 ` Joe Eykholt
1 sibling, 1 reply; 6+ messages in thread
From: Robert Love @ 2010-10-28 23:12 UTC (permalink / raw)
To: Hillf Danton; +Cc: devel, linux-scsi
On Wed, 2010-10-27 at 20:40 +0800, Hillf Danton wrote:
> There seems info should get freed when error encountered.
>
> Signed-off-by: Hillf Danton <dhillf@gmail.com>
> ---
>
> --- a/drivers/scsi/libfc/fc_lport.c 2010-09-13 07:07:38.000000000 +0800
> +++ b/drivers/scsi/libfc/fc_lport.c 2010-10-27 20:33:36.000000000 +0800
> @@ -1766,8 +1766,10 @@ static int fc_lport_ct_request(struct fc
> info->sg = job->reply_payload.sg_list;
>
> if (!lport->tt.exch_seq_send(lport, fp, fc_lport_bsg_resp,
> - NULL, info, tov))
> + NULL, info, tov)) {
> + kfree(info);
We cannot free the BSG info here. If you look at fc_exch_seq_send you
can see that it's attached to the exchange and then when the response is
received it is passed to the response handler, in the case
fc_lport_bsg_resp, where it is free'd.
> return -ECOMM;
> + }
> return 0;
> }
> _______________________________________________
> devel mailing list
> devel@open-fcoe.org
> http://www.open-fcoe.org/mailman/listinfo/devel
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Open-FCoE] [PATCH 1/2] libfc: fix memory leakage in local port
2010-10-28 23:12 ` [Open-FCoE] [PATCH 1/2] " Robert Love
@ 2010-10-28 23:31 ` Joe Eykholt
2010-10-28 23:35 ` Robert Love
0 siblings, 1 reply; 6+ messages in thread
From: Joe Eykholt @ 2010-10-28 23:31 UTC (permalink / raw)
To: Robert Love; +Cc: Hillf Danton, devel, linux-scsi
On 10/28/10 4:12 PM, Robert Love wrote:
> On Wed, 2010-10-27 at 20:40 +0800, Hillf Danton wrote:
>> There seems info should get freed when error encountered.
>>
>> Signed-off-by: Hillf Danton <dhillf@gmail.com>
>> ---
>>
>> --- a/drivers/scsi/libfc/fc_lport.c 2010-09-13 07:07:38.000000000 +0800
>> +++ b/drivers/scsi/libfc/fc_lport.c 2010-10-27 20:33:36.000000000 +0800
>> @@ -1766,8 +1766,10 @@ static int fc_lport_ct_request(struct fc
>> info->sg = job->reply_payload.sg_list;
>>
>> if (!lport->tt.exch_seq_send(lport, fp, fc_lport_bsg_resp,
>> - NULL, info, tov))
>> + NULL, info, tov)) {
>> + kfree(info);
>
> We cannot free the BSG info here. If you look at fc_exch_seq_send you
> can see that it's attached to the exchange and then when the response is
> received it is passed to the response handler, in the case
> fc_lport_bsg_resp, where it is free'd.
This is on failure of seq_send(). It's confusing because that returns the
sequence, which is NULL on error. So, I think the kfree() is correct.
>
>> return -ECOMM;
>> + }
>> return 0;
>> }
>> _______________________________________________
>> devel mailing list
>> devel@open-fcoe.org
>> http://www.open-fcoe.org/mailman/listinfo/devel
>
>
> _______________________________________________
> devel mailing list
> devel@open-fcoe.org
> http://www.open-fcoe.org/mailman/listinfo/devel
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Open-FCoE] [PATCH 1/2] libfc: fix memory leakage in local port
2010-10-28 23:31 ` Joe Eykholt
@ 2010-10-28 23:35 ` Robert Love
0 siblings, 0 replies; 6+ messages in thread
From: Robert Love @ 2010-10-28 23:35 UTC (permalink / raw)
To: Joe Eykholt; +Cc: Hillf Danton, devel, linux-scsi
On Thu, 2010-10-28 at 16:31 -0700, Joe Eykholt wrote:
>
> On 10/28/10 4:12 PM, Robert Love wrote:
> > On Wed, 2010-10-27 at 20:40 +0800, Hillf Danton wrote:
> >> There seems info should get freed when error encountered.
> >>
> >> Signed-off-by: Hillf Danton <dhillf@gmail.com>
> >> ---
> >>
> >> --- a/drivers/scsi/libfc/fc_lport.c 2010-09-13 07:07:38.000000000 +0800
> >> +++ b/drivers/scsi/libfc/fc_lport.c 2010-10-27 20:33:36.000000000 +0800
> >> @@ -1766,8 +1766,10 @@ static int fc_lport_ct_request(struct fc
> >> info->sg = job->reply_payload.sg_list;
> >>
> >> if (!lport->tt.exch_seq_send(lport, fp, fc_lport_bsg_resp,
> >> - NULL, info, tov))
> >> + NULL, info, tov)) {
> >> + kfree(info);
> >
> > We cannot free the BSG info here. If you look at fc_exch_seq_send you
> > can see that it's attached to the exchange and then when the response is
> > received it is passed to the response handler, in the case
> > fc_lport_bsg_resp, where it is free'd.
>
> This is on failure of seq_send(). It's confusing because that returns the
> sequence, which is NULL on error. So, I think the kfree() is correct.
>
> >
> >> return -ECOMM;
Ah, you're right... and that's why we return an error. Thanks Joe.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-10-28 23:35 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-10-27 12:40 [PATCH 1/2] libfc: fix memory leakage in local port Hillf Danton
2010-10-27 12:43 ` [PATCH 2/2] " Hillf Danton
2010-10-28 23:13 ` [Open-FCoE] " Robert Love
2010-10-28 23:12 ` [Open-FCoE] [PATCH 1/2] " Robert Love
2010-10-28 23:31 ` Joe Eykholt
2010-10-28 23:35 ` Robert Love
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).