On Wed, Dec 16, 2015 at 09:46:26PM +0100, Alexander Bluhm wrote:
> 10.188.70.17 fe:e1:ba:d0:d5:6d UHLS 0 3 - 8 vio0
This is this route that crashed the machine when the arp entry expired.
When I move the rtref()/rtfree() calls into rtdeletemsg() it also
protects the calls from arptfree().
> __assert() at __assert+0x25
> rtfree() at rtfree+0x11e
> rtable_delete() at rtable_delete+0x5d
> rt_delete() at rt_delete+0x6e
> rtdeletemsg() at rtdeletemsg+0x2d
> arptfree() at arptfree+0x4f
> arptimer() at arptimer+0x63
After rt_delete() has called rtable_delete(), the route is still
used and should not be destroyed.
bluhm
Index: net/route.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/net/route.c,v
retrieving revision 1.292
diff -u -p -r1.292 route.c
--- net/route.c 11 Dec 2015 08:58:23 -0000 1.292
+++ net/route.c 16 Dec 2015 22:42:34 -0000
@@ -159,8 +159,7 @@ struct sockaddr *rt_plentosa(sa_family_t
struct ifaddr *ifa_ifwithroute(int, struct sockaddr *, struct sockaddr *,
u_int);
-int rtrequest_delete(struct rt_addrinfo *, u_int8_t, struct ifnet *,
- struct rtentry **, u_int);
+int rt_delete(struct rtentry *, struct ifnet *, unsigned int);
#ifdef DDB
void db_print_sa(struct sockaddr *);
@@ -613,34 +612,22 @@ out:
}
/*
- * Delete a route and generate a message
+ * Delete a route and generate a message. The caller must hold a reference
+ * on ``rt''.
*/
int
-rtdeletemsg(struct rtentry *rt, struct ifnet *ifp, u_int tableid)
+rtdeletemsg(struct rtentry *rt, struct ifnet *ifp, unsigned int rtableid)
{
int error;
- struct rt_addrinfo info;
- unsigned int ifidx;
- struct sockaddr_in6 sa_mask;
KASSERT(rt->rt_ifidx == ifp->if_index);
- /*
- * Request the new route so that the entry is not actually
- * deleted. That will allow the information being reported to
- * be accurate (and consistent with route_output()).
- */
- bzero((caddr_t)&info, sizeof(info));
- info.rti_info[RTAX_DST] = rt_key(rt);
- if (!ISSET(rt->rt_flags, RTF_HOST))
- info.rti_info[RTAX_NETMASK] = rt_plen2mask(rt, &sa_mask);
- info.rti_info[RTAX_GATEWAY] = rt->rt_gateway;
- info.rti_flags = rt->rt_flags;
- ifidx = rt->rt_ifidx;
- error = rtrequest_delete(&info, rt->rt_priority, ifp, &rt, tableid);
- rt_missmsg(RTM_DELETE, &info, info.rti_flags, ifidx, error, tableid);
+ rtref(rt);
+ error = rt_delete(rt, ifp, rtableid);
if (error == 0)
- rtfree(rt);
+ rt_sendmsg(rt, RTM_DELETE, rtableid);
+ rtfree(rt);
+
return (error);
}
@@ -671,10 +658,10 @@ rtflushclone1(struct rtentry *rt, void *
* the routes are being purged by rt_if_remove().
*/
if (ifp == NULL)
- return 0;
+ return 0;
if (ISSET(rt->rt_flags, RTF_CLONED) && rtequal(rt->rt_parent, parent))
- rtdeletemsg(rt, ifp, id);
+ rtdeletemsg(rt, ifp, id);
if_put(ifp);
return 0;
@@ -818,60 +805,20 @@ rt_getifa(struct rt_addrinfo *info, u_in
}
int
-rtrequest_delete(struct rt_addrinfo *info, u_int8_t prio, struct ifnet *ifp,
- struct rtentry **ret_nrt, u_int tableid)
+rt_delete(struct rtentry *rt, struct ifnet *ifp, unsigned int rtableid)
{
- struct rtentry *rt;
- int error;
-
- splsoftassert(IPL_SOFTNET);
-
- if (!rtable_exists(tableid))
- return (EAFNOSUPPORT);
- rt = rtable_lookup(tableid, info->rti_info[RTAX_DST],
- info->rti_info[RTAX_NETMASK], info->rti_info[RTAX_GATEWAY], prio);
- if (rt == NULL)
- return (ESRCH);
-
- /* Make sure that's the route the caller want to delete. */
- if (ifp != NULL && ifp->if_index != rt->rt_ifidx) {
- rtfree(rt);
- return (ESRCH);
- }
+ struct sockaddr_in6 sa_mask;
-#ifndef SMALL_KERNEL
- /*
- * If we got multipath routes, we require users to specify
- * a matching gateway.
- */
- if ((rt->rt_flags & RTF_MPATH) &&
- info->rti_info[RTAX_GATEWAY] == NULL) {
- rtfree(rt);
- return (ESRCH);
- }
-#endif
+ KASSERT(ifp->if_index == rt->rt_ifidx);
- /*
- * Since RTP_LOCAL cannot be set by userland, make
- * sure that local routes are only modified by the
- * kernel.
- */
- if ((rt->rt_flags & (RTF_LOCAL|RTF_BROADCAST)) &&
- prio != RTP_LOCAL) {
- rtfree(rt);
- return (EINVAL);
- }
+ splsoftassert(IPL_SOFTNET);
- error = rtable_delete(tableid, info->rti_info[RTAX_DST],
- info->rti_info[RTAX_NETMASK], rt);
- if (error != 0) {
- rtfree(rt);
+ if (rtable_delete(rtableid, rt_key(rt), rt_plen2mask(rt, &sa_mask), rt))
return (ESRCH);
- }
/* clean up any cloned children */
if ((rt->rt_flags & RTF_CLONING) != 0)
- rtflushclone(tableid, rt);
+ rtflushclone(rtableid, rt);
rtfree(rt->rt_gwroute);
rt->rt_gwroute = NULL;
@@ -881,23 +828,10 @@ rtrequest_delete(struct rt_addrinfo *inf
rt->rt_flags &= ~RTF_UP;
- if (ifp == NULL) {
- ifp = if_get(rt->rt_ifidx);
- KASSERT(ifp != NULL);
- ifp->if_rtrequest(ifp, RTM_DELETE, rt);
- if_put(ifp);
- } else {
- KASSERT(ifp->if_index == rt->rt_ifidx);
- ifp->if_rtrequest(ifp, RTM_DELETE, rt);
- }
+ ifp->if_rtrequest(ifp, RTM_DELETE, rt);
atomic_inc_int(&rttrash);
- if (ret_nrt != NULL)
- *ret_nrt = rt;
- else
- rtfree(rt);
-
return (0);
}
@@ -924,9 +858,50 @@ rtrequest(int req, struct rt_addrinfo *i
info->rti_info[RTAX_NETMASK] = NULL;
switch (req) {
case RTM_DELETE:
- error = rtrequest_delete(info, prio, NULL, ret_nrt, tableid);
- if (error)
+ rt = rtable_lookup(tableid, info->rti_info[RTAX_DST],
+ info->rti_info[RTAX_NETMASK],
+ info->rti_info[RTAX_GATEWAY], prio);
+ if (rt == NULL)
+ return (ESRCH);
+
+#ifndef SMALL_KERNEL
+ /*
+ * If we got multipath routes, we require users to specify
+ * a matching gateway.
+ */
+ if (ISSET(rt->rt_flags, RTF_MPATH) &&
+ info->rti_info[RTAX_GATEWAY] == NULL) {
+ rtfree(rt);
+ return (ESRCH);
+ }
+#endif
+
+ /*
+ * Since RTP_LOCAL cannot be set by userland, make
+ * sure that local routes are only modified by the
+ * kernel.
+ */
+ if (ISSET(rt->rt_flags, RTF_LOCAL|RTF_BROADCAST) &&
+ prio != RTP_LOCAL) {
+ rtfree(rt);
+ return (EINVAL);
+ }
+
+ ifp = if_get(rt->rt_ifidx);
+ KASSERT(ifp != NULL);
+ error = rt_delete(rt, ifp, tableid);
+ if_put(ifp);
+
+ if (error) {
+ rtfree(rt);
return (error);
+ }
+
+ if (ret_nrt != NULL)
+ *ret_nrt = rt;
+ else
+ rtfree(rt);
+
break;
case RTM_RESOLVE:
@@ -1265,8 +1240,8 @@ rt_ifa_del(struct ifaddr *ifa, int flags
struct rtentry *rt;
struct mbuf *m = NULL;
struct sockaddr *deldst;
- struct rt_addrinfo info;
- struct sockaddr_rtlabel sa_rl;
+ struct sockaddr *mask = NULL;
+ struct sockaddr *gateway = NULL;
unsigned int rtableid = ifp->if_rdomain;
uint8_t prio = ifp->if_priority + RTP_STATIC;
int error;
@@ -1286,16 +1261,10 @@ rt_ifa_del(struct ifaddr *ifa, int flags
dst = deldst;
}
- memset(&info, 0, sizeof(info));
- info.rti_ifa = ifa;
- info.rti_flags = flags;
- info.rti_info[RTAX_DST] = dst;
if ((flags & RTF_LLINFO) == 0)
- info.rti_info[RTAX_GATEWAY] = ifa->ifa_addr;
- info.rti_info[RTAX_LABEL] = rtlabel_id2sa(ifp->if_rtlabelid, &sa_rl);
-
+ gateway = ifa->ifa_addr;
if ((flags & RTF_HOST) == 0)
- info.rti_info[RTAX_NETMASK] = ifa->ifa_netmask;
+ mask = ifa->ifa_netmask;
if (flags & (RTF_LOCAL|RTF_BROADCAST))
prio = RTP_LOCAL;
@@ -1303,13 +1272,32 @@ rt_ifa_del(struct ifaddr *ifa, int flags
if (flags & RTF_CONNECTED)
prio = RTP_CONNECTED;
- error = rtrequest_delete(&info, prio, ifp, &rt, rtableid);
- if (error == 0) {
- rt_sendmsg(rt, RTM_DELETE, rtableid);
- if (flags & RTF_LOCAL)
- rt_sendaddrmsg(rt, RTM_DELADDR);
+ rt = rtable_lookup(rtableid, dst, mask, gateway, prio);
+ if (rt == NULL)
+ return (ESRCH);
+
+#ifndef SMALL_KERNEL
+ /*
+ * If we got multipath routes, we require users to specify
+ * a matching gateway.
+ */
+ if (ISSET(rt->rt_flags, RTF_MPATH) && gateway == NULL) {
rtfree(rt);
+ return (ESRCH);
}
+#endif
+
+ /* Make sure that's the route the caller want to delete. */
+ if (rt->rt_ifidx != ifp->if_index) {
+ rtfree(rt);
+ return (ESRCH);
+ }
+
+ error = rtdeletemsg(rt, ifp, rtableid);
+ if (error == 0 && (flags & RTF_LOCAL))
+ rt_sendaddrmsg(rt, RTM_DELADDR);
+ rtfree(rt);
+
if (m != NULL)
m_free(m);
@@ -1721,7 +1709,7 @@ rt_if_remove_rtdelete(struct rtentry *rt
struct ifnet *ifp = vifp;
if (rt->rt_ifidx == ifp->if_index) {
- int cloning = (rt->rt_flags & RTF_CLONING);
+ int cloning = ISSET(rt->rt_flags, RTF_CLONING);
if (rtdeletemsg(rt, ifp, id) == 0 && cloning)
return (EAGAIN);
