Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/sys/netipsec Prepare to stop using isr->sav



details:   https://anonhg.NetBSD.org/src/rev/3ee8d08e3d1b
branches:  trunk
changeset: 825434:3ee8d08e3d1b
user:      ozaki-r <ozaki-r%NetBSD.org@localhost>
date:      Fri Jul 14 12:26:26 2017 +0000

description:
Prepare to stop using isr->sav

isr is a shared resource and using isr->sav as a temporal storage
for each packet processing is racy. And also having a reference from
isr to sav makes the lifetime of sav non-deterministic; such a reference
is removed when a packet is processed and isr->sav is overwritten by
new one. Let's have a sav locally for each packet processing instead of
using shared isr->sav.

However this change doesn't stop using isr->sav yet because there are
some users of isr->sav. isr->sav will be removed after the users find
a way to not use isr->sav.

diffstat:

 sys/netipsec/ipsec.h        |   4 +-
 sys/netipsec/ipsec_output.c |  62 +++++++++++++++++++++++++-------------------
 sys/netipsec/key.c          |  12 +++++--
 sys/netipsec/key.h          |   4 +-
 sys/netipsec/xform.h        |   7 ++--
 sys/netipsec/xform_ah.c     |  10 ++----
 sys/netipsec/xform_esp.c    |  12 ++-----
 sys/netipsec/xform_ipcomp.c |  14 ++++-----
 sys/netipsec/xform_ipip.c   |  10 ++----
 sys/netipsec/xform_tcp.c    |   6 ++--
 10 files changed, 72 insertions(+), 69 deletions(-)

diffs (truncated from 634 to 300 lines):

diff -r 5e2f78517eeb -r 3ee8d08e3d1b sys/netipsec/ipsec.h
--- a/sys/netipsec/ipsec.h      Fri Jul 14 11:54:52 2017 +0000
+++ b/sys/netipsec/ipsec.h      Fri Jul 14 12:26:26 2017 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: ipsec.h,v 1.51 2017/07/05 03:44:59 ozaki-r Exp $       */
+/*     $NetBSD: ipsec.h,v 1.52 2017/07/14 12:26:26 ozaki-r Exp $       */
 /*     $FreeBSD: /usr/local/www/cvsroot/FreeBSD/src/sys/netipsec/ipsec.h,v 1.2.4.2 2004/02/14 22:23:23 bms Exp $       */
 /*     $KAME: ipsec.h,v 1.53 2001/11/20 08:32:38 itojun Exp $  */
 
@@ -341,7 +341,7 @@
 int ipsec4_common_input_cb(struct mbuf *, struct secasvar *,
                        int, int);
 int ipsec4_process_packet(struct mbuf *, struct ipsecrequest *);
-int ipsec_process_done (struct mbuf *, struct ipsecrequest *);
+int ipsec_process_done(struct mbuf *, struct ipsecrequest *, struct secasvar *);
 #define ipsec_indone(m)        \
        (m_tag_find((m), PACKET_TAG_IPSEC_IN_DONE, NULL) != NULL)
 
diff -r 5e2f78517eeb -r 3ee8d08e3d1b sys/netipsec/ipsec_output.c
--- a/sys/netipsec/ipsec_output.c       Fri Jul 14 11:54:52 2017 +0000
+++ b/sys/netipsec/ipsec_output.c       Fri Jul 14 12:26:26 2017 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: ipsec_output.c,v 1.53 2017/07/13 01:48:52 ozaki-r Exp $        */
+/*     $NetBSD: ipsec_output.c,v 1.54 2017/07/14 12:26:26 ozaki-r Exp $        */
 
 /*-
  * Copyright (c) 2002, 2003 Sam Leffler, Errno Consulting
@@ -29,7 +29,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ipsec_output.c,v 1.53 2017/07/13 01:48:52 ozaki-r Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ipsec_output.c,v 1.54 2017/07/14 12:26:26 ozaki-r Exp $");
 
 /*
  * IPsec output processing.
@@ -142,9 +142,9 @@
 }
 
 int
-ipsec_process_done(struct mbuf *m, struct ipsecrequest *isr)
+ipsec_process_done(struct mbuf *m, struct ipsecrequest *isr,
+    struct secasvar *sav)
 {
-       struct secasvar *sav;
        struct secasindex *saidx;
        int error;
 #ifdef INET
@@ -162,7 +162,6 @@
 
        KASSERT(m != NULL);
        KASSERT(isr != NULL);
-       sav = isr->sav;
        KASSERT(sav != NULL);
 
        saidx = &sav->sah->saidx;
@@ -293,7 +292,8 @@
        struct mbuf *m,
        struct ipsecrequest *isr,
        int af,
-       int *error
+       int *error,
+       struct secasvar **ret
 )
 {
 #define        IPSEC_OSTAT(type)                                               \
@@ -311,7 +311,7 @@
        }                                                               \
 } while (/*CONSTCOND*/0)
 
-       struct secasvar *sav;
+       struct secasvar *sav = NULL;
        struct secasindex *saidx;
 
        IPSEC_SPLASSERT_SOFTNET("ipsec_nextisr");
@@ -380,7 +380,7 @@
        /*
         * Lookup SA and validate it.
         */
-       *error = key_checkrequest(isr);
+       *error = key_checkrequest(isr, &sav);
        if (*error != 0) {
                /*
                 * IPsec processing is required, but no SA found.
@@ -392,7 +392,6 @@
                IPSEC_STATINC(IPSEC_STAT_OUT_NOSA);
                goto bad;
        }
-       sav = isr->sav;
        /* sav may be NULL here if we have an USE rule */
        if (sav == NULL) {              
                KASSERTMSG(ipsec_get_reqlevel(isr) == IPSEC_LEVEL_USE,
@@ -404,6 +403,7 @@
                 * It can happen when the last rules are USE rules
                 * */
                if (isr == NULL) {
+                       *ret = NULL;
                        *error = 0;             
                        return isr;
                }
@@ -420,6 +420,7 @@
                    " to policy (check your sysctls)\n");
                IPSEC_OSTAT(PDROPS);
                *error = EHOSTUNREACH;
+               KEY_FREESAV(&sav);
                goto bad;
        }
 
@@ -428,6 +429,7 @@
         * before they invoke the xform output method.
         */
        KASSERT(sav->tdb_xform != NULL);
+       *ret = sav;
        return isr;
 bad:
        KASSERTMSG(*error != 0, "error return w/ no error code");
@@ -442,7 +444,7 @@
 int
 ipsec4_process_packet(struct mbuf *m, struct ipsecrequest *isr)
 {
-       struct secasvar *sav;
+       struct secasvar *sav = NULL;
        struct ip *ip;
        int s, error, i, off;
        union sockaddr_union *dst;
@@ -453,7 +455,7 @@
 
        s = splsoftnet();                       /* insure SA contents don't change */
 
-       isr = ipsec_nextisr(m, isr, AF_INET, &error);
+       isr = ipsec_nextisr(m, isr, AF_INET, &error, &sav);
        if (isr == NULL) {
                if (error != 0) {
                        goto bad;
@@ -466,7 +468,7 @@
                }
        }
 
-       sav = isr->sav;
+       KASSERT(sav != NULL);
        dst = &sav->sah->saidx.dst;
 
        /*
@@ -476,7 +478,7 @@
                if (m->m_len < sizeof (struct ip) &&
                    (m = m_pullup(m, sizeof (struct ip))) == NULL) {
                        error = ENOBUFS;
-                       goto bad;
+                       goto unrefsav;
                }
                ip = mtod(m, struct ip *);
                /* Honor system-wide control of how to handle IP_DF */
@@ -511,7 +513,7 @@
                if (m->m_len < sizeof (struct ip) &&
                    (m = m_pullup(m, sizeof (struct ip))) == NULL) {
                        error = ENOBUFS;
-                       goto bad;
+                       goto unrefsav;
                }
                ip = mtod(m, struct ip *);
                ip->ip_len = htons(m->m_pkthdr.len);
@@ -519,7 +521,7 @@
                ip->ip_sum = in_cksum(m, ip->ip_hl << 2);
 
                /* Encapsulate the packet */
-               error = ipip_output(m, isr, &mp, 0, 0);
+               error = ipip_output(m, isr, sav, &mp, 0, 0);
                if (mp == NULL && !error) {
                        /* Should never happen. */
                        IPSECLOG(LOG_DEBUG,
@@ -532,7 +534,7 @@
                                m_freem(mp);
                        }
                        m = NULL; /* ipip_output() already freed it */
-                       goto bad;
+                       goto unrefsav;
                }
                m = mp, mp = NULL;
                /*
@@ -546,7 +548,7 @@
                        if (m->m_len < sizeof (struct ip) &&
                            (m = m_pullup(m, sizeof (struct ip))) == NULL) {
                                error = ENOBUFS;
-                               goto bad;
+                               goto unrefsav;
                        }
                        ip = mtod(m, struct ip *);
                        ip->ip_off |= htons(IP_DF);
@@ -572,12 +574,15 @@
                        i = sizeof(struct ip6_hdr);
                        off = offsetof(struct ip6_hdr, ip6_nxt);
                }
-               error = (*sav->tdb_xform->xf_output)(m, isr, NULL, i, off);
+               error = (*sav->tdb_xform->xf_output)(m, isr, sav, NULL, i, off);
        } else {
-               error = ipsec_process_done(m, isr);
+               error = ipsec_process_done(m, isr, sav);
        }
+       KEY_FREESAV(&sav);
        splx(s);
        return error;
+unrefsav:
+       KEY_FREESAV(&sav);
 bad:
        splx(s);
        if (m)
@@ -673,7 +678,7 @@
        struct ipsecrequest *isr
     )
 {
-       struct secasvar *sav;
+       struct secasvar *sav = NULL;
        struct ip6_hdr *ip6;
        int s, error, i, off;
        union sockaddr_union *dst;
@@ -683,7 +688,7 @@
 
        s = splsoftnet();   /* insure SA contents don't change */
 
-       isr = ipsec_nextisr(m, isr, AF_INET6, &error);
+       isr = ipsec_nextisr(m, isr, AF_INET6, &error, &sav);
        if (isr == NULL) {
                if (error != 0) {
                        /* XXX Should we send a notification ? */
@@ -697,7 +702,7 @@
                }
        }
 
-       sav = isr->sav;
+       KASSERT(sav != NULL);
        dst = &sav->sah->saidx.dst;
 
        ip6 = mtod(m, struct ip6_hdr *); /* XXX */
@@ -715,21 +720,21 @@
                if (m->m_len < sizeof(struct ip6_hdr)) {
                        if ((m = m_pullup(m,sizeof(struct ip6_hdr))) == NULL) {
                                error = ENOBUFS;
-                               goto bad;
+                               goto unrefsav;
                        }
                }
 
                if (m->m_pkthdr.len - sizeof(*ip6) > IPV6_MAXPACKET) {
                        /* No jumbogram support. */
                        error = ENXIO;   /*XXX*/
-                       goto bad;
+                       goto unrefsav;
                }
 
                ip6 = mtod(m, struct ip6_hdr *);
                ip6->ip6_plen = htons(m->m_pkthdr.len - sizeof(*ip6));
 
                /* Encapsulate the packet */
-               error = ipip_output(m, isr, &mp, 0, 0);
+               error = ipip_output(m, isr, sav, &mp, 0, 0);
                if (mp == NULL && !error) {
                        /* Should never happen. */
                        IPSECLOG(LOG_DEBUG,
@@ -743,7 +748,7 @@
                                m_freem(mp);
                        }
                        m = NULL; /* ipip_output() already freed it */
-                       goto bad;
+                       goto unrefsav;
                }
 
                m = mp;
@@ -758,9 +763,12 @@
        } else {        
                compute_ipsec_pos(m, &i, &off);
        }
-       error = (*sav->tdb_xform->xf_output)(m, isr, NULL, i, off);
+       error = (*sav->tdb_xform->xf_output)(m, isr, sav, NULL, i, off);
+       KEY_FREESAV(&sav);
        splx(s);
        return error;
+unrefsav:
+       KEY_FREESAV(&sav);
 bad:
        splx(s);
        if (m)
diff -r 5e2f78517eeb -r 3ee8d08e3d1b sys/netipsec/key.c
--- a/sys/netipsec/key.c        Fri Jul 14 11:54:52 2017 +0000
+++ b/sys/netipsec/key.c        Fri Jul 14 12:26:26 2017 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: key.c,v 1.183 2017/07/14 01:30:08 ozaki-r Exp $        */
+/*     $NetBSD: key.c,v 1.184 2017/07/14 12:26:26 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.183 2017/07/14 01:30:08 ozaki-r Exp $");
+__KERNEL_RCSID(0, "$NetBSD: key.c,v 1.184 2017/07/14 12:26:26 ozaki-r Exp $");
 
 /*
  * This code is referd to RFC 2367
@@ -837,7 +837,7 @@



Home | Main Index | Thread Index | Old Index