On 07/12/15(Mon) 12:29, Martin Pieuchot wrote:
> On 06/12/15(Sun) 13:52, aaron.mille...@gmail.com wrote:
> > >Synopsis: Unplugging my urtwn device causes an assertion failure in the
> > >driver code (see trace)
> > >Category: system kernel amd64
> > >Environment:
> > System : OpenBSD 5.8
> > Details : OpenBSD 5.8-current (GENERIC.MP) #1706: Fri Dec 4
> > 01:49:55 MST 2015
> >
> > dera...@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP
> >
> > Architecture: OpenBSD.amd64
> > Machine : amd64
> > >Description:
> >
> > In the past, my urtwn(4) device (RTL8192CU chipset, I think) would stop
> > working after a while, and would start working again after unplugging and
> > plugging back in. I suspect it stopped working due to overheating, because
> > it's
> > really hot when I pull it out. Occasionally, this would crash the system.
> >
> > I upgraded from the Nov 25 snapshot to yesterday's snapshot, and now it
> > seems to fail every time with an assertion that drops me to the ddb prompt.
> > Here's the traceback I typed from a picture of the screen:
> >
> > panic: kernel diagnostic assertion "ifp != NULL" failed: file
> > "../../../../net/route.c", line 863
> > Stopped at Debugger+0x9: leave
> > TID PID UID PRFLAGS PFLAGS CPU COMMAND
> > 18228 18228 77 0x10 0x80 3 dhclient
> > *26266 26266 0 0x14000 0x200 6 usbtask
> > Debugger() at Debugger+0x9
> > panic() at panic+0xfe
> > __assert() at __assert+0x25
> > rtrequest_delete() at rtrequest_delete+0x20a
> > rtdeletemsg() at rtdeletemsg+0xae
> > rtflushclone1() at rtflushclone1+0x98
> > rn_walktree() at rn_walktree+0x83
> > rtrequest_delete() at rtrequest_delete+0x18a
> > rtdeletemsg() at rtdeletemsg+0xae
> > rt_if_remove_rtdelete() at rt_if_remove_rtdelete+0x2b
> > rn_walktree() at rn_walktree+0x83
> > rt_if_remove() at rt_if_remove+0x58
> > if_detach() at if_detach+0x9e
> > urtwn_detach() at urtwn_detach+0x81
> > end trace frame 0xffff800032cdddc0, count: 0
>
> Could you show me the output of "route -n show" before unplugging your
> device? Are you using dhclient(8) on multiple interfaces?
Ok I found the bug, can you tell me if the diff below helps?
Index: net/route.c
===================================================================
RCS file: /cvs/src/sys/net/route.c,v
retrieving revision 1.289
diff -u -p -r1.289 route.c
--- net/route.c 5 Dec 2015 10:07:55 -0000 1.289
+++ net/route.c 7 Dec 2015 15:10:15 -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,38 +612,29 @@ 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;
- /*
- * 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);
+ KASSERT(rt->rt_ifidx == ifp->if_index);
+
+ error = rt_delete(rt, ifp, rtableid);
if (error == 0)
- rtfree(rt);
+ rt_sendmsg(rt, RTM_DELETE, rtableid);
+
return (error);
}
static inline int
rtequal(struct rtentry *a, struct rtentry *b)
{
+ if (a == b)
+ return 1;
+
if (memcmp(rt_key(a), rt_key(b), rt_key(a)->sa_len) == 0 &&
rt_plen(a) == rt_plen(b))
return 1;
@@ -656,10 +646,25 @@ int
rtflushclone1(struct rtentry *rt, void *arg, u_int id)
{
struct rtentry *parent = arg;
+ struct ifnet *ifp;
+
+ ifp = if_get(rt->rt_ifidx);
+
+ /*
+ * This happens when an interface with a RTF_CLONING route is
+ * being detached. In this case it's safe to bail because all
+ * the routes are being purged by rt_if_remove().
+ */
+ if (ifp == NULL)
+ return 0;
- if ((rt->rt_flags & RTF_CLONED) != 0 && (rt->rt_parent == parent ||
- rtequal(rt->rt_parent, parent)))
- rtdeletemsg(rt, NULL, id);
+ if (ISSET(rt->rt_flags, RTF_CLONED) && rtequal(rt->rt_parent, parent)) {
+ rtref(rt);
+ rtdeletemsg(rt, ifp, id);
+ rtfree(rt);
+ }
+
+ if_put(ifp);
return 0;
}
@@ -801,53 +806,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;
+ struct sockaddr_in6 sa_mask;
- splsoftassert(IPL_SOFTNET);
+ KASSERT(ifp->if_index == rt->rt_ifidx);
- 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);
-#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
-
- /*
- * 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;
@@ -857,23 +829,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);
}
@@ -900,9 +859,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:
@@ -1067,7 +1067,7 @@ rtrequest(int req, struct rt_addrinfo *i
if (error != 0 && (crt = rtalloc(ndst, 0, tableid)) != NULL) {
/* overwrite cloned route */
if ((crt->rt_flags & RTF_CLONED) != 0) {
- rtdeletemsg(crt, NULL, tableid);
+ rtdeletemsg(crt, ifp, tableid);
error = rtable_insert(tableid, ndst,
info->rti_info[RTAX_NETMASK],
info->rti_info[RTAX_GATEWAY],
@@ -1235,8 +1235,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;
@@ -1256,16 +1256,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;
@@ -1273,13 +1267,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);
@@ -1691,9 +1704,12 @@ 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 error, cloning = ISSET(rt->rt_flags, RTF_CLONING);
- if (rtdeletemsg(rt, ifp, id) == 0 && cloning)
+ rtref(rt);
+ error = rtdeletemsg(rt, ifp, id);
+ rtfree(rt);
+ if (error == 0 && cloning)
return (EAGAIN);
}
@@ -1751,7 +1767,9 @@ rt_if_linkstate_change(struct rtentry *r
* clone a new route from a better source.
*/
if (rt->rt_flags & RTF_CLONED) {
+ rtref(rt);
rtdeletemsg(rt, ifp, id);
+ rtfree(rt);
return (0);
}
/* take route down */
Index: netinet/ip_icmp.c
===================================================================
RCS file: /cvs/src/sys/netinet/ip_icmp.c,v
retrieving revision 1.150
diff -u -p -r1.150 ip_icmp.c
--- netinet/ip_icmp.c 3 Dec 2015 21:11:53 -0000 1.150
+++ netinet/ip_icmp.c 7 Dec 2015 12:40:06 -0000
@@ -1042,19 +1042,21 @@ icmp_mtudisc(struct icmp *icp, u_int rta
void
icmp_mtudisc_timeout(struct rtentry *rt, struct rttimer *r)
{
- if (rt == NULL)
- panic("icmp_mtudisc_timeout: bad route to timeout");
+ struct ifnet *ifp;
+ int s;
- if ((rt->rt_flags & (RTF_DYNAMIC | RTF_HOST)) ==
- (RTF_DYNAMIC | RTF_HOST)) {
+ ifp = if_get(rt->rt_ifidx);
+ if (ifp == NULL)
+ return;
+
+ if ((rt->rt_flags & (RTF_DYNAMIC|RTF_HOST)) == (RTF_DYNAMIC|RTF_HOST)) {
void *(*ctlfunc)(int, struct sockaddr *, u_int, void *);
struct sockaddr_in sin;
- int s;
sin = *satosin(rt_key(rt));
s = splsoftnet();
- rtdeletemsg(rt, NULL, r->rtt_tableid);
+ rtdeletemsg(rt, ifp, r->rtt_tableid);
/* Notify TCP layer of increased Path MTU estimate */
ctlfunc = inetsw[ip_protox[IPPROTO_TCP]].pr_ctlinput;
@@ -1062,9 +1064,12 @@ icmp_mtudisc_timeout(struct rtentry *rt,
(*ctlfunc)(PRC_MTUINC, sintosa(&sin),
r->rtt_tableid, NULL);
splx(s);
- } else
+ } else {
if ((rt->rt_rmx.rmx_locks & RTV_MTU) == 0)
rt->rt_rmx.rmx_mtu = 0;
+ }
+
+ if_put(ifp);
}
/*
@@ -1088,17 +1093,20 @@ icmp_ratelimit(const struct in_addr *dst
void
icmp_redirect_timeout(struct rtentry *rt, struct rttimer *r)
{
- if (rt == NULL)
- panic("icmp_redirect_timeout: bad route to timeout");
+ struct ifnet *ifp;
+ int s;
- if ((rt->rt_flags & (RTF_DYNAMIC | RTF_HOST)) ==
- (RTF_DYNAMIC | RTF_HOST)) {
- int s;
+ ifp = if_get(rt->rt_ifidx);
+ if (ifp == NULL)
+ return;
+ if ((rt->rt_flags & (RTF_DYNAMIC|RTF_HOST)) == (RTF_DYNAMIC|RTF_HOST)) {
s = splsoftnet();
- rtdeletemsg(rt, NULL, r->rtt_tableid);
+ rtdeletemsg(rt, ifp, r->rtt_tableid);
splx(s);
}
+
+ if_put(ifp);
}
int
Index: netinet6/icmp6.c
===================================================================
RCS file: /cvs/src/sys/netinet6/icmp6.c,v
retrieving revision 1.182
diff -u -p -r1.182 icmp6.c
--- netinet6/icmp6.c 3 Dec 2015 21:11:53 -0000 1.182
+++ netinet6/icmp6.c 7 Dec 2015 12:39:28 -0000
@@ -1952,34 +1952,42 @@ icmp6_mtudisc_clone(struct sockaddr *dst
void
icmp6_mtudisc_timeout(struct rtentry *rt, struct rttimer *r)
{
- if (rt == NULL)
- panic("icmp6_mtudisc_timeout: bad route to timeout");
- if ((rt->rt_flags & (RTF_DYNAMIC | RTF_HOST)) ==
- (RTF_DYNAMIC | RTF_HOST)) {
- int s;
+ struct ifnet *ifp;
+ int s;
+ ifp = if_get(rt->rt_ifidx);
+ if (ifp == NULL)
+ return;
+
+ if ((rt->rt_flags & (RTF_DYNAMIC|RTF_HOST)) == (RTF_DYNAMIC|RTF_HOST)) {
s = splsoftnet();
- rtdeletemsg(rt, NULL, r->rtt_tableid);
+ rtdeletemsg(rt, ifp, r->rtt_tableid);
splx(s);
} else {
if (!(rt->rt_rmx.rmx_locks & RTV_MTU))
rt->rt_rmx.rmx_mtu = 0;
}
+
+ if_put(ifp);
}
void
icmp6_redirect_timeout(struct rtentry *rt, struct rttimer *r)
{
- if (rt == NULL)
- panic("icmp6_redirect_timeout: bad route to timeout");
- if ((rt->rt_flags & (RTF_GATEWAY | RTF_DYNAMIC | RTF_HOST)) ==
- (RTF_GATEWAY | RTF_DYNAMIC | RTF_HOST)) {
- int s;
+ struct ifnet *ifp;
+ int s;
+ ifp = if_get(rt->rt_ifidx);
+ if (ifp == NULL)
+ return;
+
+ if ((rt->rt_flags & (RTF_DYNAMIC|RTF_HOST)) == (RTF_DYNAMIC|RTF_HOST)) {
s = splsoftnet();
- rtdeletemsg(rt, NULL, r->rtt_tableid);
+ rtdeletemsg(rt, ifp, r->rtt_tableid);
splx(s);
}
+
+ if_put(ifp);
}
int *icmpv6ctl_vars[ICMPV6CTL_MAXID] = ICMPV6CTL_VARS;
