On 16/05/18(Wed) 08:06, Harald Dunkel wrote:
> Hi folks,
Thanks for the report.
> hopefully its allowed to repost this message here:
>
> One gateway running 6.3 ran into the debugger last night. Last words:
>
> login: kernel: protection fault trap, code=0
> Stopped at export_sa+0x5c: movl 0(%rcx),%ecx
> ddb{0}> show panic
> the kernel did not panic
> ddb{0}> trace
> export_sa(10,ffff800033445e70) at export_sa+0x5c
> pfkeyv2_expire(ffff8000013d4c00,ffff8000013d4c00) at pfkeyv2_expire+0x14e
> tdb_timeout(ffff800033446020) at tdb_timeout+0x39
> softclock_thread(0) at softclock_thread+0xc6
> end trace frame: 0x0, count: -4
> ddb{0}> show registers
> rdi 0xffff800033445e98
> rsi 0xffff8000013d4c00
> rbp 0xffff800033445e70
> rbx 0xffff800033445e98
> rdx 0xffffffff81abdff0 cpu_info_full_primary+0x1ff0
> rcx 0xdeadbeefdeadbeef
^^^^^^^^^^^^^^^^^^
That means that the TDB has already been freed. This is possible
because the timeout sleeps on the NET_LOCK(). Diff below should prevent
that by introducing a tdb_reaper() function like we do in other parts of
the stack.
Index: netinet/ip_ipsp.c
===================================================================
RCS file: /cvs/src/sys/netinet/ip_ipsp.c,v
retrieving revision 1.229
diff -u -p -r1.229 ip_ipsp.c
--- netinet/ip_ipsp.c 6 Nov 2017 15:12:43 -0000 1.229
+++ netinet/ip_ipsp.c 16 May 2018 08:17:59 -0000
@@ -79,10 +79,11 @@ void tdb_hashstats(void);
#endif
void tdb_rehash(void);
-void tdb_timeout(void *v);
-void tdb_firstuse(void *v);
-void tdb_soft_timeout(void *v);
-void tdb_soft_firstuse(void *v);
+void tdb_reaper(void *);
+void tdb_timeout(void *);
+void tdb_firstuse(void *);
+void tdb_soft_timeout(void *);
+void tdb_soft_firstuse(void *);
int tdb_hash(u_int, u_int32_t, union sockaddr_union *, u_int8_t);
int ipsec_in_use = 0;
@@ -541,14 +542,13 @@ tdb_timeout(void *v)
{
struct tdb *tdb = v;
- if (!(tdb->tdb_flags & TDBF_TIMER))
- return;
-
NET_LOCK();
- /* If it's an "invalid" TDB do a silent expiration. */
- if (!(tdb->tdb_flags & TDBF_INVALID))
- pfkeyv2_expire(tdb, SADB_EXT_LIFETIME_HARD);
- tdb_delete(tdb);
+ if (tdb->tdb_flags & TDBF_TIMER) {
+ /* If it's an "invalid" TDB do a silent expiration. */
+ if (!(tdb->tdb_flags & TDBF_INVALID))
+ pfkeyv2_expire(tdb, SADB_EXT_LIFETIME_HARD);
+ tdb_delete(tdb);
+ }
NET_UNLOCK();
}
@@ -557,14 +557,13 @@ tdb_firstuse(void *v)
{
struct tdb *tdb = v;
- if (!(tdb->tdb_flags & TDBF_SOFT_FIRSTUSE))
- return;
-
NET_LOCK();
- /* If the TDB hasn't been used, don't renew it. */
- if (tdb->tdb_first_use != 0)
- pfkeyv2_expire(tdb, SADB_EXT_LIFETIME_HARD);
- tdb_delete(tdb);
+ if (tdb->tdb_flags & TDBF_SOFT_FIRSTUSE) {
+ /* If the TDB hasn't been used, don't renew it. */
+ if (tdb->tdb_first_use != 0)
+ pfkeyv2_expire(tdb, SADB_EXT_LIFETIME_HARD);
+ tdb_delete(tdb);
+ }
NET_UNLOCK();
}
@@ -573,13 +572,12 @@ tdb_soft_timeout(void *v)
{
struct tdb *tdb = v;
- if (!(tdb->tdb_flags & TDBF_SOFT_TIMER))
- return;
-
NET_LOCK();
- /* Soft expirations. */
- pfkeyv2_expire(tdb, SADB_EXT_LIFETIME_SOFT);
- tdb->tdb_flags &= ~TDBF_SOFT_TIMER;
+ if (tdb->tdb_flags & TDBF_SOFT_TIMER) {
+ /* Soft expirations. */
+ pfkeyv2_expire(tdb, SADB_EXT_LIFETIME_SOFT);
+ tdb->tdb_flags &= ~TDBF_SOFT_TIMER;
+ }
NET_UNLOCK();
}
@@ -588,14 +586,13 @@ tdb_soft_firstuse(void *v)
{
struct tdb *tdb = v;
- if (!(tdb->tdb_flags & TDBF_SOFT_FIRSTUSE))
- return;
-
NET_LOCK();
- /* If the TDB hasn't been used, don't renew it. */
- if (tdb->tdb_first_use != 0)
- pfkeyv2_expire(tdb, SADB_EXT_LIFETIME_SOFT);
- tdb->tdb_flags &= ~TDBF_SOFT_FIRSTUSE;
+ if (tdb->tdb_flags & TDBF_SOFT_FIRSTUSE) {
+ /* If the TDB hasn't been used, don't renew it. */
+ if (tdb->tdb_first_use != 0)
+ pfkeyv2_expire(tdb, SADB_EXT_LIFETIME_SOFT);
+ tdb->tdb_flags &= ~TDBF_SOFT_FIRSTUSE;
+ }
NET_UNLOCK();
}
@@ -841,14 +838,6 @@ tdb_free(struct tdb *tdbp)
ipo->ipo_last_searched = 0; /* Force a re-search. */
}
- /* Remove expiration timeouts. */
- tdbp->tdb_flags &= ~(TDBF_FIRSTUSE | TDBF_SOFT_FIRSTUSE | TDBF_TIMER |
- TDBF_SOFT_TIMER);
- timeout_del(&tdbp->tdb_timer_tmo);
- timeout_del(&tdbp->tdb_first_tmo);
- timeout_del(&tdbp->tdb_stimer_tmo);
- timeout_del(&tdbp->tdb_sfirst_tmo);
-
if (tdbp->tdb_ids) {
ipsp_ids_free(tdbp->tdb_ids);
tdbp->tdb_ids = NULL;
@@ -866,6 +855,23 @@ tdb_free(struct tdb *tdbp)
if ((tdbp->tdb_inext) && (tdbp->tdb_inext->tdb_onext == tdbp))
tdbp->tdb_inext->tdb_onext = NULL;
+
+ /* Remove expiration timeouts. */
+ tdbp->tdb_flags &= ~(TDBF_FIRSTUSE | TDBF_SOFT_FIRSTUSE | TDBF_TIMER |
+ TDBF_SOFT_TIMER);
+ timeout_del(&tdbp->tdb_timer_tmo);
+ timeout_del(&tdbp->tdb_first_tmo);
+ timeout_del(&tdbp->tdb_stimer_tmo);
+ timeout_del(&tdbp->tdb_sfirst_tmo);
+
+ timeout_set_proc(&tdbp->tdb_timer_tmo, tdb_reaper, tdbp);
+ timeout_add(&tdbp->tdb_timer_tmo, 0);
+}
+
+void
+tdb_reaper(void *xtdbp)
+{
+ struct tdb *tdbp = xtdbp;
free(tdbp, M_TDB, 0);
}
