Re: crypto kernel lock
On 21/06/21(Mon) 23:45, Alexander Bluhm wrote:
> On Thu, Jun 17, 2021 at 03:19:11PM +0200, Alexander Bluhm wrote:
> > On Thu, Jun 17, 2021 at 10:09:47AM +0200, Martin Pieuchot wrote:
> > > Could you annotate which field is being protected by the KERNEL_LOCK()?
> >
> > No. I do not want to invest into fine grained crypto locking. I
> > need a stable test machine.
>
> Now my machine is stable again, I can do some annotations.
>
> - remove unused variable cryptodesc_pool
> - document global variables in crypto.c
> - assert kernel lock where needed
> - remove dead code from crypto_get_driverid()
> - move crypto_init() prototype into header file
Diff is ok mpi@.
The annotations are not really coherent with the rest of the tree but this
can be improved in tree, especially if we start using a mutex to replace
the various splvm/splx dances and protect the cc_* fields.
>
> ok?
>
> bluhm
>
> Index: crypto/crypto.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/crypto/crypto.c,v
> retrieving revision 1.82
> diff -u -p -r1.82 crypto.c
> --- crypto/crypto.c 30 Mar 2020 17:48:39 - 1.82
> +++ crypto/crypto.c 21 Jun 2021 21:00:19 -
> @@ -27,16 +27,23 @@
>
> #include
>
> -void crypto_init(void);
> -
> -struct cryptocap *crypto_drivers = NULL;
> -int crypto_drivers_num = 0;
> -
> -struct pool cryptop_pool;
> -struct pool cryptodesc_pool;
> +/*
> + * Locks used to protect struct members in this file:
> + * A allocated during driver attach, no hotplug, no detach
> + * I initialized by main()
This doesn't tell if a lock is needed or not. Other data structure use:
"I immutable after creation"
Is it what you meant? That the variables aren't marked as 'const'
because they are initialized once but after that never change?
> + * K modified with kernel lock
Can we use the same wording as other files:
"K kernel lock"
> + */
>
> -struct taskq *crypto_taskq;
> -struct taskq *crypto_taskq_mpsafe;
> +struct cryptocap *crypto_drivers;/* [A] array allocated by driver
Isn't it possible to have a USB driver calling crypto_get_driverid()?
If that's the case what's protecting the structure is the KERNEL_LOCK().
If we want to simplify things and prevent hotpluging then maybe we
should assert for `cold'?
> +[K] driver data and session count */
Until now we've been annotating the fields in the data structure
definitions, you're referring to `cc_sessions', `cc_newsession', etc,
right?
> +int crypto_drivers_num = 0; /* [A] attached drivers array size */
> +
> +struct pool cryptop_pool;/* [I] set of crypto descriptors */
Pool are protected by their own lock, not sure we should start annotating
them.
> +
> +struct taskq *crypto_taskq; /* [I] run crypto_invoke() and callback
> +with kernel lock */
> +struct taskq *crypto_taskq_mpsafe; /* [I] run crypto_invoke()
> +without kernel lock */
>
> /*
> * Create a new session.
> @@ -52,6 +59,8 @@ crypto_newsession(u_int64_t *sid, struct
> if (crypto_drivers == NULL)
> return EINVAL;
>
> + KERNEL_ASSERT_LOCKED();
> +
> s = splvm();
>
> /*
> @@ -186,6 +195,8 @@ crypto_freesession(u_int64_t sid)
> if (hid >= crypto_drivers_num)
> return ENOENT;
>
> + KERNEL_ASSERT_LOCKED();
> +
> s = splvm();
>
> if (crypto_drivers[hid].cc_sessions)
> @@ -215,6 +226,9 @@ crypto_get_driverid(u_int8_t flags)
> {
> struct cryptocap *newdrv;
> int i, s;
> +
> + /* called from attach routines */
> + KERNEL_ASSERT_LOCKED();
>
> s = splvm();
>
> @@ -241,39 +255,33 @@ crypto_get_driverid(u_int8_t flags)
> }
>
> /* Out of entries, allocate some more. */
> - if (i == crypto_drivers_num) {
> - if (crypto_drivers_num >= CRYPTO_DRIVERS_MAX) {
> - splx(s);
> - return -1;
> - }
> -
> - newdrv = mallocarray(crypto_drivers_num,
> - 2 * sizeof(struct cryptocap), M_CRYPTO_DATA, M_NOWAIT);
> - if (newdrv == NULL) {
> - splx(s);
> - return -1;
> - }
> + if (crypto_drivers_num >= CRYPTO_DRIVERS_MAX) {
> + splx(s);
> + return -1;
> + }
>
> - memcpy(newdrv, crypto_drivers,
> - crypto_drivers_num * sizeof(struct cryptocap));
> - bzero([crypto_drivers_num],
> - crypto_drivers_num * sizeof(struct cryptocap));
> + newdrv = mallocarray(crypto_drivers_num,
> + 2 * sizeof(struct cryptocap), M_CRYPTO_DATA, M_NOWAIT);
> + if (newdrv == NULL) {
> + splx(s);
> + return -1;
> + }
>
> -
crypto kernel lock
On Thu, Jun 17, 2021 at 03:19:11PM +0200, Alexander Bluhm wrote:
> On Thu, Jun 17, 2021 at 10:09:47AM +0200, Martin Pieuchot wrote:
> > Could you annotate which field is being protected by the KERNEL_LOCK()?
>
> No. I do not want to invest into fine grained crypto locking. I
> need a stable test machine.
Now my machine is stable again, I can do some annotations.
- remove unused variable cryptodesc_pool
- document global variables in crypto.c
- assert kernel lock where needed
- remove dead code from crypto_get_driverid()
- move crypto_init() prototype into header file
ok?
bluhm
Index: crypto/crypto.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/crypto/crypto.c,v
retrieving revision 1.82
diff -u -p -r1.82 crypto.c
--- crypto/crypto.c 30 Mar 2020 17:48:39 - 1.82
+++ crypto/crypto.c 21 Jun 2021 21:00:19 -
@@ -27,16 +27,23 @@
#include
-void crypto_init(void);
-
-struct cryptocap *crypto_drivers = NULL;
-int crypto_drivers_num = 0;
-
-struct pool cryptop_pool;
-struct pool cryptodesc_pool;
+/*
+ * Locks used to protect struct members in this file:
+ * A allocated during driver attach, no hotplug, no detach
+ * I initialized by main()
+ * K modified with kernel lock
+ */
-struct taskq *crypto_taskq;
-struct taskq *crypto_taskq_mpsafe;
+struct cryptocap *crypto_drivers; /* [A] array allocated by driver
+ [K] driver data and session count */
+int crypto_drivers_num = 0;/* [A] attached drivers array size */
+
+struct pool cryptop_pool; /* [I] set of crypto descriptors */
+
+struct taskq *crypto_taskq;/* [I] run crypto_invoke() and callback
+ with kernel lock */
+struct taskq *crypto_taskq_mpsafe; /* [I] run crypto_invoke()
+ without kernel lock */
/*
* Create a new session.
@@ -52,6 +59,8 @@ crypto_newsession(u_int64_t *sid, struct
if (crypto_drivers == NULL)
return EINVAL;
+ KERNEL_ASSERT_LOCKED();
+
s = splvm();
/*
@@ -186,6 +195,8 @@ crypto_freesession(u_int64_t sid)
if (hid >= crypto_drivers_num)
return ENOENT;
+ KERNEL_ASSERT_LOCKED();
+
s = splvm();
if (crypto_drivers[hid].cc_sessions)
@@ -215,6 +226,9 @@ crypto_get_driverid(u_int8_t flags)
{
struct cryptocap *newdrv;
int i, s;
+
+ /* called from attach routines */
+ KERNEL_ASSERT_LOCKED();
s = splvm();
@@ -241,39 +255,33 @@ crypto_get_driverid(u_int8_t flags)
}
/* Out of entries, allocate some more. */
- if (i == crypto_drivers_num) {
- if (crypto_drivers_num >= CRYPTO_DRIVERS_MAX) {
- splx(s);
- return -1;
- }
-
- newdrv = mallocarray(crypto_drivers_num,
- 2 * sizeof(struct cryptocap), M_CRYPTO_DATA, M_NOWAIT);
- if (newdrv == NULL) {
- splx(s);
- return -1;
- }
+ if (crypto_drivers_num >= CRYPTO_DRIVERS_MAX) {
+ splx(s);
+ return -1;
+ }
- memcpy(newdrv, crypto_drivers,
- crypto_drivers_num * sizeof(struct cryptocap));
- bzero([crypto_drivers_num],
- crypto_drivers_num * sizeof(struct cryptocap));
+ newdrv = mallocarray(crypto_drivers_num,
+ 2 * sizeof(struct cryptocap), M_CRYPTO_DATA, M_NOWAIT);
+ if (newdrv == NULL) {
+ splx(s);
+ return -1;
+ }
- newdrv[i].cc_sessions = 1; /* Mark */
- newdrv[i].cc_flags = flags;
+ memcpy(newdrv, crypto_drivers,
+ crypto_drivers_num * sizeof(struct cryptocap));
+ bzero([crypto_drivers_num],
+ crypto_drivers_num * sizeof(struct cryptocap));
- free(crypto_drivers, M_CRYPTO_DATA,
- crypto_drivers_num * sizeof(struct cryptocap));
+ newdrv[i].cc_sessions = 1; /* Mark */
+ newdrv[i].cc_flags = flags;
- crypto_drivers_num *= 2;
- crypto_drivers = newdrv;
- splx(s);
- return i;
- }
+ free(crypto_drivers, M_CRYPTO_DATA,
+ crypto_drivers_num * sizeof(struct cryptocap));
- /* Shouldn't really get here... */
+ crypto_drivers_num *= 2;
+ crypto_drivers = newdrv;
splx(s);
- return -1;
+ return i;
}
/*
@@ -287,11 +295,13 @@ crypto_register(u_int32_t driverid, int
{
int s, i;
-
if (driverid >= crypto_drivers_num || alg == NULL ||
crypto_drivers == NULL)
return EINVAL;
+ /* called from attach routines */
+ KERNEL_ASSERT_LOCKED();
+
Re: ipsec crypto kernel lock
On Thu, Jun 17, 2021 at 03:19:11PM +0200, Alexander Bluhm wrote:
> On Thu, Jun 17, 2021 at 10:09:47AM +0200, Martin Pieuchot wrote:
> > It's not clear to me which field is the KERNEL_LOCK() protecting. Is it
> > the access to `swcr_sessions'? Is it a reference? If so grabbing/releasing
> > the lock might not be enough to fix the use-after-free.
>
> I am running the fix for nearly a day now. Seems to be stable. I
> guess the crash is a regression from unlocking the pfkey socket.
> Rekeying had the kernel lock before, now it does not. Calling
> crypto operations without kernel lock looks wrong. There is no
> other locking in crypto.
>
> > Could you annotate which field is being protected by the KERNEL_LOCK()?
>
> No. I do not want to invest into fine grained crypto locking. I
> need a stable test machine. If you think my aporoach of throwing
> the kernel lock between the boundary of network stack, pfkey and
> crypto is too naive, I would suggest that we backoout the pfkey
> unlocking.
>
> But my aproach is stable and has less locks than before. I think
> it is an improvement.
We perform crypto(9) access from net stack without kernel lock held, so
pfkey unlocking backout could not be enough and 'tdbp->tdb_xform->xf_*'
routines should be also wrapped by kernel lock. So this diff is welcomed
anyway.
>
> 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 - 1.146
> > > +++ netinet/ip_ah.c 16 Jun 2021 17:29:37 -
> > > @@ -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 =
> > > }
> > >
> > > - return crypto_newsession(>tdb_cryptoid, , 0);
> > > + KERNEL_LOCK();
> > > + error = crypto_newsession(>tdb_cryptoid, , 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_dst, >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_dst, >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 - 1.162
> > > +++ netinet/ip_esp.c 16 Jun 2021 17:29:55 -
> >
Re: ipsec crypto kernel lock
On Thu, Jun 17, 2021 at 10:09:47AM +0200, Martin Pieuchot wrote:
> It's not clear to me which field is the KERNEL_LOCK() protecting. Is it
> the access to `swcr_sessions'? Is it a reference? If so grabbing/releasing
> the lock might not be enough to fix the use-after-free.
I am running the fix for nearly a day now. Seems to be stable. I
guess the crash is a regression from unlocking the pfkey socket.
Rekeying had the kernel lock before, now it does not. Calling
crypto operations without kernel lock looks wrong. There is no
other locking in crypto.
> Could you annotate which field is being protected by the KERNEL_LOCK()?
No. I do not want to invest into fine grained crypto locking. I
need a stable test machine. If you think my aporoach of throwing
the kernel lock between the boundary of network stack, pfkey and
crypto is too naive, I would suggest that we backoout the pfkey
unlocking.
But my aproach is stable and has less locks than before. I think
it is an improvement.
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 - 1.146
> > +++ netinet/ip_ah.c 16 Jun 2021 17:29:37 -
> > @@ -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 =
> > }
> >
> > - return crypto_newsession(>tdb_cryptoid, , 0);
> > + KERNEL_LOCK();
> > + error = crypto_newsession(>tdb_cryptoid, , 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_dst, >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_dst, >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.c25 Feb 2021 02:48:21 - 1.162
> > +++ netinet/ip_esp.c16 Jun 2021 17:29:55 -
> > @@ -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 =
Re: ipsec crypto kernel lock
On 16/06/21(Wed) 22:05, Alexander Bluhm wrote:
> 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,8000246e1db0) at aes_ctr_crypt+0x1e
> swcr_authenc(fd8132a21b08) at swcr_authenc+0x5c3
> swcr_process(fd8132a21b08) at swcr_process+0x1e8
> crypto_invoke(fd8132a21b08) at crypto_invoke+0xde
> taskq_thread(80200500) at taskq_thread+0x81
> end trace frame: 0x0, count: -5
>
> *64926 109760 0 0 7 0x14200crypto
>
> 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.
It's not clear to me which field is the KERNEL_LOCK() protecting. Is it
the access to `swcr_sessions'? Is it a reference? If so grabbing/releasing
the lock might not be enough to fix the use-after-free.
Could you annotate which field is being protected by the KERNEL_LOCK()?
> 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 - 1.146
> +++ netinet/ip_ah.c 16 Jun 2021 17:29:37 -
> @@ -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 =
> }
>
> - return crypto_newsession(>tdb_cryptoid, , 0);
> + KERNEL_LOCK();
> + error = crypto_newsession(>tdb_cryptoid, , 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_dst, >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_dst, >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
>
Re: ipsec crypto kernel lock
ok mvs@
> On 17 Jun 2021, at 01:06, Alexander Bluhm wrote:
>
> On Wed, Jun 16, 2021 at 11:58:48PM +0300, Vitaliy Makkoveev wrote:
>> crypto_getreq() and crypto_freereq() don???t require kernel lock.
>
> Indeed, new diff.
>
> 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 - 1.146
> +++ netinet/ip_ah.c 16 Jun 2021 21:59:38 -
> @@ -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 =
> }
>
> - return crypto_newsession(>tdb_cryptoid, , 0);
> + KERNEL_LOCK();
> + error = crypto_newsession(>tdb_cryptoid, , 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;
> }
>
> /*
> @@ -696,7 +702,10 @@ ah_input(struct mbuf *m, struct tdb *tdb
> tc->tc_rdomain = tdb->tdb_rdomain;
> memcpy(>tc_dst, >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 *td
> tc->tc_rdomain = tdb->tdb_rdomain;
> memcpy(>tc_dst, >tdb_dst, sizeof(union sockaddr_union));
>
> - return crypto_dispatch(crp);
> + KERNEL_LOCK();
> + error = crypto_dispatch(crp);
> + KERNEL_UNLOCK();
> + return error;
>
> drop:
> m_freem(m);
> 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 - 1.162
> +++ netinet/ip_esp.c 16 Jun 2021 22:00:08 -
> @@ -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(>tdb_cryptoid,
> + KERNEL_LOCK();
> + error = crypto_newsession(>tdb_cryptoid,
> (tdbp->tdb_encalgxform ? : ), 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)
> @@ -519,7 +525,10 @@ 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);
> @@ -1006,7 +1015,10 @@ 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);
> 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
> ---
Re: ipsec crypto kernel lock
On Wed, Jun 16, 2021 at 11:58:48PM +0300, Vitaliy Makkoveev wrote:
> crypto_getreq() and crypto_freereq() don???t require kernel lock.
Indeed, new diff.
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 - 1.146
+++ netinet/ip_ah.c 16 Jun 2021 21:59:38 -
@@ -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 =
}
- return crypto_newsession(>tdb_cryptoid, , 0);
+ KERNEL_LOCK();
+ error = crypto_newsession(>tdb_cryptoid, , 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;
}
/*
@@ -696,7 +702,10 @@ ah_input(struct mbuf *m, struct tdb *tdb
tc->tc_rdomain = tdb->tdb_rdomain;
memcpy(>tc_dst, >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 *td
tc->tc_rdomain = tdb->tdb_rdomain;
memcpy(>tc_dst, >tdb_dst, sizeof(union sockaddr_union));
- return crypto_dispatch(crp);
+ KERNEL_LOCK();
+ error = crypto_dispatch(crp);
+ KERNEL_UNLOCK();
+ return error;
drop:
m_freem(m);
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.c25 Feb 2021 02:48:21 - 1.162
+++ netinet/ip_esp.c16 Jun 2021 22:00:08 -
@@ -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(>tdb_cryptoid,
+ KERNEL_LOCK();
+ error = crypto_newsession(>tdb_cryptoid,
(tdbp->tdb_encalgxform ? : ), 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)
@@ -519,7 +525,10 @@ 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);
@@ -1006,7 +1015,10 @@ 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);
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 - 1.67
+++ netinet/ip_ipcomp.c 16 Jun 2021 22:00:45 -
@@ -79,6 +79,7 @@ ipcomp_init(struct tdb *tdbp, struct xfo
{
Re: ipsec crypto kernel lock
Hi,
crypto_getreq() and crypto_freereq() don’t require kernel lock.
> On 16 Jun 2021, at 23:05, Alexander Bluhm wrote:
>
> 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,8000246e1db0) at aes_ctr_crypt+0x1e
> swcr_authenc(fd8132a21b08) at swcr_authenc+0x5c3
> swcr_process(fd8132a21b08) at swcr_process+0x1e8
> crypto_invoke(fd8132a21b08) at crypto_invoke+0xde
> taskq_thread(80200500) at taskq_thread+0x81
> end trace frame: 0x0, count: -5
>
> *64926 109760 0 0 7 0x14200crypto
>
> 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 - 1.146
> +++ netinet/ip_ah.c 16 Jun 2021 17:29:37 -
> @@ -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 =
> }
>
> - return crypto_newsession(>tdb_cryptoid, , 0);
> + KERNEL_LOCK();
> + error = crypto_newsession(>tdb_cryptoid, , 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_dst, >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_dst, >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 - 1.162
> +++
ipsec crypto kernel lock
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,8000246e1db0) at aes_ctr_crypt+0x1e
swcr_authenc(fd8132a21b08) at swcr_authenc+0x5c3
swcr_process(fd8132a21b08) at swcr_process+0x1e8
crypto_invoke(fd8132a21b08) at crypto_invoke+0xde
taskq_thread(80200500) at taskq_thread+0x81
end trace frame: 0x0, count: -5
*64926 109760 0 0 7 0x14200crypto
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 - 1.146
+++ netinet/ip_ah.c 16 Jun 2021 17:29:37 -
@@ -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 =
}
- return crypto_newsession(>tdb_cryptoid, , 0);
+ KERNEL_LOCK();
+ error = crypto_newsession(>tdb_cryptoid, , 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_dst, >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_dst, >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.c25 Feb 2021 02:48:21 - 1.162
+++ netinet/ip_esp.c16 Jun 2021 17:29:55 -
@@ -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
