On Sun, Jun 19, 2022 at 01:26:27PM +0200, Stephan Mending wrote:
> Hi,
> it crashed again.
> Here is the dmesg, this time the kernel had debugging symbols enabled.
>
> [...]
> ic0 at ichiic0
> spdmem0 at iic0 addr 0x50: 2GB DDR3 SDRAM PC3-12800 SO-DIMM
> isa0 at pcib0
> isadma0 at isa0
> vga0 at isa0 port 0x3b0/48 iomem 0xa0000/131072
> wsdisplay at vga0 not configured
> pcppi0 at isa0 port 0x61
> spkr0 at pcppi0
> wbsio0 at isa0 port 0x2e/2: NCT5104D rev 0x53
> wbsio0 port 0xa10/2 not configured
> vmm0 at mainbus0: VMX/EPT
> run0 at uhub0 port 4 configuration 1 interface 0 "Ralink 802.11 n WLAN" rev
> 2.0
> 0/1.01 addr 2
> run0: MAC/BBP RT5592 (rev 0x0222), RF RT5592 (MIMO 2T2R), address
> d8:61:62:37:5
> 6:c8
> uhub2 at uhub1 port 1 configuration 1 interface 0 "Intel Rate Matching Hub"
> rev
> 2.00/0.04 addr 2
> vscsi0 at root
> scsibus2 at vscsi0: 256 targets
> softraid0 at root
> scsibus3 at softraid0: 256 targets
> root on sd0a (7ec83d15890e2a71.a) swap on sd0b dump on sd0b
> inteldrm0: 1024x768, 32bpp
> wsdisplay0 at inteldrm0 mux 1
> wsdisplay0: screen 0-5 added (std, vt100 emulation)
> kernel: protection fault trap, code=0
> Stopped at icmp_mtudisc_timeout+0x77
> [/usr/src/sys/netinet/ip_icmp.c:1072]
> : movq 0(%rax),%rcx
> ddb{0}> ddb{0}>
> ddb{0}> bt
> icmp_mtudisc_timeout(fffffd807a4e0620,0) at icmp_mtudisc_timeout+0x77
> [/usr/src
> /sys/netinet/ip_icmp.c:1072]
> rt_timer_timer(ffffffff82324248) at rt_timer_timer+0x1cc
> [/usr/src/sys/net/rout
> e.c:1551]
> softclock_thread(ffff8000fffff260) at softclock_thread+0x13b
> [/usr/src/sys/kern
> /kern_timeout.c:681]
> end trace frame: 0x0, count: -3
> ddb{0}> call db_show_rtentry(fffffd807a4e0620, 0, 0)
> Symbol not found
>
> I'd love to know whats going wrong here.
>
This is a race condition in the rttimer code that was introduced by bluhm@
when he added the mutex around the global list.
Can you try the diff below which is a refactor I did some time ago which
changes this and uses a per route timeout instead of the global one. With
this we should not have this use after free anymore.
--
:wq Claudio
Index: net/route.c
===================================================================
RCS file: /cvs/src/sys/net/route.c,v
retrieving revision 1.410
diff -u -p -r1.410 route.c
--- net/route.c 5 May 2022 13:57:40 -0000 1.410
+++ net/route.c 13 May 2022 11:49:00 -0000
@@ -1361,7 +1361,16 @@ rt_ifa_purge_walker(struct rtentry *rt,
*/
struct mutex rttimer_mtx;
-LIST_HEAD(, rttimer_queue) rttimer_queue_head; /* [T] */
+
+struct rttimer {
+ TAILQ_ENTRY(rttimer) rtt_next; /* [T] entry on timer queue */
+ LIST_ENTRY(rttimer) rtt_link; /* [T] timers per rtentry */
+ struct timeout rtt_timeout; /* [I] timeout for this entry */
+ struct rttimer_queue *rtt_queue; /* [I] back pointer to queue */
+ struct rtentry *rtt_rt; /* [T] back pointer to route */
+ time_t rtt_expire; /* [I] rt expire time */
+ u_int rtt_tableid; /* [I] rtable id of rtt_rt */
+};
#define RTTIMER_CALLOUT(r) { \
if (r->rtt_queue->rtq_func != NULL) { \
@@ -1388,15 +1397,9 @@ LIST_HEAD(, rttimer_queue) rttimer_queue
void
rt_timer_init(void)
{
- static struct timeout rt_timer_timeout;
-
pool_init(&rttimer_pool, sizeof(struct rttimer), 0,
IPL_MPFLOOR, 0, "rttmr", NULL);
-
mtx_init(&rttimer_mtx, IPL_MPFLOOR);
- LIST_INIT(&rttimer_queue_head);
- timeout_set_proc(&rt_timer_timeout, rt_timer_timer, &rt_timer_timeout);
- timeout_add_sec(&rt_timer_timeout, 1);
}
void
@@ -1407,10 +1410,6 @@ rt_timer_queue_init(struct rttimer_queue
rtq->rtq_count = 0;
rtq->rtq_func = func;
TAILQ_INIT(&rtq->rtq_head);
-
- mtx_enter(&rttimer_mtx);
- LIST_INSERT_HEAD(&rttimer_queue_head, rtq, rtq_link);
- mtx_leave(&rttimer_mtx);
}
void
@@ -1453,6 +1452,25 @@ rt_timer_queue_count(struct rttimer_queu
return (rtq->rtq_count);
}
+static inline struct rttimer *
+rt_timer_unlink(struct rttimer *r)
+{
+ MUTEX_ASSERT_LOCKED(&rttimer_mtx);
+
+ LIST_REMOVE(r, rtt_link);
+ r->rtt_rt = NULL;
+
+ if (timeout_del(&r->rtt_timeout) == 0) {
+ /* timeout fired, so rt_timer_timer will do the cleanup */
+ return NULL;
+ }
+
+ TAILQ_REMOVE(&r->rtt_queue->rtq_head, r, rtt_next);
+ KASSERT(r->rtt_queue->rtq_count > 0);
+ r->rtt_queue->rtq_count--;
+ return r;
+}
+
void
rt_timer_remove_all(struct rtentry *rt)
{
@@ -1462,11 +1480,9 @@ rt_timer_remove_all(struct rtentry *rt)
TAILQ_INIT(&rttlist);
mtx_enter(&rttimer_mtx);
while ((r = LIST_FIRST(&rt->rt_timer)) != NULL) {
- LIST_REMOVE(r, rtt_link);
- TAILQ_REMOVE(&r->rtt_queue->rtq_head, r, rtt_next);
- TAILQ_INSERT_TAIL(&rttlist, r, rtt_next);
- KASSERT(r->rtt_queue->rtq_count > 0);
- r->rtt_queue->rtq_count--;
+ r = rt_timer_unlink(r);
+ if (r != NULL)
+ TAILQ_INSERT_TAIL(&rttlist, r, rtt_next);
}
mtx_leave(&rttimer_mtx);
@@ -1476,41 +1492,52 @@ rt_timer_remove_all(struct rtentry *rt)
}
}
+time_t
+rt_timer_get_expire(const struct rtentry *rt)
+{
+ const struct rttimer *r;
+ time_t expire = 0;
+
+ mtx_enter(&rttimer_mtx);
+ LIST_FOREACH(r, &rt->rt_timer, rtt_link) {
+ if (expire == 0 || expire > r->rtt_expire)
+ expire = r->rtt_expire;
+ }
+ mtx_leave(&rttimer_mtx);
+
+ return expire;
+}
+
int
rt_timer_add(struct rtentry *rt, struct rttimer_queue *queue, u_int rtableid)
{
struct rttimer *r, *rnew;
- time_t current_time;
rnew = pool_get(&rttimer_pool, PR_NOWAIT | PR_ZERO);
if (rnew == NULL)
return (ENOBUFS);
- current_time = getuptime();
-
rnew->rtt_rt = rt;
- rnew->rtt_time = current_time;
rnew->rtt_queue = queue;
rnew->rtt_tableid = rtableid;
+ rnew->rtt_expire = getuptime() + queue->rtq_timeout;
+ timeout_set_proc(&rnew->rtt_timeout, rt_timer_timer, rnew);
mtx_enter(&rttimer_mtx);
- rt->rt_expire = current_time + queue->rtq_timeout;
/*
* If there's already a timer with this action, destroy it before
* we add a new one.
*/
LIST_FOREACH(r, &rt->rt_timer, rtt_link) {
if (r->rtt_queue == queue) {
- LIST_REMOVE(r, rtt_link);
- TAILQ_REMOVE(&r->rtt_queue->rtq_head, r, rtt_next);
- KASSERT(r->rtt_queue->rtq_count > 0);
- r->rtt_queue->rtq_count--;
+ r = rt_timer_unlink(r);
break; /* only one per list, so we can quit... */
}
}
LIST_INSERT_HEAD(&rt->rt_timer, rnew, rtt_link);
TAILQ_INSERT_TAIL(&queue->rtq_head, rnew, rtt_next);
+ timeout_add_sec(&rnew->rtt_timeout, queue->rtq_timeout);
rnew->rtt_queue->rtq_count++;
mtx_leave(&rttimer_mtx);
@@ -1523,37 +1550,25 @@ rt_timer_add(struct rtentry *rt, struct
void
rt_timer_timer(void *arg)
{
- struct timeout *to = (struct timeout *)arg;
- struct rttimer_queue *rtq;
- struct rttimer *r;
- TAILQ_HEAD(, rttimer) rttlist;
- time_t current_time;
-
- current_time = getuptime();
- TAILQ_INIT(&rttlist);
+ struct rttimer *r = arg;
+ struct rttimer_queue *rtq = r->rtt_queue;
NET_LOCK();
mtx_enter(&rttimer_mtx);
- LIST_FOREACH(rtq, &rttimer_queue_head, rtq_link) {
- while ((r = TAILQ_FIRST(&rtq->rtq_head)) != NULL &&
- (r->rtt_time + rtq->rtq_timeout) < current_time) {
- LIST_REMOVE(r, rtt_link);
- TAILQ_REMOVE(&rtq->rtq_head, r, rtt_next);
- TAILQ_INSERT_TAIL(&rttlist, r, rtt_next);
- KASSERT(rtq->rtq_count > 0);
- rtq->rtq_count--;
- }
- }
+
+ if (r->rtt_rt != NULL)
+ LIST_REMOVE(r, rtt_link);
+ TAILQ_REMOVE(&rtq->rtq_head, r, rtt_next);
+ KASSERT(rtq->rtq_count > 0);
+ rtq->rtq_count--;
+
mtx_leave(&rttimer_mtx);
- while ((r = TAILQ_FIRST(&rttlist)) != NULL) {
- TAILQ_REMOVE(&rttlist, r, rtt_next);
+ if (r->rtt_rt != NULL)
RTTIMER_CALLOUT(r);
- pool_put(&rttimer_pool, r);
- }
NET_UNLOCK();
- timeout_add_sec(to, 1);
+ pool_put(&rttimer_pool, r);
}
#ifdef MPLS
Index: net/route.h
===================================================================
RCS file: /cvs/src/sys/net/route.h,v
retrieving revision 1.194
diff -u -p -r1.194 route.h
--- net/route.h 5 May 2022 13:57:40 -0000 1.194
+++ net/route.h 13 May 2022 11:49:18 -0000
@@ -92,6 +92,8 @@ struct rt_metrics {
#include <sys/queue.h>
#include <net/rtable.h>
+struct rttimer;
+
/*
* We distinguish between routes to hosts and routes to networks,
* preferring the former if available. For each route we infer
@@ -405,15 +407,6 @@ rtstat_inc(enum rtstat_counters c)
* add,timer} functions all used with the kind permission of BSDI.
* These allow functions to be called for routes at specific times.
*/
-struct rttimer {
- TAILQ_ENTRY(rttimer) rtt_next; /* [T] entry on timer queue */
- LIST_ENTRY(rttimer) rtt_link; /* [T] timers per rtentry */
- struct rttimer_queue *rtt_queue; /* [T] back pointer to queue */
- struct rtentry *rtt_rt; /* [I] back pointer to route */
- time_t rtt_time; /* [I] when timer registered */
- u_int rtt_tableid; /* [I] rtable id of rtt_rt */
-};
-
struct rttimer_queue {
TAILQ_HEAD(, rttimer) rtq_head; /* [T] */
LIST_ENTRY(rttimer_queue) rtq_link; /* [T] */
@@ -461,6 +454,7 @@ void rt_timer_init(void);
int rt_timer_add(struct rtentry *,
struct rttimer_queue *, u_int);
void rt_timer_remove_all(struct rtentry *);
+time_t rt_timer_get_expire(const struct rtentry *);
void rt_timer_queue_init(struct rttimer_queue *, int,
void(*)(struct rtentry *, u_int));
void rt_timer_queue_change(struct rttimer_queue *, int);
Index: net/rtsock.c
===================================================================
RCS file: /cvs/src/sys/net/rtsock.c,v
retrieving revision 1.331
diff -u -p -r1.331 rtsock.c
--- net/rtsock.c 27 Jun 2022 08:15:38 -0000 1.331
+++ net/rtsock.c 27 Jun 2022 14:00:22 -0000
@@ -132,7 +132,7 @@ int rtm_xaddrs(caddr_t, caddr_t, struc
int rtm_validate_proposal(struct rt_addrinfo *);
void rtm_setmetrics(u_long, const struct rt_metrics *,
struct rt_kmetrics *);
-void rtm_getmetrics(const struct rt_kmetrics *,
+void rtm_getmetrics(const struct rtentry *,
struct rt_metrics *);
int sysctl_iflist(int, struct walkarg *);
@@ -674,7 +674,7 @@ rtm_report(struct rtentry *rt, u_char ty
rtm->rtm_flags = rt->rt_flags;
rtm->rtm_pid = curproc->p_p->ps_pid;
rtm->rtm_seq = seq;
- rtm_getmetrics(&rt->rt_rmx, &rtm->rtm_rmx);
+ rtm_getmetrics(rt, &rtm->rtm_rmx);
rtm->rtm_addrs = info.rti_addrs;
#ifdef MPLS
rtm->rtm_mpls = info.rti_mpls;
@@ -1391,11 +1391,14 @@ rtm_setmetrics(u_long which, const struc
}
void
-rtm_getmetrics(const struct rt_kmetrics *in, struct rt_metrics *out)
+rtm_getmetrics(const struct rtentry *rt, struct rt_metrics *out)
{
+ const struct rt_kmetrics *in = &rt->rt_rmx;
int64_t expire;
expire = in->rmx_expire;
+ if (expire == 0)
+ expire = rt_timer_get_expire(rt);
if (expire != 0) {
expire -= getuptime();
expire += gettime();
@@ -1998,7 +2001,7 @@ sysctl_dumpentry(struct rtentry *rt, voi
rtm->rtm_pid = curproc->p_p->ps_pid;
rtm->rtm_flags = RTF_DONE | rt->rt_flags;
rtm->rtm_priority = rt->rt_priority & RTP_MASK;
- rtm_getmetrics(&rt->rt_rmx, &rtm->rtm_rmx);
+ rtm_getmetrics(rt, &rtm->rtm_rmx);
/* Do not account the routing table's reference. */
rtm->rtm_rmx.rmx_refcnt = rt->rt_refcnt - 1;
rtm->rtm_index = rt->rt_ifidx;
