Last change reworking art_walk() introduced a regression leading to a
of-by-one in rtable_walk(). You can trigger it by running the route(8)
regress test 5:
# cd /usr/src/regress/sbin/route && make rttest 5
# route -T5 -n show
Routing tables
Internet:
Destination Gateway Flags Refs Use Mtu Prio Iface
10.8.1/24 192.0.2.2 UGS 0 0 32768 18 (null)
10.8.1/24 192.0.2.2 GS 0 1 32768 16 (null)
See the two dangling pointers?
Problem is that rtable_walk_helper() have to keep a reference to the next
entry in the multipath chain otherwise we might forget it when purging all
the items. Here it should be to use the _LOCKED() variant since the helper
will be called inside art_walk() which will grab the mutex.
Diff below also correct the KASSERT() when deleting an entry. The refcount
*must* be at least 2 at this point: 1 for being in the table and 1 for the
caller. This should be committed separately.
Comments, ok?
Index: net/art.c
===================================================================
RCS file: /cvs/src/sys/net/art.c,v
retrieving revision 1.20
diff -u -p -r1.20 art.c
--- net/art.c 22 Jun 2016 06:32:32 -0000 1.20
+++ net/art.c 1 Jul 2016 10:06:06 -0000
@@ -706,11 +706,11 @@ art_walk_apply(struct art_root *ar,
{
int error = 0;
+ KERNEL_ASSERT_LOCKED();
+
if ((an != NULL) && (an != next)) {
/* this assumes an->an_dst is not used by f */
- KERNEL_UNLOCK();
error = (*f)(an, arg);
- KERNEL_LOCK();
}
return (error);
Index: net/rtable.c
===================================================================
RCS file: /cvs/src/sys/net/rtable.c,v
retrieving revision 1.48
diff -u -p -r1.48 rtable.c
--- net/rtable.c 22 Jun 2016 06:32:32 -0000 1.48
+++ net/rtable.c 1 Jul 2016 10:03:52 -0000
@@ -819,7 +819,7 @@ rtable_delete(unsigned int rtableid, str
npaths++;
if (npaths > 1) {
- KASSERT(rt->rt_refcnt >= 1);
+ KASSERT(rt->rt_refcnt > 1);
SRPL_REMOVE_LOCKED(&rt_rc, &an->an_rtlist, rt, rtentry,
rt_next);
@@ -834,7 +834,7 @@ rtable_delete(unsigned int rtableid, str
if (art_delete(ar, an, addr, plen) == NULL)
return (ESRCH);
- KASSERT(rt->rt_refcnt >= 1);
+ KASSERT(rt->rt_refcnt > 1);
SRPL_REMOVE_LOCKED(&rt_rc, &an->an_rtlist, rt, rtentry, rt_next);
art_put(an);
@@ -853,16 +853,19 @@ struct rtable_walk_cookie {
int
rtable_walk_helper(struct art_node *an, void *xrwc)
{
- struct srp_ref sr;
struct rtable_walk_cookie *rwc = xrwc;
- struct rtentry *rt;
+ struct rtentry *rt, *nrt;
int error = 0;
- SRPL_FOREACH(rt, &sr, &an->an_rtlist, rt_next) {
- if ((error = (*rwc->rwc_func)(rt, rwc->rwc_arg, rwc->rwc_rid)))
+ KERNEL_ASSERT_LOCKED();
+
+ SRPL_FOREACH_SAFE_LOCKED(rt, &an->an_rtlist, rt_next, nrt) {
+ KERNEL_UNLOCK();
+ error = (*rwc->rwc_func)(rt, rwc->rwc_arg, rwc->rwc_rid);
+ KERNEL_LOCK();
+ if (error)
break;
}
- SRPL_LEAVE(&sr);
return (error);
}
