From b7be02dc3945b7da7398b50fcbe334c0c2f5ab24 Mon Sep 17 00:00:00 2001 From: bluhm Date: Fri, 18 Jun 2021 15:34:21 +0000 Subject: [PATCH] The crypto(9) framework used by IPsec runs on a kernel task that is protected by kernel lock. There were crashes in swcr_authenc() when it was accessing swcr_sessions. As a quick fix, protect all calls from network stack to crypto with kernel lock. This also covers the rekeying case that is called from pfkey via tdb_init(). OK mvs@ --- sys/netinet/ip_ah.c | 26 +++++++++++++++++++------- sys/netinet/ip_esp.c | 26 +++++++++++++++++++------- sys/netinet/ip_ipcomp.c | 28 ++++++++++++++++++++-------- sys/netinet/ipsec_input.c | 4 +++- sys/netinet/ipsec_output.c | 4 +++- 5 files changed, 64 insertions(+), 24 deletions(-) diff --git a/sys/netinet/ip_ah.c b/sys/netinet/ip_ah.c index c4a450f49cf..c06dad5b72c 100644 --- a/sys/netinet/ip_ah.c +++ b/sys/netinet/ip_ah.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ip_ah.c,v 1.146 2021/02/25 02:48:21 dlg Exp $ */ +/* $OpenBSD: ip_ah.c,v 1.147 2021/06/18 15:34:21 bluhm Exp $ */ /* * The authors of this code are John Ioannidis (ji@tla.org), * Angelos D. Keromytis (kermit@csd.uch.gr) and @@ -98,6 +98,7 @@ ah_init(struct tdb *tdbp, struct xformsw *xsp, struct ipsecinit *ii) { struct auth_hash *thash = NULL; struct cryptoini cria, crin; + int error; /* Authentication operation. */ switch (ii->ii_authalg) { @@ -162,7 +163,10 @@ ah_init(struct tdb *tdbp, struct xformsw *xsp, struct ipsecinit *ii) cria.cri_next = &crin; } - return crypto_newsession(&tdbp->tdb_cryptoid, &cria, 0); + KERNEL_LOCK(); + error = crypto_newsession(&tdbp->tdb_cryptoid, &cria, 0); + KERNEL_UNLOCK(); + return error; } /* @@ -171,7 +175,7 @@ ah_init(struct tdb *tdbp, struct xformsw *xsp, struct ipsecinit *ii) int ah_zeroize(struct tdb *tdbp) { - int err; + int error; if (tdbp->tdb_amxkey) { explicit_bzero(tdbp->tdb_amxkey, tdbp->tdb_amxkeylen); @@ -179,9 +183,11 @@ ah_zeroize(struct tdb *tdbp) tdbp->tdb_amxkey = NULL; } - err = crypto_freesession(tdbp->tdb_cryptoid); + KERNEL_LOCK(); + error = crypto_freesession(tdbp->tdb_cryptoid); + KERNEL_UNLOCK(); tdbp->tdb_cryptoid = 0; - return err; + return error; } /* @@ -696,7 +702,10 @@ ah_input(struct mbuf *m, struct tdb *tdb, int skip, int protoff) tc->tc_rdomain = tdb->tdb_rdomain; memcpy(&tc->tc_dst, &tdb->tdb_dst, sizeof(union sockaddr_union)); - return crypto_dispatch(crp); + KERNEL_LOCK(); + error = crypto_dispatch(crp); + KERNEL_UNLOCK(); + return error; drop: m_freem(m); @@ -1144,7 +1153,10 @@ ah_output(struct mbuf *m, struct tdb *tdb, struct mbuf **mp, int skip, tc->tc_rdomain = tdb->tdb_rdomain; memcpy(&tc->tc_dst, &tdb->tdb_dst, sizeof(union sockaddr_union)); - return crypto_dispatch(crp); + KERNEL_LOCK(); + error = crypto_dispatch(crp); + KERNEL_UNLOCK(); + return error; drop: m_freem(m); diff --git a/sys/netinet/ip_esp.c b/sys/netinet/ip_esp.c index a74bfd341ac..40c3a0147b1 100644 --- a/sys/netinet/ip_esp.c +++ b/sys/netinet/ip_esp.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ip_esp.c,v 1.162 2021/02/25 02:48:21 dlg Exp $ */ +/* $OpenBSD: ip_esp.c,v 1.163 2021/06/18 15:34:21 bluhm Exp $ */ /* * The authors of this code are John Ioannidis (ji@tla.org), * Angelos D. Keromytis (kermit@csd.uch.gr) and @@ -93,6 +93,7 @@ esp_init(struct tdb *tdbp, struct xformsw *xsp, struct ipsecinit *ii) struct enc_xform *txform = NULL; struct auth_hash *thash = NULL; struct cryptoini cria, crie, crin; + int error; if (!ii->ii_encalg && !ii->ii_authalg) { DPRINTF(("esp_init(): neither authentication nor encryption " @@ -294,8 +295,11 @@ esp_init(struct tdb *tdbp, struct xformsw *xsp, struct ipsecinit *ii) cria.cri_key = ii->ii_authkey; } - return crypto_newsession(&tdbp->tdb_cryptoid, + KERNEL_LOCK(); + error = crypto_newsession(&tdbp->tdb_cryptoid, (tdbp->tdb_encalgxform ? &crie : &cria), 0); + KERNEL_UNLOCK(); + return error; } /* @@ -304,7 +308,7 @@ esp_init(struct tdb *tdbp, struct xformsw *xsp, struct ipsecinit *ii) int esp_zeroize(struct tdb *tdbp) { - int err; + int error; if (tdbp->tdb_amxkey) { explicit_bzero(tdbp->tdb_amxkey, tdbp->tdb_amxkeylen); @@ -318,9 +322,11 @@ esp_zeroize(struct tdb *tdbp) tdbp->tdb_emxkey = NULL; } - err = crypto_freesession(tdbp->tdb_cryptoid); + KERNEL_LOCK(); + error = crypto_freesession(tdbp->tdb_cryptoid); + KERNEL_UNLOCK(); tdbp->tdb_cryptoid = 0; - return err; + return error; } #define MAXBUFSIZ (AH_ALEN_MAX > ESP_MAX_IVS ? AH_ALEN_MAX : ESP_MAX_IVS) @@ -519,7 +525,10 @@ esp_input(struct mbuf *m, struct tdb *tdb, int skip, int protoff) crde->crd_len = m->m_pkthdr.len - (skip + hlen + alen); } - return crypto_dispatch(crp); + KERNEL_LOCK(); + error = crypto_dispatch(crp); + KERNEL_UNLOCK(); + return error; drop: m_freem(m); @@ -1006,7 +1015,10 @@ esp_output(struct mbuf *m, struct tdb *tdb, struct mbuf **mp, int skip, crda->crd_len = m->m_pkthdr.len - (skip + alen); } - return crypto_dispatch(crp); + KERNEL_LOCK(); + error = crypto_dispatch(crp); + KERNEL_UNLOCK(); + return error; drop: m_freem(m); diff --git a/sys/netinet/ip_ipcomp.c b/sys/netinet/ip_ipcomp.c index 37bb95ea4b9..c93c34a4c96 100644 --- a/sys/netinet/ip_ipcomp.c +++ b/sys/netinet/ip_ipcomp.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ip_ipcomp.c,v 1.67 2019/09/30 01:53:05 dlg Exp $ */ +/* $OpenBSD: ip_ipcomp.c,v 1.68 2021/06/18 15:34:21 bluhm Exp $ */ /* * Copyright (c) 2001 Jean-Jacques Bernard-Gundol (jj@wabbitt.org) @@ -79,6 +79,7 @@ ipcomp_init(struct tdb *tdbp, struct xformsw *xsp, struct ipsecinit *ii) { struct comp_algo *tcomp = NULL; struct cryptoini cric; + int error; switch (ii->ii_compalg) { case SADB_X_CALG_DEFLATE: @@ -105,7 +106,10 @@ ipcomp_init(struct tdb *tdbp, struct xformsw *xsp, struct ipsecinit *ii) memset(&cric, 0, sizeof(cric)); cric.cri_alg = tdbp->tdb_compalgxform->type; - return crypto_newsession(&tdbp->tdb_cryptoid, &cric, 0); + KERNEL_LOCK(); + error = crypto_newsession(&tdbp->tdb_cryptoid, &cric, 0); + KERNEL_UNLOCK(); + return error; } /* @@ -114,11 +118,13 @@ ipcomp_init(struct tdb *tdbp, struct xformsw *xsp, struct ipsecinit *ii) int ipcomp_zeroize(struct tdb *tdbp) { - int err; + int error; - err = crypto_freesession(tdbp->tdb_cryptoid); + KERNEL_LOCK(); + error = crypto_freesession(tdbp->tdb_cryptoid); + KERNEL_UNLOCK(); tdbp->tdb_cryptoid = 0; - return err; + return error; } /* @@ -129,7 +135,7 @@ ipcomp_input(struct mbuf *m, struct tdb *tdb, int skip, int protoff) { struct comp_algo *ipcompx = (struct comp_algo *) tdb->tdb_compalgxform; struct tdb_crypto *tc; - int hlen; + int hlen, error; struct cryptodesc *crdc = NULL; struct cryptop *crp; @@ -178,7 +184,10 @@ ipcomp_input(struct mbuf *m, struct tdb *tdb, int skip, int protoff) tc->tc_rdomain = tdb->tdb_rdomain; tc->tc_dst = tdb->tdb_dst; - return crypto_dispatch(crp); + KERNEL_LOCK(); + error = crypto_dispatch(crp); + KERNEL_UNLOCK(); + return error; } int @@ -472,7 +481,10 @@ ipcomp_output(struct mbuf *m, struct tdb *tdb, struct mbuf **mp, int skip, crp->crp_opaque = (caddr_t)tc; crp->crp_sid = tdb->tdb_cryptoid; - return crypto_dispatch(crp); + KERNEL_LOCK(); + error = crypto_dispatch(crp); + KERNEL_UNLOCK(); + return error; drop: m_freem(m); diff --git a/sys/netinet/ipsec_input.c b/sys/netinet/ipsec_input.c index 3b32d5373de..414830c4653 100644 --- a/sys/netinet/ipsec_input.c +++ b/sys/netinet/ipsec_input.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ipsec_input.c,v 1.173 2020/09/01 01:53:34 gnezdo Exp $ */ +/* $OpenBSD: ipsec_input.c,v 1.174 2021/06/18 15:34:21 bluhm Exp $ */ /* * The authors of this code are John Ioannidis (ji@tla.org), * Angelos D. Keromytis (kermit@csd.uch.gr) and @@ -376,6 +376,8 @@ ipsec_input_cb(struct cryptop *crp) struct tdb *tdb = NULL; int clen, error; + KERNEL_ASSERT_LOCKED(); + if (m == NULL) { DPRINTF(("%s: bogus returned buffer from crypto\n", __func__)); ipsecstat_inc(ipsec_crypto); diff --git a/sys/netinet/ipsec_output.c b/sys/netinet/ipsec_output.c index 5e2b5d07e32..f72dba28944 100644 --- a/sys/netinet/ipsec_output.c +++ b/sys/netinet/ipsec_output.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ipsec_output.c,v 1.79 2021/03/10 10:21:49 jsg Exp $ */ +/* $OpenBSD: ipsec_output.c,v 1.80 2021/06/18 15:34:21 bluhm Exp $ */ /* * The author of this code is Angelos D. Keromytis (angelos@cis.upenn.edu) * @@ -391,6 +391,8 @@ ipsec_output_cb(struct cryptop *crp) struct tdb *tdb = NULL; int error, ilen, olen; + KERNEL_ASSERT_LOCKED(); + if (m == NULL) { DPRINTF(("%s: bogus returned buffer from crypto\n", __func__)); ipsecstat_inc(ipsec_crypto); -- 2.20.1