From fa253399af72f4968a7eddd5210066700b92d2c0 Mon Sep 17 00:00:00 2001 From: glebius Date: Thu, 11 Aug 2005 08:14:53 +0000 Subject: [PATCH] o Make rt_check() function more strict: - rt0 passed to rt_check() must not be NULL, assert this. - rt returned by rt_check() must be valid locked rtentry, if no error occured. o Modify callers, so that they never pass NULL rt0 to rt_check(). Reviewed by: sam, ume (nd6.c) --- sys/net/if_atmsubr.c | 12 +++--- sys/net/if_fwsubr.c | 12 +++--- sys/net/if_iso88025subr.c | 12 +++--- sys/net/route.c | 88 +++++++++++++++++++-------------------- sys/netinet6/nd6.c | 6 +++ 5 files changed, 71 insertions(+), 59 deletions(-) diff --git a/sys/net/if_atmsubr.c b/sys/net/if_atmsubr.c index 6e662a02f563..a0e84ea7981c 100644 --- a/sys/net/if_atmsubr.c +++ b/sys/net/if_atmsubr.c @@ -152,14 +152,16 @@ atm_output(struct ifnet *ifp, struct mbuf *m0, struct sockaddr *dst, case AF_INET: case AF_INET6: { - struct rtentry *rt; + struct rtentry *rt = NULL; /* * check route */ - error = rt_check(&rt, &rt0, dst); - if (error) - goto bad; - RT_UNLOCK(rt); + if (rt0 != NULL) { + error = rt_check(&rt, &rt0, dst); + if (error) + goto bad; + RT_UNLOCK(rt); + } if (dst->sa_family == AF_INET6) etype = ETHERTYPE_IPV6; diff --git a/sys/net/if_fwsubr.c b/sys/net/if_fwsubr.c index 0aaa949b0cd5..b08ed6a68fe8 100644 --- a/sys/net/if_fwsubr.c +++ b/sys/net/if_fwsubr.c @@ -81,7 +81,7 @@ firewire_output(struct ifnet *ifp, struct mbuf *m, struct sockaddr *dst, { struct fw_com *fc = IFP2FC(ifp); int error, type; - struct rtentry *rt; + struct rtentry *rt = NULL; struct m_tag *mtag; union fw_encap *enc; struct fw_hwaddr *destfw; @@ -103,10 +103,12 @@ firewire_output(struct ifnet *ifp, struct mbuf *m, struct sockaddr *dst, goto bad; } - error = rt_check(&rt, &rt0, dst); - if (error) - goto bad; - RT_UNLOCK(rt); + if (rt0 != NULL) { + error = rt_check(&rt, &rt0, dst); + if (error) + goto bad; + RT_UNLOCK(rt); + } /* * For unicast, we make a tag to store the lladdr of the diff --git a/sys/net/if_iso88025subr.c b/sys/net/if_iso88025subr.c index 2170e0e52215..5e8382faea64 100644 --- a/sys/net/if_iso88025subr.c +++ b/sys/net/if_iso88025subr.c @@ -243,7 +243,7 @@ iso88025_output(ifp, m, dst, rt0) struct iso88025_header *th; struct iso88025_header gen_th; struct sockaddr_dl *sdl = NULL; - struct rtentry *rt; + struct rtentry *rt = NULL; #ifdef MAC error = mac_check_ifnet_transmit(ifp, m); @@ -260,10 +260,12 @@ iso88025_output(ifp, m, dst, rt0) /* Calculate routing info length based on arp table entry */ /* XXX any better way to do this ? */ - error = rt_check(&rt, &rt0, dst); - if (error) - goto bad; - RT_UNLOCK(rt); + if (rt0 != NULL) { + error = rt_check(&rt, &rt0, dst); + if (error) + goto bad; + RT_UNLOCK(rt); + } if (rt && (sdl = (struct sockaddr_dl *)rt->rt_gateway)) if (SDL_ISO88025(sdl)->trld_rcf != 0) diff --git a/sys/net/route.c b/sys/net/route.c index 51b2631a51e6..126d69ab6634 100644 --- a/sys/net/route.c +++ b/sys/net/route.c @@ -1262,51 +1262,51 @@ rt_check(struct rtentry **lrt, struct rtentry **lrt0, struct sockaddr *dst) struct rtentry *rt0; int error; - rt0 = *lrt0; - rt = rt0; - if (rt) { - /* NB: the locking here is tortuous... */ - RT_LOCK(rt); - if ((rt->rt_flags & RTF_UP) == 0) { - RT_UNLOCK(rt); - rt = rtalloc1(dst, 1, 0UL); - if (rt != NULL) { - RT_REMREF(rt); - /* XXX what about if change? */ - } else - senderr(EHOSTUNREACH); - rt0 = rt; - } - /* XXX BSD/OS checks dst->sa_family != AF_NS */ - if (rt->rt_flags & RTF_GATEWAY) { - if (rt->rt_gwroute == NULL) - goto lookup; - rt = rt->rt_gwroute; - RT_LOCK(rt); /* NB: gwroute */ - if ((rt->rt_flags & RTF_UP) == 0) { - rtfree(rt); /* unlock gwroute */ - rt = rt0; - lookup: - RT_UNLOCK(rt0); - rt = rtalloc1(rt->rt_gateway, 1, 0UL); - RT_LOCK(rt0); - rt0->rt_gwroute = rt; - if (rt == NULL) { - RT_UNLOCK(rt0); - senderr(EHOSTUNREACH); - } - } - RT_UNLOCK(rt0); - } - /* XXX why are we inspecting rmx_expire? */ - error = (rt->rt_flags & RTF_REJECT) && - (rt->rt_rmx.rmx_expire == 0 || - time_second < rt->rt_rmx.rmx_expire); - if (error) { - RT_UNLOCK(rt); - senderr(rt == rt0 ? EHOSTDOWN : EHOSTUNREACH); - } + KASSERT(*lrt0 != NULL, ("rt_check")); + rt = rt0 = *lrt0; + + /* NB: the locking here is tortuous... */ + RT_LOCK(rt); + if ((rt->rt_flags & RTF_UP) == 0) { + RT_UNLOCK(rt); + rt = rtalloc1(dst, 1, 0UL); + if (rt != NULL) { + RT_REMREF(rt); + /* XXX what about if change? */ + } else + senderr(EHOSTUNREACH); + rt0 = rt; } + /* XXX BSD/OS checks dst->sa_family != AF_NS */ + if (rt->rt_flags & RTF_GATEWAY) { + if (rt->rt_gwroute == NULL) + goto lookup; + rt = rt->rt_gwroute; + RT_LOCK(rt); /* NB: gwroute */ + if ((rt->rt_flags & RTF_UP) == 0) { + rtfree(rt); /* unlock gwroute */ + rt = rt0; + lookup: + RT_UNLOCK(rt0); + rt = rtalloc1(rt->rt_gateway, 1, 0UL); + RT_LOCK(rt0); + rt0->rt_gwroute = rt; + if (rt == NULL) { + RT_UNLOCK(rt0); + senderr(EHOSTUNREACH); + } + } + RT_UNLOCK(rt0); + } + /* XXX why are we inspecting rmx_expire? */ + error = (rt->rt_flags & RTF_REJECT) && + (rt->rt_rmx.rmx_expire == 0 || + time_second < rt->rt_rmx.rmx_expire); + if (error) { + RT_UNLOCK(rt); + senderr(rt == rt0 ? EHOSTDOWN : EHOSTUNREACH); + } + *lrt = rt; *lrt0 = rt0; return (0); diff --git a/sys/netinet6/nd6.c b/sys/netinet6/nd6.c index 2c4cc53ae766..4265b3152352 100644 --- a/sys/netinet6/nd6.c +++ b/sys/netinet6/nd6.c @@ -2065,6 +2065,12 @@ nd6_storelladdr(ifp, rt0, m, dst, desten) } } + if (rt0 == NULL) { + /* this could happen, if we could not allocate memory */ + m_freem(m); + return (ENOMEM); + } + error = rt_check(&rt, &rt0, dst); if (error) { m_freem(m);