Re: rtentry leak

2014-11-06 Thread Martin Pieuchot 
On 05/11/14(Wed) 19:53, Chris Cappuccio wrote:
 Martin Pieuchot [mpieuc...@nolizard.org] wrote:
  
  @@ -653,12 +653,12 @@ ifa_ifwithroute(int flags, struct sockad
  struct rtentry  *rt = rtalloc(gateway, 0, rtableid);
  if (rt == NULL)
  return (NULL);
  -   rt-rt_refcnt--;
  /* The gateway must be local if the same address family. */
  if ((rt-rt_flags  RTF_GATEWAY) 
  rt_key(rt)-sa_family == dst-sa_family)
  return (NULL);
 
 And
   if ((rt-rt_flags  RTF_GATEWAY) 
   rt_key(rt)-sa_family == dst-sa_family) {
 + rtfree(rt);
   return (NULL);
   }
 
 
  ifa = rt-rt_ifa;
  +   rtfree(rt);
  if (ifa == NULL || ifa-ifa_ifp == NULL)
  return (NULL);

Indeed!  And the ifa might also be freed so this chunk is completely
wrong.  Here's a version of the diff without it, ok?


Index: net/route.c
===
RCS file: /home/ncvs/src/sys/net/route.c,v
retrieving revision 1.189
diff -u -p -r1.189 route.c
--- net/route.c 4 Nov 2014 15:24:40 -   1.189
+++ net/route.c 6 Nov 2014 10:24:02 -
@@ -215,7 +215,7 @@ route_init(void)
 {
struct domain*dom;
 
-   pool_init(rtentry_pool, sizeof(struct rtentry), 0, 0, 0, rtent,
+   pool_init(rtentry_pool, sizeof(struct rtentry), 0, 0, 0, rtentry,
NULL);
rn_init();  /* initialize all zeroes, all ones, mask table */
 
@@ -1220,7 +1220,7 @@ rt_ifa_addloop(struct ifaddr *ifa)
if (rt == NULL || !ISSET(rt-rt_flags, flags));
rt_ifa_add(ifa, RTF_UP | flags, ifa-ifa_addr);
if (rt)
-   rt-rt_refcnt--;
+   rtfree(rt);
 }
 
 /*
@@ -1267,7 +1267,7 @@ rt_ifa_delloop(struct ifaddr *ifa)
if (rt != NULL  ISSET(rt-rt_flags, flags))
rt_ifa_del(ifa, flags, ifa-ifa_addr);
if (rt)
-   rt-rt_refcnt--;
+   rtfree(rt);
 }
 
 /*

Re: rtentry leak

2014-11-06 Thread Chris Cappuccio 
Martin Pieuchot [mpieuc...@nolizard.org] wrote:
 
 Indeed!  And the ifa might also be freed so this chunk is completely
 wrong.  Here's a version of the diff without it, ok?
 

This looks ok to me

 
 Index: net/route.c
 ===
 RCS file: /home/ncvs/src/sys/net/route.c,v
 retrieving revision 1.189
 diff -u -p -r1.189 route.c
 --- net/route.c   4 Nov 2014 15:24:40 -   1.189
 +++ net/route.c   6 Nov 2014 10:24:02 -
 @@ -215,7 +215,7 @@ route_init(void)
  {
   struct domain*dom;
  
 - pool_init(rtentry_pool, sizeof(struct rtentry), 0, 0, 0, rtent,
 + pool_init(rtentry_pool, sizeof(struct rtentry), 0, 0, 0, rtentry,
   NULL);
   rn_init();  /* initialize all zeroes, all ones, mask table */
  
 @@ -1220,7 +1220,7 @@ rt_ifa_addloop(struct ifaddr *ifa)
   if (rt == NULL || !ISSET(rt-rt_flags, flags));
   rt_ifa_add(ifa, RTF_UP | flags, ifa-ifa_addr);
   if (rt)
 - rt-rt_refcnt--;
 + rtfree(rt);
  }
  
  /*
 @@ -1267,7 +1267,7 @@ rt_ifa_delloop(struct ifaddr *ifa)
   if (rt != NULL  ISSET(rt-rt_flags, flags))
   rt_ifa_del(ifa, flags, ifa-ifa_addr);
   if (rt)
 - rt-rt_refcnt--;
 + rtfree(rt);
  }
  
  /*

-- 
The bums will always lose

rtentry leak

2014-11-05 Thread Martin Pieuchot 

Diff below fixes a rtentry leak in rt_ifa_delloop() and do two other
conversions to rtfree(9).  While here rename the pool in rtentry
which makes it easier to understand where to look for leaks.

I can commit these chunks separately if needed.

ok?

Index: net/route.c
===
RCS file: /home/ncvs/src/sys/net/route.c,v
retrieving revision 1.189
diff -u -p -r1.189 route.c
--- net/route.c 4 Nov 2014 15:24:40 -   1.189
+++ net/route.c 5 Nov 2014 15:26:45 -
@@ -215,7 +215,7 @@ route_init(void)
 {
struct domain*dom;

-   pool_init(rtentry_pool, sizeof(struct rtentry), 0, 0, 0, rtent,
+   pool_init(rtentry_pool, sizeof(struct rtentry), 0, 0, 0, rtentry,
NULL);
rn_init();  /* initialize all zeroes, all ones, mask table */

@@ -653,12 +653,12 @@ ifa_ifwithroute(int flags, struct sockad
struct rtentry  *rt = rtalloc(gateway, 0, rtableid);
if (rt == NULL)
return (NULL);
-   rt-rt_refcnt--;
/* The gateway must be local if the same address family. */
if ((rt-rt_flags  RTF_GATEWAY) 
rt_key(rt)-sa_family == dst-sa_family)
return (NULL);
ifa = rt-rt_ifa;
+   rtfree(rt);
if (ifa == NULL || ifa-ifa_ifp == NULL)
return (NULL);
}
@@ -1220,7 +1220,7 @@ rt_ifa_addloop(struct ifaddr *ifa)
if (rt == NULL || !ISSET(rt-rt_flags, flags));
rt_ifa_add(ifa, RTF_UP | flags, ifa-ifa_addr);
if (rt)
-   rt-rt_refcnt--;
+   rtfree(rt);
 }

 /*
@@ -1267,7 +1267,7 @@ rt_ifa_delloop(struct ifaddr *ifa)
if (rt != NULL  ISSET(rt-rt_flags, flags))
rt_ifa_del(ifa, flags, ifa-ifa_addr);
if (rt)
-   rt-rt_refcnt--;
+   rtfree(rt);
 }

 /*

Re: rtentry leak

2014-11-05 Thread Chris Cappuccio 
Martin Pieuchot [mpieuc...@nolizard.org] wrote:
 
 @@ -653,12 +653,12 @@ ifa_ifwithroute(int flags, struct sockad
   struct rtentry  *rt = rtalloc(gateway, 0, rtableid);
   if (rt == NULL)
   return (NULL);
 - rt-rt_refcnt--;
   /* The gateway must be local if the same address family. */
   if ((rt-rt_flags  RTF_GATEWAY) 
   rt_key(rt)-sa_family == dst-sa_family)
   return (NULL);

And
if ((rt-rt_flags  RTF_GATEWAY) 
rt_key(rt)-sa_family == dst-sa_family) {
+   rtfree(rt);
return (NULL);
}


   ifa = rt-rt_ifa;
 + rtfree(rt);
   if (ifa == NULL || ifa-ifa_ifp == NULL)
   return (NULL);