From 804d0a749c66d5dc8ae510207a392a249d98c2c7 Mon Sep 17 00:00:00 2001 From: tobhe Date: Wed, 11 Oct 2023 22:13:16 +0000 Subject: [PATCH] Prevent deref-after-free when tdb_timeout() fires on invalid new tdb. When receiving a pfkeyv2 SADB_ADD message, a newly created tdb can fail in tdb_init(), which causes the tdb to not get added to the global tdb list and an immediate dereference. If a lifetime timeout triggers on this tdb, it will unconditionally try to remove it from the list and in the process deref once more than allowed, causing a one bit corruption in the already freed up slot in the tdb pool. We resolve this issue by moving timeout_add() after tdb_init() just before puttdb(). This means tdbs failing initialization get discarded immediately as they only hold a single reference. Valid tdbs get their timeouts activated just before we add them to the tdb list, meaning the timeout can safely assume they are linked. Feedback from mvs@ and millert@ ok mvs@ mbuhl@ --- sys/net/pfkeyv2.c | 9 ++++++++- sys/net/pfkeyv2_convert.c | 8 +------- sys/netinet/ip_ipsp.c | 19 ++++++++++++++++++- sys/netinet/ip_ipsp.h | 3 ++- 4 files changed, 29 insertions(+), 10 deletions(-) diff --git a/sys/net/pfkeyv2.c b/sys/net/pfkeyv2.c index d4ca7c2358e..6916810af94 100644 --- a/sys/net/pfkeyv2.c +++ b/sys/net/pfkeyv2.c @@ -1,4 +1,4 @@ -/* $OpenBSD: pfkeyv2.c,v 1.258 2023/09/29 18:40:08 tobhe Exp $ */ +/* $OpenBSD: pfkeyv2.c,v 1.259 2023/10/11 22:13:16 tobhe Exp $ */ /* * @(#)COPYRIGHT 1.1 (NRL) 17 January 1995 @@ -1391,6 +1391,9 @@ pfkeyv2_dosend(struct socket *so, void *message, int len) /* Delete old version of the SA, insert new one */ tdb_delete(sa2); + + tdb_addtimeouts(newsa); + puttdb(newsa); } else { /* @@ -1423,6 +1426,8 @@ pfkeyv2_dosend(struct socket *so, void *message, int len) #endif import_iface(sa2, headers[SADB_X_EXT_IFACE]); + tdb_addtimeouts(sa2); + if (headers[SADB_EXT_ADDRESS_SRC] || headers[SADB_EXT_ADDRESS_PROXY]) { mtx_enter(&tdb_sadb_mtx); @@ -1565,6 +1570,8 @@ pfkeyv2_dosend(struct socket *so, void *message, int len) goto ret; } + tdb_addtimeouts(newsa); + /* Add TDB in table */ puttdb(newsa); } diff --git a/sys/net/pfkeyv2_convert.c b/sys/net/pfkeyv2_convert.c index 83a1feb614e..2a7e1d94c45 100644 --- a/sys/net/pfkeyv2_convert.c +++ b/sys/net/pfkeyv2_convert.c @@ -1,4 +1,4 @@ -/* $OpenBSD: pfkeyv2_convert.c,v 1.81 2023/09/16 09:33:27 mpi Exp $ */ +/* $OpenBSD: pfkeyv2_convert.c,v 1.82 2023/10/11 22:13:16 tobhe Exp $ */ /* * The author of this code is Angelos D. Keromytis (angelos@keromytis.org) * @@ -302,9 +302,6 @@ import_lifetime(struct tdb *tdb, struct sadb_lifetime *sadb_lifetime, int type) if ((tdb->tdb_exp_timeout = sadb_lifetime->sadb_lifetime_addtime) != 0) { tdb->tdb_flags |= TDBF_TIMER; - if (timeout_add_sec(&tdb->tdb_timer_tmo, - tdb->tdb_exp_timeout)) - tdb_ref(tdb); } else tdb->tdb_flags &= ~TDBF_TIMER; @@ -331,9 +328,6 @@ import_lifetime(struct tdb *tdb, struct sadb_lifetime *sadb_lifetime, int type) if ((tdb->tdb_soft_timeout = sadb_lifetime->sadb_lifetime_addtime) != 0) { tdb->tdb_flags |= TDBF_SOFT_TIMER; - if (timeout_add_sec(&tdb->tdb_stimer_tmo, - tdb->tdb_soft_timeout)) - tdb_ref(tdb); } else tdb->tdb_flags &= ~TDBF_SOFT_TIMER; diff --git a/sys/netinet/ip_ipsp.c b/sys/netinet/ip_ipsp.c index 90179f0f8f3..ee97b159021 100644 --- a/sys/netinet/ip_ipsp.c +++ b/sys/netinet/ip_ipsp.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ip_ipsp.c,v 1.276 2023/08/07 03:43:57 dlg Exp $ */ +/* $OpenBSD: ip_ipsp.c,v 1.277 2023/10/11 22:13:16 tobhe Exp $ */ /* * The authors of this code are John Ioannidis (ji@tla.org), * Angelos D. Keromytis (kermit@csd.uch.gr), @@ -716,6 +716,23 @@ tdb_firstuse(void *v) NET_UNLOCK(); } +void +tdb_addtimeouts(struct tdb *tdbp) +{ + mtx_enter(&tdbp->tdb_mtx); + if (tdbp->tdb_flags & TDBF_TIMER) { + if (timeout_add_sec(&tdbp->tdb_timer_tmo, + tdbp->tdb_exp_timeout)) + tdb_ref(tdbp); + } + if (tdbp->tdb_flags & TDBF_SOFT_TIMER) { + if (timeout_add_sec(&tdbp->tdb_stimer_tmo, + tdbp->tdb_soft_timeout)) + tdb_ref(tdbp); + } + mtx_leave(&tdbp->tdb_mtx); +} + void tdb_soft_timeout(void *v) { diff --git a/sys/netinet/ip_ipsp.h b/sys/netinet/ip_ipsp.h index c24174d0f90..d5100a55eb8 100644 --- a/sys/netinet/ip_ipsp.h +++ b/sys/netinet/ip_ipsp.h @@ -1,4 +1,4 @@ -/* $OpenBSD: ip_ipsp.h,v 1.242 2023/08/07 01:44:51 dlg Exp $ */ +/* $OpenBSD: ip_ipsp.h,v 1.243 2023/10/11 22:13:16 tobhe Exp $ */ /* * The authors of this code are John Ioannidis (ji@tla.org), * Angelos D. Keromytis (kermit@csd.uch.gr), @@ -613,6 +613,7 @@ void tdb_unlink(struct tdb *); void tdb_unlink_locked(struct tdb *); void tdb_cleanspd(struct tdb *); void tdb_unbundle(struct tdb *); +void tdb_addtimeouts(struct tdb *); void tdb_deltimeouts(struct tdb *); int tdb_walk(u_int, int (*)(struct tdb *, void *, int), void *); void tdb_printit(void *, int, int (*)(const char *, ...)); -- 2.20.1