Is the memory block allocated on line 315 leaked every time tcp_ipt_dump
is called?
> Is the memory block allocated on line 315 leaked every time tcp_ipt_dump is
> called?
What kernel version?
Jan Engelhardt
--
Yichen Xie <[email protected]> wrote:
> Is the memory block allocated on line 315 leaked every time tcp_ipt_dump
> is called?
It seems to be. This patch should free it.
Signed-off-by: Herbert Xu <[email protected]>
BTW, please report networking bugs to [email protected].
Thanks,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
===== net/sched/ipt.c 1.14 vs edited =====
--- 1.14/net/sched/ipt.c 2005-02-07 16:39:40 +11:00
+++ edited/net/sched/ipt.c 2005-03-23 22:28:13 +11:00
@@ -284,10 +284,12 @@
tm.lastuse = jiffies_to_clock_t(jiffies - p->tm.lastuse);
tm.expires = jiffies_to_clock_t(p->tm.expires);
RTA_PUT(skb, TCA_IPT_TM, sizeof (tm), &tm);
+ kfree(t);
return skb->len;
rtattr_failure:
skb_trim(skb, b - skb->data);
+ kfree(t);
return -1;
}
On Wed, 2005-03-23 at 06:30, Herbert Xu wrote:
> Yichen Xie <[email protected]> wrote:
> > Is the memory block allocated on line 315 leaked every time tcp_ipt_dump
> > is called?
>
> It seems to be. This patch should free it.
>
> Signed-off-by: Herbert Xu <[email protected]>
>
> BTW, please report networking bugs to [email protected].
>
Thanks for pointing this out - I _know_ its in the kernel list FAQ.
I was sitting beside R Gooch when he added "thou shalt post to netdev".
And of course i am gonna bitch about it every time someone doesnt
post to netdev;->
Just a small correction to patchlet:
The second kfree should check for existence of t.
cheers,
jamal
> Thanks,
* jamal <[email protected]> 2005-03-23 07:40
> On Wed, 2005-03-23 at 06:30, Herbert Xu wrote:
> > Yichen Xie <[email protected]> wrote:
> > > Is the memory block allocated on line 315 leaked every time tcp_ipt_dump
> > > is called?
> >
> > It seems to be. This patch should free it.
> >
> > Signed-off-by: Herbert Xu <[email protected]>
> Just a small correction to patchlet:
> The second kfree should check for existence of t.
t is either valid or NULL so it's not a problem, unless you want
to create janitor work of course. ;->
>Just a small correction to patchlet:
>The second kfree should check for existence of t.
Just look at all the recent patches about "un-useful NULL around kfree", they
are changing " if(t) kfreee(t) " => kfree(t).
No prob there.
Jan Engelhardt
--
On Wed, 2005-03-23 at 07:55, Thomas Graf wrote:
> * jamal <[email protected]> 2005-03-23 07:40
> > Just a small correction to patchlet:
> > The second kfree should check for existence of t.
>
> t is either valid or NULL so it's not a problem, unless you want
> to create janitor work of course. ;->
if t is null you still goto rtattr_failure
I have seen people put little comments of "kfree will work if you
pass it NULL" - are you saying such assumptions exist all over
net/sched? didnt understand the janitor part.
cheers,
jamal
* jamal <[email protected]> 2005-03-23 08:11
> On Wed, 2005-03-23 at 07:55, Thomas Graf wrote:
> > * jamal <[email protected]> 2005-03-23 07:40
>
> > > Just a small correction to patchlet:
> > > The second kfree should check for existence of t.
> >
> > t is either valid or NULL so it's not a problem, unless you want
> > to create janitor work of course. ;->
>
> if t is null you still goto rtattr_failure
> I have seen people put little comments of "kfree will work if you
> pass it NULL" - are you saying such assumptions exist all over
> net/sched?
kfree simply does nothing if it is given a null pointer so that
goto rtattr_failure for t == NULL is handled just fine without
a check. I will never get used to this behaviour and policy as
well though, it somewhat makes code less readable.
> didnt understand the janitor part.
It will probably be removed again by one of the regular 'remove
unnecessary pre kfree checks' patchsets.
On Wed, 2005-03-23 at 08:23, Thomas Graf wrote:
>
> kfree simply does nothing if it is given a null pointer so that
> goto rtattr_failure for t == NULL is handled just fine without
> a check. I will never get used to this behaviour and policy as
> well though, it somewhat makes code less readable.
>
I cant get used to it either; actually i dont think i would be able
to stop my fingers ;->
> > didnt understand the janitor part.
>
> It will probably be removed again by one of the regular 'remove
> unnecessary pre kfree checks' patchsets.
>
Yes, already Jan Engelhardt <[email protected]> has made that
point - although he did not post to netdev!! ;->
So ignore my comment Herbert
cheers,
jamal
>I have seen people put little comments of "kfree will work if you
>pass it NULL" - are you saying such assumptions exist all over
>net/sched?
Not only net/sched. The C standard requires that free(NULL) works.
Jan Engelhardt
--
On Wed, 2005-03-23 at 08:40, Jan Engelhardt wrote:
> >I have seen people put little comments of "kfree will work if you
> >pass it NULL" - are you saying such assumptions exist all over
> >net/sched?
>
> Not only net/sched. The C standard requires that free(NULL) works.
Thanks for clarifying this.
cheers,
jamal
Actually its well documented even on the man pages - so should have not
even have bothered discussing it;-> Should get new brand of coffee.
Apologies for unecessarily abused electrons - which means dont
respond ;->
cheers,
jamal
On Wed, 2005-03-23 at 09:05, jamal wrote:
> On Wed, 2005-03-23 at 08:40, Jan Engelhardt wrote:
> > >I have seen people put little comments of "kfree will work if you
> > >pass it NULL" - are you saying such assumptions exist all over
> > >net/sched?
> >
> > Not only net/sched. The C standard requires that free(NULL) works.
>
> Thanks for clarifying this.
>
> cheers,
> jamal
>
>
>
Thanks for confirming. Are you guys interested in this kind of leaks? I have
a list of about a hundred generated by our tool. -yichen
> -----Original Message-----
> From: Herbert Xu [mailto:[email protected]]
> Sent: Wednesday, March 23, 2005 3:31 AM
> To: Yichen Xie
> Cc: [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]
> Subject: Re: memory leak in net/sched/ipt.c?
>
> Yichen Xie <[email protected]> wrote:
> > Is the memory block allocated on line 315 leaked every time
> > tcp_ipt_dump is called?
>
> It seems to be. This patch should free it.
>
> Signed-off-by: Herbert Xu <[email protected]>
>
> BTW, please report networking bugs to [email protected].
>
> Thanks,
> --
> Visit Openswan at http://www.openswan.org/
> Email: Herbert Xu ~{PmV>HI~} <[email protected]>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
> --
> ===== net/sched/ipt.c 1.14 vs edited =====
> --- 1.14/net/sched/ipt.c 2005-02-07 16:39:40 +11:00
> +++ edited/net/sched/ipt.c 2005-03-23 22:28:13 +11:00
> @@ -284,10 +284,12 @@
> tm.lastuse = jiffies_to_clock_t(jiffies - p->tm.lastuse);
> tm.expires = jiffies_to_clock_t(p->tm.expires);
> RTA_PUT(skb, TCA_IPT_TM, sizeof (tm), &tm);
> + kfree(t);
> return skb->len;
>
> rtattr_failure:
> skb_trim(skb, b - skb->data);
> + kfree(t);
> return -1;
> }
>
>
>Thanks for confirming. Are you guys interested in this kind of leaks? I have
>a list of about a hundred generated by our tool. -yichen
This list is not SUN (where you would have to enter a service contract to
report bugs (efficiently)).
Jan Engelhardt
--
Jan Engelhardt wrote:
>>Thanks for confirming. Are you guys interested in this kind of leaks? I have
>>a list of about a hundred generated by our tool. -yichen
>
>
> This list is not SUN (where you would have to enter a service contract to
> report bugs (efficiently)).
>
>
> Jan Engelhardt
Just to be clear(er), that's a "Yes" answer to your question.
--
~Randy
On Wed, Mar 23, 2005 at 10:27:32AM -0800, Yichen Xie wrote:
> Thanks for confirming. Are you guys interested in this kind of leaks? I have
> a list of about a hundred generated by our tool. -yichen
Certainly. Please post all the networking leaks to [email protected].
Thanks,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt