This is an interesting case to understand why we're using interface
indexes instead pointers.
When the code below will be executed without being serialized with
ifdetach(), it might have to deal with an invalid ``ipforward_rt''.
By "invalid" here I mean a route entry pointing to a detached ifp.
Every cached entry can become "invalid" and they can be found in
multiple places: PCBs, tunneling interfaces, gateway routes, TCP
syncache...
Currently, when a route is removed from the table it loses its
RTF_UP flag. Since (hopefully) all our code check for this flag
via rtisvalid(9) and it is serialized with ifdetach(), we're safe.
But as soon as CPU0 will be able to execute the chunk below while
CPU1 is detaching the interface pointed by rt_ifp we can trigger
a bad dereference quite complicated to reproduce.
By making use of indexes with if_get()/if_put() we are sure that
the interface won't disappear while we're accessing it.
Another alternative would have been to reference count the interface
pointer on the route entry but this doesn't really fly with our
approach of sleeping until all the references are dropped when an
interface is detached.
Ok?
Index: netinet/ip_input.c
===================================================================
RCS file: /cvs/src/sys/netinet/ip_input.c,v
retrieving revision 1.261
diff -u -p -u -5 -r1.261 ip_input.c
--- netinet/ip_input.c 14 Nov 2015 15:40:40 -0000 1.261
+++ netinet/ip_input.c 19 Nov 2015 12:36:05 -0000
@@ -1514,12 +1514,18 @@ ip_forward(struct mbuf *m, struct ifnet
if (ipforward_rt.ro_rt) {
struct rtentry *rt = ipforward_rt.ro_rt;
if (rt->rt_rmx.rmx_mtu)
destmtu = rt->rt_rmx.rmx_mtu;
- else
- destmtu = ipforward_rt.ro_rt->rt_ifp->if_mtu;
+ else {
+ struct ifnet *destifp;
+
+ destifp = if_get(rt->rt_ifidx);
+ if (destifp != NULL)
+ destmtu = destifp->if_mtu;
+ if_put(destifp);
+ }
}
#endif /*IPSEC*/
ipstat.ips_cantfrag++;
break;
