rtfree(9) conversion in rt_getifa()

2015-08-17 Thread Martin Pieuchot
Here a route lookup is done to find an existing ``ifa'' in order to
attach a new route.

This code runs under KERNEL_LOCK and returns a route entry found in
the table (RT_VALID), so rtfree(9) will not free the route.

Note that it is safe to rtfree() a route entry while keeping a pointer
to a *valid* ``ifa'' because ifa insertion/removal are serialized by
the KERNEL_LOCK.  Yes, route entries increment ifas refcounter but this
only matters for stall ifas, and there's already a check for that here.

Ok?

Index: net/route.c
===
RCS file: /cvs/src/sys/net/route.c,v
retrieving revision 1.220
diff -u -p -r1.220 route.c
--- net/route.c 17 Aug 2015 09:50:12 -  1.220
+++ net/route.c 17 Aug 2015 10:25:24 -
@@ -666,14 +666,18 @@ 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)
+   rt_key(rt)-sa_family == dst-sa_family) {
+   rtfree(rt);
return (NULL);
+   }
ifa = rt-rt_ifa;
-   if (ifa == NULL || ifa-ifa_ifp == NULL)
+   if (ifa == NULL || ifa-ifa_ifp == NULL) {
+   rtfree(rt);
return (NULL);
+   }
+   rtfree(rt);
}
if (ifa-ifa_addr-sa_family != dst-sa_family) {
struct ifaddr   *oifa = ifa;



Re: rtfree(9) conversion in rt_getifa()

2015-08-17 Thread Alexander Bluhm
On Mon, Aug 17, 2015 at 12:29:46PM +0200, Martin Pieuchot wrote:
 Here a route lookup is done to find an existing ``ifa'' in order to
 attach a new route.
 
 This code runs under KERNEL_LOCK and returns a route entry found in
 the table (RT_VALID), so rtfree(9) will not free the route.
 
 Note that it is safe to rtfree() a route entry while keeping a pointer
 to a *valid* ``ifa'' because ifa insertion/removal are serialized by
 the KERNEL_LOCK.  Yes, route entries increment ifas refcounter but this
 only matters for stall ifas, and there's already a check for that here.
 
 Ok?

OK bluhm@

 
 Index: net/route.c
 ===
 RCS file: /cvs/src/sys/net/route.c,v
 retrieving revision 1.220
 diff -u -p -r1.220 route.c
 --- net/route.c   17 Aug 2015 09:50:12 -  1.220
 +++ net/route.c   17 Aug 2015 10:25:24 -
 @@ -666,14 +666,18 @@ 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)
 + rt_key(rt)-sa_family == dst-sa_family) {
 + rtfree(rt);
   return (NULL);
 + }
   ifa = rt-rt_ifa;
 - if (ifa == NULL || ifa-ifa_ifp == NULL)
 + if (ifa == NULL || ifa-ifa_ifp == NULL) {
 + rtfree(rt);
   return (NULL);
 + }
 + rtfree(rt);
   }
   if (ifa-ifa_addr-sa_family != dst-sa_family) {
   struct ifaddr   *oifa = ifa;