Hi,

I have seen a kernel crash with while forwarding and encrypting
much traffic through OpenBSD IPsec gateways running iked.

kernel: protection fault trap, code=0
Stopped at      aes_ctr_crypt+0x1e:     addb    $0x1,0x2e3(%rdi)

ddb{2}> trace
aes_ctr_crypt(16367ed4be021a53,ffff8000246e1db0) at aes_ctr_crypt+0x1e
swcr_authenc(fffffd8132a21b08) at swcr_authenc+0x5c3
swcr_process(fffffd8132a21b08) at swcr_process+0x1e8
crypto_invoke(fffffd8132a21b08) at crypto_invoke+0xde
taskq_thread(ffff800000200500) at taskq_thread+0x81
end trace frame: 0x0, count: -5

*64926  109760      0      0  7     0x14200                crypto

swcr_authenc() passes swe->sw_kschedule to aes_ctr_crypt(), swe has
been freed and contains deaf006cdeaf4152, which looks like some
sort of poison.  I suspect a use after free.

The swe value comes from the swcr_sessions global pointers.  Its
content looks sane in ddb.  Noone touches it in swcr_authenc().  So
I guess that an other CPU changes the global structures while
swcr_authenc() is working with it.

The crypto thread is protected by kernel lock, both network stack
and pfkey use net lock.  The kernel lock has been recently removed
from pfkey.

I think the required lock for the crypto framework is the kernel
lock.  If crypto_ functions are called, IPsec must grab the kernel
lock.  pfkey accesses crypto only via tdb_ functions, so this diff
also covers that case.

ok?

bluhm

Index: netinet/ip_ah.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_ah.c,v
retrieving revision 1.146
diff -u -p -r1.146 ip_ah.c
--- netinet/ip_ah.c     25 Feb 2021 02:48:21 -0000      1.146
+++ netinet/ip_ah.c     16 Jun 2021 17:29:37 -0000
@@ -98,6 +98,7 @@ ah_init(struct tdb *tdbp, struct xformsw
 {
        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
                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
 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;
 }
 
 /*
@@ -626,7 +632,9 @@ ah_input(struct mbuf *m, struct tdb *tdb
        }
 
        /* Get crypto descriptors. */
+       KERNEL_LOCK();
        crp = crypto_getreq(1);
+       KERNEL_UNLOCK();
        if (crp == NULL) {
                DPRINTF(("%s: failed to acquire crypto descriptors\n",
                    __func__));
@@ -696,11 +704,16 @@ ah_input(struct mbuf *m, struct tdb *tdb
        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);
+       KERNEL_LOCK();
        crypto_freereq(crp);
+       KERNEL_UNLOCK();
        free(tc, M_XDATA, 0);
        return error;
 }
@@ -1047,7 +1060,9 @@ ah_output(struct mbuf *m, struct tdb *td
 #endif
 
        /* Get crypto descriptors. */
+       KERNEL_LOCK();
        crp = crypto_getreq(1);
+       KERNEL_UNLOCK();
        if (crp == NULL) {
                DPRINTF(("%s: failed to acquire crypto descriptors\n",
                    __func__));
@@ -1144,11 +1159,16 @@ ah_output(struct mbuf *m, struct tdb *td
        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);
+       KERNEL_LOCK();
        crypto_freereq(crp);
+       KERNEL_UNLOCK();
        free(tc, M_XDATA, 0);
        return error;
 }
Index: netinet/ip_esp.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_esp.c,v
retrieving revision 1.162
diff -u -p -r1.162 ip_esp.c
--- netinet/ip_esp.c    25 Feb 2021 02:48:21 -0000      1.162
+++ netinet/ip_esp.c    16 Jun 2021 17:29:55 -0000
@@ -93,6 +93,7 @@ esp_init(struct tdb *tdbp, struct xforms
        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 xforms
                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 xforms
 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)
@@ -438,7 +444,9 @@ esp_input(struct mbuf *m, struct tdb *td
        }
 
        /* Get crypto descriptors */
+       KERNEL_LOCK();
        crp = crypto_getreq(esph && espx ? 2 : 1);
+       KERNEL_UNLOCK();
        if (crp == NULL) {
                DPRINTF(("%s: failed to acquire crypto descriptors\n", 
__func__));
                espstat_inc(esps_crypto);
@@ -519,11 +527,16 @@ esp_input(struct mbuf *m, struct tdb *td
                        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);
+       KERNEL_LOCK();
        crypto_freereq(crp);
+       KERNEL_UNLOCK();
        free(tc, M_XDATA, 0);
        return error;
 }
@@ -919,7 +932,9 @@ esp_output(struct mbuf *m, struct tdb *t
        m_copyback(m, protoff, sizeof(u_int8_t), &prot, M_NOWAIT);
 
        /* Get crypto descriptors. */
+       KERNEL_LOCK();
        crp = crypto_getreq(esph && espx ? 2 : 1);
+       KERNEL_UNLOCK();
        if (crp == NULL) {
                DPRINTF(("%s: failed to acquire crypto descriptors\n",
                    __func__));
@@ -1006,11 +1021,16 @@ esp_output(struct mbuf *m, struct tdb *t
                        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);
+       KERNEL_LOCK();
        crypto_freereq(crp);
+       KERNEL_UNLOCK();
        free(tc, M_XDATA, 0);
        return error;
 }
Index: netinet/ip_ipcomp.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_ipcomp.c,v
retrieving revision 1.67
diff -u -p -r1.67 ip_ipcomp.c
--- netinet/ip_ipcomp.c 30 Sep 2019 01:53:05 -0000      1.67
+++ netinet/ip_ipcomp.c 16 Jun 2021 20:02:28 -0000
@@ -79,6 +79,7 @@ ipcomp_init(struct tdb *tdbp, struct xfo
 {
        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 xfo
        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 xfo
 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 
 {
        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;
@@ -137,7 +143,9 @@ ipcomp_input(struct mbuf *m, struct tdb 
        hlen = IPCOMP_HLENGTH;
 
        /* Get crypto descriptors */
+       KERNEL_LOCK();
        crp = crypto_getreq(1);
+       KERNEL_UNLOCK();
        if (crp == NULL) {
                m_freem(m);
                DPRINTF(("%s: failed to acquire crypto descriptors\n", 
__func__));
@@ -148,7 +156,9 @@ ipcomp_input(struct mbuf *m, struct tdb 
        tc = malloc(sizeof(*tc), M_XDATA, M_NOWAIT | M_ZERO);
        if (tc == NULL) {
                m_freem(m);
+               KERNEL_LOCK();
                crypto_freereq(crp);
+               KERNEL_UNLOCK();
                DPRINTF(("%s: failed to allocate tdb_crypto\n", __func__));
                ipcompstat_inc(ipcomps_crypto);
                return ENOBUFS;
@@ -178,7 +188,10 @@ ipcomp_input(struct mbuf *m, struct tdb 
        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
@@ -431,7 +444,9 @@ ipcomp_output(struct mbuf *m, struct tdb
        /* Ok now, we can pass to the crypto processing */
 
        /* Get crypto descriptors */
+       KERNEL_LOCK();
        crp = crypto_getreq(1);
+       KERNEL_UNLOCK();
        if (crp == NULL) {
                DPRINTF(("%s: failed to acquire crypto descriptors\n", 
__func__));
                ipcompstat_inc(ipcomps_crypto);
@@ -472,11 +487,16 @@ ipcomp_output(struct mbuf *m, struct tdb
        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);
+       KERNEL_LOCK();
        crypto_freereq(crp);
+       KERNEL_UNLOCK();
        return error;
 }
 
Index: netinet/ipsec_input.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ipsec_input.c,v
retrieving revision 1.173
diff -u -p -r1.173 ipsec_input.c
--- netinet/ipsec_input.c       1 Sep 2020 01:53:34 -0000       1.173
+++ netinet/ipsec_input.c       16 Jun 2021 12:37:54 -0000
@@ -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);
Index: netinet/ipsec_output.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ipsec_output.c,v
retrieving revision 1.79
diff -u -p -r1.79 ipsec_output.c
--- netinet/ipsec_output.c      10 Mar 2021 10:21:49 -0000      1.79
+++ netinet/ipsec_output.c      16 Jun 2021 12:37:53 -0000
@@ -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);

Reply via email to