Source-Changes-HG archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
[src/trunk]: src/sys Make IPsec SPD MP-safe
details: https://anonhg.NetBSD.org/src/rev/033d7f09ec58
branches: trunk
changeset: 355543:033d7f09ec58
user: ozaki-r <ozaki-r%NetBSD.org@localhost>
date: Wed Aug 02 01:28:02 2017 +0000
description:
Make IPsec SPD MP-safe
We use localcount(9), not psref(9), to make the sptree and secpolicy (SP)
entries MP-safe because SPs need to be referenced over opencrypto
processing that executes a callback in a different context.
SPs on sockets aren't managed by the sptree and can be destroyed in softint.
localcount_drain cannot be used in softint so we delay the destruction of
such SPs to a thread context. To do so, a list to manage such SPs is added
(key_socksplist) and key_timehandler_spd deletes dead SPs in the list.
For more details please read the locking notes in key.c.
Proposed on tech-kern@ and tech-net@
diffstat:
sys/netinet6/ip6_forward.c | 6 +-
sys/netinet6/ip6_output.c | 6 +-
sys/netipsec/ipsec.c | 133 ++++++---
sys/netipsec/ipsec.h | 5 +-
sys/netipsec/key.c | 462 ++++++++++++++++++++++-------------
sys/netipsec/key.h | 12 +-
sys/netipsec/xform_ah.c | 25 +-
sys/netipsec/xform_esp.c | 25 +-
sys/netipsec/xform_ipcomp.c | 25 +-
sys/rump/librump/rumpnet/net_stub.c | 6 +-
10 files changed, 458 insertions(+), 247 deletions(-)
diffs (truncated from 1603 to 300 lines):
diff -r ed760e74d7b7 -r 033d7f09ec58 sys/netinet6/ip6_forward.c
--- a/sys/netinet6/ip6_forward.c Wed Aug 02 00:58:18 2017 +0000
+++ b/sys/netinet6/ip6_forward.c Wed Aug 02 01:28:02 2017 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: ip6_forward.c,v 1.87 2017/05/09 04:24:10 ozaki-r Exp $ */
+/* $NetBSD: ip6_forward.c,v 1.88 2017/08/02 01:28:03 ozaki-r Exp $ */
/* $KAME: ip6_forward.c,v 1.109 2002/09/11 08:10:17 sakane Exp $ */
/*
@@ -31,7 +31,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ip6_forward.c,v 1.87 2017/05/09 04:24:10 ozaki-r Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ip6_forward.c,v 1.88 2017/08/02 01:28:03 ozaki-r Exp $");
#ifdef _KERNEL_OPT
#include "opt_gateway.h"
@@ -462,7 +462,7 @@
out:
#ifdef IPSEC
if (sp != NULL)
- KEY_FREESP(&sp);
+ KEY_SP_UNREF(&sp);
#endif
rtcache_unref(rt, ro);
if (ro != NULL)
diff -r ed760e74d7b7 -r 033d7f09ec58 sys/netinet6/ip6_output.c
--- a/sys/netinet6/ip6_output.c Wed Aug 02 00:58:18 2017 +0000
+++ b/sys/netinet6/ip6_output.c Wed Aug 02 01:28:02 2017 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: ip6_output.c,v 1.192 2017/06/26 08:01:53 ozaki-r Exp $ */
+/* $NetBSD: ip6_output.c,v 1.193 2017/08/02 01:28:03 ozaki-r Exp $ */
/* $KAME: ip6_output.c,v 1.172 2001/03/25 09:55:56 itojun Exp $ */
/*
@@ -62,7 +62,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ip6_output.c,v 1.192 2017/06/26 08:01:53 ozaki-r Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ip6_output.c,v 1.193 2017/08/02 01:28:03 ozaki-r Exp $");
#ifdef _KERNEL_OPT
#include "opt_inet.h"
@@ -1069,7 +1069,7 @@
#ifdef IPSEC
if (sp != NULL)
- KEY_FREESP(&sp);
+ KEY_SP_UNREF(&sp);
#endif /* IPSEC */
if_put(ifp, &psref);
diff -r ed760e74d7b7 -r 033d7f09ec58 sys/netipsec/ipsec.c
--- a/sys/netipsec/ipsec.c Wed Aug 02 00:58:18 2017 +0000
+++ b/sys/netipsec/ipsec.c Wed Aug 02 01:28:02 2017 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: ipsec.c,v 1.112 2017/07/26 07:39:54 ozaki-r Exp $ */
+/* $NetBSD: ipsec.c,v 1.113 2017/08/02 01:28:03 ozaki-r Exp $ */
/* $FreeBSD: /usr/local/www/cvsroot/FreeBSD/src/sys/netipsec/ipsec.c,v 1.2.2.2 2003/07/01 01:38:13 sam Exp $ */
/* $KAME: ipsec.c,v 1.103 2001/05/24 07:14:18 sakane Exp $ */
@@ -32,7 +32,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ipsec.c,v 1.112 2017/07/26 07:39:54 ozaki-r Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ipsec.c,v 1.113 2017/08/02 01:28:03 ozaki-r Exp $");
/*
* IPsec controller part.
@@ -59,6 +59,7 @@
#include <sys/kauth.h>
#include <sys/cpu.h>
#include <sys/kmem.h>
+#include <sys/pserialize.h>
#include <net/if.h>
#include <net/route.h>
@@ -201,6 +202,7 @@
static int ipsec_set_policy (struct secpolicy **, int, const void *, size_t,
kauth_cred_t);
static int ipsec_get_policy (struct secpolicy *, struct mbuf **);
+static void ipsec_destroy_policy(struct secpolicy *);
static void vshiftl (unsigned char *, int, int);
static size_t ipsec_hdrsiz (const struct secpolicy *);
@@ -211,34 +213,49 @@
ipsec_checkpcbcache(struct mbuf *m, struct inpcbpolicy *pcbsp, int dir)
{
struct secpolicyindex spidx;
+ struct secpolicy *sp = NULL;
+ int s;
KASSERT(IPSEC_DIR_IS_VALID(dir));
KASSERT(pcbsp != NULL);
KASSERT(dir < __arraycount(pcbsp->sp_cache));
KASSERT(inph_locked(pcbsp->sp_inph));
+ /*
+ * Checking the generation and sp->state and taking a reference to an SP
+ * must be in a critical section of pserialize. See key_unlink_sp.
+ */
+ s = pserialize_read_enter();
/* SPD table change invalidate all the caches. */
if (ipsec_spdgen != pcbsp->sp_cache[dir].cachegen) {
ipsec_invalpcbcache(pcbsp, dir);
- return NULL;
+ goto out;
}
- if (!pcbsp->sp_cache[dir].cachesp)
- return NULL;
- if (pcbsp->sp_cache[dir].cachesp->state != IPSEC_SPSTATE_ALIVE) {
+ sp = pcbsp->sp_cache[dir].cachesp;
+ if (sp == NULL)
+ goto out;
+ if (sp->state != IPSEC_SPSTATE_ALIVE) {
+ sp = NULL;
ipsec_invalpcbcache(pcbsp, dir);
- return NULL;
+ goto out;
}
if ((pcbsp->sp_cacheflags & IPSEC_PCBSP_CONNECTED) == 0) {
- if (ipsec_setspidx(m, &spidx, 1) != 0)
- return NULL;
+ /* NB: assume ipsec_setspidx never sleep */
+ if (ipsec_setspidx(m, &spidx, 1) != 0) {
+ sp = NULL;
+ goto out;
+ }
/*
* We have to make an exact match here since the cached rule
* might have lower priority than a rule that would otherwise
* have matched the packet.
*/
- if (memcmp(&pcbsp->sp_cache[dir].cacheidx, &spidx, sizeof(spidx)))
- return NULL;
+ if (memcmp(&pcbsp->sp_cache[dir].cacheidx, &spidx,
+ sizeof(spidx))) {
+ sp = NULL;
+ goto out;
+ }
} else {
/*
* The pcb is connected, and the L4 code is sure that:
@@ -252,13 +269,14 @@
*/
}
- pcbsp->sp_cache[dir].cachesp->lastused = time_second;
- KEY_SP_REF(pcbsp->sp_cache[dir].cachesp);
+ sp->lastused = time_second;
+ KEY_SP_REF(sp);
KEYDEBUG_PRINTF(KEYDEBUG_IPSEC_STAMP,
"DP cause refcnt++:%d SP:%p\n",
- key_sp_refcnt(pcbsp->sp_cache[dir].cachesp),
- pcbsp->sp_cache[dir].cachesp);
- return pcbsp->sp_cache[dir].cachesp;
+ key_sp_refcnt(sp), pcbsp->sp_cache[dir].cachesp);
+out:
+ pserialize_read_exit(s);
+ return sp;
}
static int
@@ -270,8 +288,6 @@
KASSERT(dir < __arraycount(pcbsp->sp_cache));
KASSERT(inph_locked(pcbsp->sp_inph));
- if (pcbsp->sp_cache[dir].cachesp)
- KEY_FREESP(&pcbsp->sp_cache[dir].cachesp);
pcbsp->sp_cache[dir].cachesp = NULL;
pcbsp->sp_cache[dir].cachehint = IPSEC_PCBHINT_UNKNOWN;
if (ipsec_setspidx(m, &pcbsp->sp_cache[dir].cacheidx, 1) != 0) {
@@ -279,7 +295,6 @@
}
pcbsp->sp_cache[dir].cachesp = sp;
if (pcbsp->sp_cache[dir].cachesp) {
- KEY_SP_REF(pcbsp->sp_cache[dir].cachesp);
KEYDEBUG_PRINTF(KEYDEBUG_IPSEC_STAMP,
"DP cause refcnt++:%d SP:%p\n",
key_sp_refcnt(pcbsp->sp_cache[dir].cachesp),
@@ -317,8 +332,6 @@
for (i = IPSEC_DIR_INBOUND; i <= IPSEC_DIR_OUTBOUND; i++) {
if (dir != IPSEC_DIR_ANY && i != dir)
continue;
- if (pcbsp->sp_cache[i].cachesp)
- KEY_FREESP(&pcbsp->sp_cache[i].cachesp);
pcbsp->sp_cache[i].cachesp = NULL;
pcbsp->sp_cache[i].cachehint = IPSEC_PCBHINT_UNKNOWN;
pcbsp->sp_cache[i].cachegen = 0;
@@ -609,7 +622,7 @@
break;
case IPSEC_POLICY_BYPASS:
case IPSEC_POLICY_NONE:
- KEY_FREESP(&sp);
+ KEY_SP_UNREF(&sp);
sp = NULL; /* NB: force NULL result */
break;
case IPSEC_POLICY_IPSEC:
@@ -617,7 +630,7 @@
break;
}
if (*error != 0) {
- KEY_FREESP(&sp);
+ KEY_SP_UNREF(&sp);
sp = NULL;
IPSECLOG(LOG_DEBUG, "done, error %d\n", *error);
}
@@ -697,7 +710,7 @@
*/
*mtu = _mtu;
*natt_frag = true;
- KEY_FREESP(&sp);
+ KEY_SP_UNREF(&sp);
splx(s);
return 0;
}
@@ -711,7 +724,7 @@
*/
if (error == ENOENT)
error = 0;
- KEY_FREESP(&sp);
+ KEY_SP_UNREF(&sp);
splx(s);
*done = true;
return error;
@@ -734,7 +747,7 @@
* Check security policy against packet attributes.
*/
error = ipsec_in_reject(sp, m);
- KEY_FREESP(&sp);
+ KEY_SP_UNREF(&sp);
splx(s);
if (error) {
return error;
@@ -753,7 +766,7 @@
sp = ipsec4_checkpolicy(m, IPSEC_DIR_OUTBOUND, flags, &error, NULL);
if (sp != NULL) {
m->m_flags &= ~M_CANFASTFWD;
- KEY_FREESP(&sp);
+ KEY_SP_UNREF(&sp);
}
splx(s);
return 0;
@@ -802,7 +815,7 @@
rtcache_unref(rt, ro);
KEY_FREESAV(&sav);
}
- KEY_FREESP(&sp);
+ KEY_SP_UNREF(&sp);
return 0;
}
@@ -838,7 +851,7 @@
break;
case IPSEC_POLICY_BYPASS:
case IPSEC_POLICY_NONE:
- KEY_FREESP(&sp);
+ KEY_SP_UNREF(&sp);
sp = NULL; /* NB: force NULL result */
break;
case IPSEC_POLICY_IPSEC:
@@ -846,7 +859,7 @@
break;
}
if (*error != 0) {
- KEY_FREESP(&sp);
+ KEY_SP_UNREF(&sp);
sp = NULL;
IPSECLOG(LOG_DEBUG, "done, error %d\n", *error);
}
@@ -1236,20 +1249,26 @@
else
new->priv = 0;
+ /*
+ * These SPs are dummy. Never be used because the policy
+ * is ENTRUST. See ipsec_getpolicybysock.
+ */
if ((new->sp_in = KEY_NEWSP()) == NULL) {
ipsec_delpcbpolicy(new);
return ENOBUFS;
}
new->sp_in->state = IPSEC_SPSTATE_ALIVE;
new->sp_in->policy = IPSEC_POLICY_ENTRUST;
+ new->sp_in->created = 0; /* Indicates dummy */
if ((new->sp_out = KEY_NEWSP()) == NULL) {
- KEY_FREESP(&new->sp_in);
+ KEY_SP_UNREF(&new->sp_in);
ipsec_delpcbpolicy(new);
return ENOBUFS;
}
new->sp_out->state = IPSEC_SPSTATE_ALIVE;
new->sp_out->policy = IPSEC_POLICY_ENTRUST;
+ new->sp_out->created = 0; /* Indicates dummy */
Home |
Main Index |
Thread Index |
Old Index