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@

1 files changed, 8 insertions, 1 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);
 		}