From c17f87d4e159635b45f282332ed457f7d697e2e3 Mon Sep 17 00:00:00 2001 From: ozaki-r Date: Fri, 2 Mar 2018 07:37:13 +0000 Subject: [PATCH] Avoid data races on lifetime counters by using percpu(9) We don't make them percpu(9) directly because the structure is exposed to userland and we don't want to break ABI. So we add another member variable for percpu(9) and use it internally. When we export them to userland, they are converted to the original format. --- sys/netipsec/key.c | 88 +++++++++++++++++++++++++++++++++++++------- sys/netipsec/keydb.h | 6 ++- 2 files changed, 80 insertions(+), 14 deletions(-) diff --git a/sys/netipsec/key.c b/sys/netipsec/key.c index 889ff37b0f39..0a3f5a9dd1c6 100644 --- a/sys/netipsec/key.c +++ b/sys/netipsec/key.c @@ -1,4 +1,4 @@ -/* $NetBSD: key.c,v 1.248 2018/02/08 20:57:41 maxv Exp $ */ +/* $NetBSD: key.c,v 1.249 2018/03/02 07:37:13 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 -__KERNEL_RCSID(0, "$NetBSD: key.c,v 1.248 2018/02/08 20:57:41 maxv Exp $"); +__KERNEL_RCSID(0, "$NetBSD: key.c,v 1.249 2018/03/02 07:37:13 ozaki-r Exp $"); /* * This code is referred to RFC 2367 @@ -785,6 +785,26 @@ static struct callout key_timehandler_ch; static struct workqueue *key_timehandler_wq; static struct work key_timehandler_wk; +/* + * Utilities for percpu counters for sadb_lifetime_allocations and + * sadb_lifetime_bytes. + */ +#define LIFETIME_COUNTER_ALLOCATIONS 0 +#define LIFETIME_COUNTER_BYTES 1 +#define LIFETIME_COUNTER_SIZE 2 + +typedef uint64_t lifetime_counters_t[LIFETIME_COUNTER_SIZE]; + +static void +key_sum_lifetime_counters(void *p, void *arg, struct cpu_info *ci __unused) +{ + lifetime_counters_t *one = p; + lifetime_counters_t *sum = arg; + + (*sum)[LIFETIME_COUNTER_ALLOCATIONS] += (*one)[LIFETIME_COUNTER_ALLOCATIONS]; + (*sum)[LIFETIME_COUNTER_BYTES] += (*one)[LIFETIME_COUNTER_BYTES]; +} + u_int key_sp_refcnt(const struct secpolicy *sp) { @@ -3257,6 +3277,8 @@ key_newsav(struct mbuf *m, const struct sadb_msghdr *mhp, /* We don't allow lft_c to be NULL */ newsav->lft_c = kmem_zalloc(sizeof(struct sadb_lifetime), KM_SLEEP); + newsav->lft_c_counters_percpu = + percpu_alloc(sizeof(lifetime_counters_t)); } /* reset created */ @@ -3467,6 +3489,10 @@ key_freesaval(struct secasvar *sav) kmem_intr_free(sav->key_auth, sav->key_auth_len); if (sav->key_enc != NULL) kmem_intr_free(sav->key_enc, sav->key_enc_len); + if (sav->lft_c_counters_percpu != NULL) { + percpu_free(sav->lft_c_counters_percpu, + sizeof(lifetime_counters_t)); + } if (sav->lft_c != NULL) kmem_intr_free(sav->lft_c, sizeof(*(sav->lft_c))); if (sav->lft_h != NULL) @@ -3635,6 +3661,8 @@ key_setsaval(struct secasvar *sav, struct mbuf *m, sav->lft_c->sadb_lifetime_addtime = time_uptime; sav->lft_c->sadb_lifetime_usetime = 0; + sav->lft_c_counters_percpu = percpu_alloc(sizeof(lifetime_counters_t)); + /* lifetimes for HARD and SOFT */ { const struct sadb_lifetime *lft0; @@ -3818,7 +3846,9 @@ key_setdumpsa(struct secasvar *sav, u_int8_t type, u_int8_t satype, p = sav->key_enc; break; - case SADB_EXT_LIFETIME_CURRENT: + case SADB_EXT_LIFETIME_CURRENT: { + lifetime_counters_t sum = {0}; + KASSERT(sav->lft_c != NULL); l = PFKEY_UNUNIT64(((struct sadb_ext *)sav->lft_c)->sadb_ext_len); memcpy(<, sav->lft_c, sizeof(struct sadb_lifetime)); @@ -3826,8 +3856,15 @@ key_setdumpsa(struct secasvar *sav, u_int8_t type, u_int8_t satype, time_mono_to_wall(lt.sadb_lifetime_addtime); lt.sadb_lifetime_usetime = time_mono_to_wall(lt.sadb_lifetime_usetime); + percpu_foreach(sav->lft_c_counters_percpu, + key_sum_lifetime_counters, sum); + lt.sadb_lifetime_allocations = + sum[LIFETIME_COUNTER_ALLOCATIONS]; + lt.sadb_lifetime_bytes = + sum[LIFETIME_COUNTER_BYTES]; p = < break; + } case SADB_EXT_LIFETIME_HARD: if (!sav->lft_h) @@ -4857,9 +4894,17 @@ restart: * when new SA is installed. Caution when it's * installed too big lifetime by time. */ - else if (sav->lft_s->sadb_lifetime_bytes != 0 && - sav->lft_s->sadb_lifetime_bytes < - sav->lft_c->sadb_lifetime_bytes) { + else { + uint64_t lft_c_bytes = 0; + lifetime_counters_t sum = {0}; + + percpu_foreach(sav->lft_c_counters_percpu, + key_sum_lifetime_counters, sum); + lft_c_bytes = sum[LIFETIME_COUNTER_BYTES]; + + if (sav->lft_s->sadb_lifetime_bytes == 0 || + sav->lft_s->sadb_lifetime_bytes >= lft_c_bytes) + continue; key_sa_chgstate(sav, SADB_SASTATE_DYING); mutex_exit(&key_sad.lock); @@ -4907,9 +4952,18 @@ restart: } #endif /* check HARD lifetime by bytes */ - else if (sav->lft_h->sadb_lifetime_bytes != 0 && - sav->lft_h->sadb_lifetime_bytes < - sav->lft_c->sadb_lifetime_bytes) { + else { + uint64_t lft_c_bytes = 0; + lifetime_counters_t sum = {0}; + + percpu_foreach(sav->lft_c_counters_percpu, + key_sum_lifetime_counters, sum); + lft_c_bytes = sum[LIFETIME_COUNTER_BYTES]; + + if (sav->lft_h->sadb_lifetime_bytes == 0 || + sav->lft_h->sadb_lifetime_bytes >= lft_c_bytes) + continue; + key_sa_chgstate(sav, SADB_SASTATE_DEAD); goto restart_sav_DYING; } @@ -7178,6 +7232,7 @@ key_expire(struct secasvar *sav) int len; int error = -1; struct sadb_lifetime *lt; + lifetime_counters_t sum = {0}; /* XXX: Why do we lock ? */ s = splsoftnet(); /*called from softclock()*/ @@ -7210,8 +7265,10 @@ key_expire(struct secasvar *sav) lt = mtod(m, struct sadb_lifetime *); lt->sadb_lifetime_len = PFKEY_UNIT64(sizeof(struct sadb_lifetime)); lt->sadb_lifetime_exttype = SADB_EXT_LIFETIME_CURRENT; - lt->sadb_lifetime_allocations = sav->lft_c->sadb_lifetime_allocations; - lt->sadb_lifetime_bytes = sav->lft_c->sadb_lifetime_bytes; + percpu_foreach(sav->lft_c_counters_percpu, + key_sum_lifetime_counters, sum); + lt->sadb_lifetime_allocations = sum[LIFETIME_COUNTER_ALLOCATIONS]; + lt->sadb_lifetime_bytes = sum[LIFETIME_COUNTER_BYTES]; lt->sadb_lifetime_addtime = time_mono_to_wall(sav->lft_c->sadb_lifetime_addtime); lt->sadb_lifetime_usetime = @@ -8171,16 +8228,19 @@ key_getuserfqdn(void) void key_sa_recordxfer(struct secasvar *sav, struct mbuf *m) { + lifetime_counters_t *counters; KASSERT(sav != NULL); KASSERT(sav->lft_c != NULL); KASSERT(m != NULL); + counters = percpu_getref(sav->lft_c_counters_percpu); + /* * XXX Currently, there is a difference of bytes size * between inbound and outbound processing. */ - sav->lft_c->sadb_lifetime_bytes += m->m_pkthdr.len; + (*counters)[LIFETIME_COUNTER_BYTES] += m->m_pkthdr.len; /* to check bytes lifetime is done in key_timehandler(). */ /* @@ -8188,9 +8248,11 @@ key_sa_recordxfer(struct secasvar *sav, struct mbuf *m) * sadb_lifetime_allocations. We increment the variable * whenever {esp,ah}_{in,out}put is called. */ - sav->lft_c->sadb_lifetime_allocations++; + (*counters)[LIFETIME_COUNTER_ALLOCATIONS]++; /* XXX check for expires? */ + percpu_putref(sav->lft_c_counters_percpu); + /* * NOTE: We record CURRENT sadb_lifetime_usetime by using wall clock, * in seconds. HARD and SOFT lifetime are measured by the time diff --git a/sys/netipsec/keydb.h b/sys/netipsec/keydb.h index 0cb0c997b37b..f32348c58b1e 100644 --- a/sys/netipsec/keydb.h +++ b/sys/netipsec/keydb.h @@ -1,4 +1,4 @@ -/* $NetBSD: keydb.h,v 1.20 2017/08/09 09:48:11 ozaki-r Exp $ */ +/* $NetBSD: keydb.h,v 1.21 2018/03/02 07:37:13 ozaki-r Exp $ */ /* $FreeBSD: src/sys/netipsec/keydb.h,v 1.1.4.1 2003/01/24 05:11:36 sam Exp $ */ /* $KAME: keydb.h,v 1.14 2000/08/02 17:58:26 sakane Exp $ */ @@ -37,6 +37,7 @@ #ifdef _KERNEL #include +#include #include #include @@ -118,6 +119,9 @@ struct secasvar { struct sadb_lifetime *lft_h; /* HARD lifetime */ struct sadb_lifetime *lft_s; /* SOFT lifetime */ + /* percpu counters for lft_c->sadb_lifetime_{allocations,bytes} */ + percpu_t *lft_c_counters_percpu; + u_int32_t seq; /* sequence number */ pid_t pid; /* message's pid */