Module Name: src
Committed By: ozaki-r
Date: Wed Aug 9 04:29:36 UTC 2017
Modified Files:
src/sys/netipsec: key.c
Log Message:
Fix deadlock between key_sendup_mbuf called from key_acquire and
localcount_drain
If we call key_sendup_mbuf from key_acquire that is called on packet
processing, a deadlock can happen like this:
- At key_acquire, a reference to an SP (and an SA) is held
- key_sendup_mbuf will try to take key_so_mtx
- Some other thread may try to localcount_drain to the SP with
holding key_so_mtx in say key_api_spdflush
- In this case localcount_drain never return because key_sendup_mbuf
that has stuck on key_so_mtx never release a reference to the SP
Fix the deadlock by deferring key_sendup_mbuf to the timer
(key_timehandler).
To generate a diff of this commit:
cvs rdiff -u -r1.219 -r1.220 src/sys/netipsec/key.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/netipsec/key.c
diff -u src/sys/netipsec/key.c:1.219 src/sys/netipsec/key.c:1.220
--- src/sys/netipsec/key.c:1.219 Wed Aug 9 03:41:11 2017
+++ src/sys/netipsec/key.c Wed Aug 9 04:29:36 2017
@@ -1,4 +1,4 @@
-/* $NetBSD: key.c,v 1.219 2017/08/09 03:41:11 ozaki-r Exp $ */
+/* $NetBSD: key.c,v 1.220 2017/08/09 04:29:36 ozaki-r Exp $ */
/* $FreeBSD: src/sys/netipsec/key.c,v 1.3.2.3 2004/02/14 22:23:23 bms Exp $ */
/* $KAME: key.c,v 1.191 2001/06/27 10:46:49 sakane Exp $ */
@@ -32,7 +32,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: key.c,v 1.219 2017/08/09 03:41:11 ozaki-r Exp $");
+__KERNEL_RCSID(0, "$NetBSD: key.c,v 1.220 2017/08/09 04:29:36 ozaki-r Exp $");
/*
* This code is referd to RFC 2367
@@ -735,6 +735,8 @@ static struct mbuf *key_getcomb_ipcomp (
static struct mbuf *key_getprop (const struct secasindex *);
static int key_acquire (const struct secasindex *, struct secpolicy *);
+static void key_acquire_sendup_mbuf_later(struct mbuf *);
+static void key_acquire_sendup_pending_mbuf(void);
#ifndef IPSEC_NONBLOCK_ACQUIRE
static struct secacq *key_newacq (const struct secasindex *);
static struct secacq *key_getacq (const struct secasindex *);
@@ -4867,6 +4869,8 @@ key_timehandler_spacq(time_t now)
#endif
}
+static unsigned int key_timehandler_work_enqueued = 0;
+
/*
* time handler.
* scanning SPD and SAD to check status for each entries,
@@ -4878,6 +4882,9 @@ key_timehandler_work(struct work *wk, vo
time_t now = time_uptime;
IPSEC_DECLARE_LOCK_VARIABLE;
+ /* We can allow enqueuing another work at this point */
+ atomic_swap_uint(&key_timehandler_work_enqueued, 0);
+
IPSEC_ACQUIRE_GLOBAL_LOCKS();
key_timehandler_spd(now);
@@ -4885,6 +4892,8 @@ key_timehandler_work(struct work *wk, vo
key_timehandler_acq(now);
key_timehandler_spacq(now);
+ key_acquire_sendup_pending_mbuf();
+
/* do exchange to tick time !! */
callout_reset(&key_timehandler_ch, hz, key_timehandler, NULL);
@@ -4896,6 +4905,10 @@ static void
key_timehandler(void *arg)
{
+ /* Avoid enqueuing another work when one is already enqueued */
+ if (atomic_swap_uint(&key_timehandler_work_enqueued, 1) == 1)
+ return;
+
workqueue_enqueue(key_timehandler_wq, &key_timehandler_wk, NULL);
}
@@ -6631,7 +6644,20 @@ key_acquire(const struct secasindex *sai
mtod(result, struct sadb_msg *)->sadb_msg_len =
PFKEY_UNIT64(result->m_pkthdr.len);
- return key_sendup_mbuf(NULL, result, KEY_SENDUP_REGISTERED);
+ /*
+ * XXX we cannot call key_sendup_mbuf directly here because
+ * it can cause a deadlock:
+ * - We have a reference to an SP (and an SA) here
+ * - key_sendup_mbuf will try to take key_so_mtx
+ * - Some other thread may try to localcount_drain to the SP with
+ * holding key_so_mtx in say key_api_spdflush
+ * - In this case localcount_drain never return because key_sendup_mbuf
+ * that has stuck on key_so_mtx never release a reference to the SP
+ *
+ * So defer key_sendup_mbuf to the timer.
+ */
+ key_acquire_sendup_mbuf_later(result);
+ return 0;
fail:
if (result)
@@ -6639,6 +6665,57 @@ key_acquire(const struct secasindex *sai
return error;
}
+static struct mbuf *key_acquire_mbuf_head = NULL;
+
+static void
+key_acquire_sendup_pending_mbuf(void)
+{
+ struct mbuf *m, *prev = NULL;
+ int error;
+
+again:
+ mutex_enter(&key_misc.lock);
+ m = key_acquire_mbuf_head;
+ /* Get an earliest mbuf (one at the tail of the list) */
+ while (m != NULL) {
+ if (m->m_nextpkt == NULL) {
+ if (prev != NULL)
+ prev->m_nextpkt = NULL;
+ if (m == key_acquire_mbuf_head)
+ key_acquire_mbuf_head = NULL;
+ break;
+ }
+ prev = m;
+ m = m->m_nextpkt;
+ }
+ mutex_exit(&key_misc.lock);
+
+ if (m == NULL)
+ return;
+
+ error = key_sendup_mbuf(NULL, m, KEY_SENDUP_REGISTERED);
+ if (error != 0)
+ IPSECLOG(LOG_WARNING, "key_sendup_mbuf failed (error=%d)\n",
+ error);
+
+ if (prev != NULL)
+ goto again;
+}
+
+static void
+key_acquire_sendup_mbuf_later(struct mbuf *m)
+{
+
+ mutex_enter(&key_misc.lock);
+ /* Enqueue mbuf at the head of the list */
+ m->m_nextpkt = key_acquire_mbuf_head;
+ key_acquire_mbuf_head = m;
+ mutex_exit(&key_misc.lock);
+
+ /* Kick the timer */
+ key_timehandler(NULL);
+}
+
#ifndef IPSEC_NONBLOCK_ACQUIRE
static struct secacq *
key_newacq(const struct secasindex *saidx)
