Source-Changes-HG archive

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

[src/netbsd-8]: src/sys/netipsec Pull up following revision(s) (requested by ...



details:   https://anonhg.NetBSD.org/src/rev/1c0f04576688
branches:  netbsd-8
changeset: 434456:1c0f04576688
user:      martin <martin%NetBSD.org@localhost>
date:      Thu Nov 30 14:57:34 2017 +0000

description:
Pull up following revision(s) (requested by ozaki-r in ticket #406):
        sys/netipsec/key.c: revision 1.239
        sys/netipsec/key.c: revision 1.240
        sys/netipsec/key.c: revision 1.241
        sys/netipsec/key.c: revision 1.242
        sys/netipsec/key.h: revision 1.33
        sys/netipsec/ipsec.c: revision 1.123
        sys/netipsec/key.c: revision 1.236
        sys/netipsec/key.c: revision 1.237
        sys/netipsec/key.c: revision 1.238
Provide a function to call MGETHDR and MCLGET
The change fixes two usages of MGETHDR that don't check whether a mbuf is really
allocated before passing it to MCLGET.
Fix error handling of MCLGET in key_alloc_mbuf
Add missing splx to key_spdexpire
Use M_WAITOK to allocate mbufs wherever sleepable
Further changes will get rid of unnecessary NULL checks then.
Get rid of unnecessary NULL checks that are obsoleted by M_WAITOK
Simply the code by avoiding unnecessary error checks
- Remove unnecessary m_pullup for self-allocated mbufs
- Replace some if-fails-return sanity checks with KASSERT
Call key_sendup_mbuf immediately unless key_acquire is called in softint
We need to defer it only if it's called in softint to avoid deadlock.

diffstat:

 sys/netipsec/ipsec.c |    6 +-
 sys/netipsec/key.c   |  562 +++++++++++++++-----------------------------------
 sys/netipsec/key.h   |    4 +-
 3 files changed, 180 insertions(+), 392 deletions(-)

diffs (truncated from 1260 to 300 lines):

diff -r 7e63524d7e55 -r 1c0f04576688 sys/netipsec/ipsec.c
--- a/sys/netipsec/ipsec.c      Thu Nov 30 14:40:46 2017 +0000
+++ b/sys/netipsec/ipsec.c      Thu Nov 30 14:57:34 2017 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: ipsec.c,v 1.99.2.1 2017/10/21 19:43:54 snj Exp $       */
+/*     $NetBSD: ipsec.c,v 1.99.2.2 2017/11/30 14:57:34 martin 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.99.2.1 2017/10/21 19:43:54 snj Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ipsec.c,v 1.99.2.2 2017/11/30 14:57:34 martin Exp $");
 
 /*
  * IPsec controller part.
@@ -1422,7 +1422,7 @@
        if (policy == NULL || mp == NULL)
                return EINVAL;
 
-       *mp = key_sp2msg(policy);
+       *mp = key_sp2msg(policy, M_NOWAIT);
        if (!*mp) {
                IPSECLOG(LOG_DEBUG, "No more memory.\n");
                return ENOBUFS;
diff -r 7e63524d7e55 -r 1c0f04576688 sys/netipsec/key.c
--- a/sys/netipsec/key.c        Thu Nov 30 14:40:46 2017 +0000
+++ b/sys/netipsec/key.c        Thu Nov 30 14:57:34 2017 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: key.c,v 1.163.2.2 2017/11/21 11:11:20 martin Exp $     */
+/*     $NetBSD: key.c,v 1.163.2.3 2017/11/30 14:57:34 martin 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.163.2.2 2017/11/21 11:11:20 martin Exp $");
+__KERNEL_RCSID(0, "$NetBSD: key.c,v 1.163.2.3 2017/11/30 14:57:34 martin Exp $");
 
 /*
  * This code is referred to RFC 2367
@@ -601,23 +601,18 @@
        return PFKEY_ADDR_SADDR(mhp->ext[idx]);
 }
 
-static struct mbuf *
+static void
 key_fill_replymsg(struct mbuf *m, int seq)
 {
        struct sadb_msg *msg;
 
-       if (m->m_len < sizeof(*msg)) {
-               m = m_pullup(m, sizeof(*msg));
-               if (m == NULL)
-                       return NULL;
-       }
+       KASSERT(m->m_len >= sizeof(*msg));
+
        msg = mtod(m, struct sadb_msg *);
        msg->sadb_msg_errno = 0;
        msg->sadb_msg_len = PFKEY_UNIT64(m->m_pkthdr.len);
        if (seq != 0)
                msg->sadb_msg_seq = seq;
-
-       return m;
 }
 
 #if 0
@@ -684,17 +679,17 @@
 static void key_porttosaddr (union sockaddr_union *, u_int16_t);
 static int key_checksalen (const union sockaddr_union *);
 static struct mbuf *key_setsadbmsg (u_int8_t, u_int16_t, u_int8_t,
-       u_int32_t, pid_t, u_int16_t);
+       u_int32_t, pid_t, u_int16_t, int);
 static struct mbuf *key_setsadbsa (struct secasvar *);
-static struct mbuf *key_setsadbaddr (u_int16_t,
-       const struct sockaddr *, u_int8_t, u_int16_t);
+static struct mbuf *key_setsadbaddr(u_int16_t,
+       const struct sockaddr *, u_int8_t, u_int16_t, int);
 #if 0
 static struct mbuf *key_setsadbident (u_int16_t, u_int16_t, void *,
        int, u_int64_t);
 #endif
 static struct mbuf *key_setsadbxsa2 (u_int8_t, u_int32_t, u_int16_t);
 static struct mbuf *key_setsadbxpolicy (u_int16_t, u_int8_t,
-       u_int32_t);
+       u_int32_t, int);
 static void *key_newbuf (const void *, u_int);
 #ifdef INET6
 static int key_ismyaddr6 (const struct sockaddr_in6 *);
@@ -748,12 +743,13 @@
        const struct sadb_msghdr *);
 
 static void key_getcomb_setlifetime (struct sadb_comb *);
-static struct mbuf *key_getcomb_esp (void);
-static struct mbuf *key_getcomb_ah (void);
-static struct mbuf *key_getcomb_ipcomp (void);
-static struct mbuf *key_getprop (const struct secasindex *);
-
-static int key_acquire(const struct secasindex *, const struct secpolicy *);
+static struct mbuf *key_getcomb_esp(int);
+static struct mbuf *key_getcomb_ah(int);
+static struct mbuf *key_getcomb_ipcomp(int);
+static struct mbuf *key_getprop(const struct secasindex *, int);
+
+static int key_acquire(const struct secasindex *, const struct secpolicy *,
+           int);
 static int key_acquire_sendup_mbuf_later(struct mbuf *);
 static void key_acquire_sendup_pending_mbuf(void);
 #ifndef IPSEC_NONBLOCK_ACQUIRE
@@ -787,7 +783,8 @@
 #endif
 static void key_sa_chgstate (struct secasvar *, u_int8_t);
 
-static struct mbuf *key_alloc_mbuf (int);
+static struct mbuf *key_alloc_mbuf(int, int);
+static struct mbuf *key_alloc_mbuf_simple(int, int);
 
 static void key_timehandler(void *);
 static void key_timehandler_work(struct work *, void *);
@@ -1025,7 +1022,7 @@
        }
 
        /* there is no SA */
-       error = key_acquire(saidx, isr->sp);
+       error = key_acquire(saidx, isr->sp, M_NOWAIT);
        if (error != 0) {
                /* XXX What should I do ? */
                IPSECLOG(LOG_DEBUG, "error %d returned from key_acquire.\n",
@@ -1969,7 +1966,7 @@
  * copy secpolicy struct to sadb_x_policy structure indicated.
  */
 struct mbuf *
-key_sp2msg(const struct secpolicy *sp)
+key_sp2msg(const struct secpolicy *sp, int mflag)
 {
        struct sadb_x_policy *xpl;
        int tlen;
@@ -1980,7 +1977,7 @@
 
        tlen = key_getspreqmsglen(sp);
 
-       m = key_alloc_mbuf(tlen);
+       m = key_alloc_mbuf(tlen, mflag);
        if (!m || m->m_next) {  /*XXX*/
                if (m)
                        m_freem(m);
@@ -2029,7 +2026,11 @@
        return m;
 }
 
-/* m will not be freed nor modified */
+/*
+ * m will not be freed nor modified. It never return NULL.
+ * If it returns a mbuf of M_PKTHDR, the mbuf ensures to have
+ * contiguous length at least sizeof(struct sadb_msg).
+ */
 static struct mbuf *
 key_gather_mbuf(struct mbuf *m, const struct sadb_msghdr *mhp,
                int ndeep, int nitem, ...)
@@ -2042,12 +2043,13 @@
 
        KASSERT(m != NULL);
        KASSERT(mhp != NULL);
+       KASSERT(!cpu_softintr_p());
 
        va_start(ap, nitem);
        for (i = 0; i < nitem; i++) {
                idx = va_arg(ap, int);
-               if (idx < 0 || idx > SADB_EXT_MAX)
-                       goto fail;
+               KASSERT(idx >= 0);
+               KASSERT(idx <= SADB_EXT_MAX);
                /* don't attempt to pull empty extension */
                if (idx == SADB_EXT_RESERVED && mhp->msg == NULL)
                        continue;
@@ -2058,29 +2060,22 @@
                if (idx == SADB_EXT_RESERVED) {
                        CTASSERT(PFKEY_ALIGN8(sizeof(struct sadb_msg)) <= MHLEN);
                        len = PFKEY_ALIGN8(sizeof(struct sadb_msg));
-                       MGETHDR(n, M_DONTWAIT, MT_DATA);
-                       if (!n)
-                               goto fail;
+                       MGETHDR(n, M_WAITOK, MT_DATA);
                        n->m_len = len;
                        n->m_next = NULL;
                        m_copydata(m, 0, sizeof(struct sadb_msg),
                            mtod(n, void *));
                } else if (i < ndeep) {
                        len = mhp->extlen[idx];
-                       n = key_alloc_mbuf(len);
-                       if (!n || n->m_next) {  /*XXX*/
-                               if (n)
-                                       m_freem(n);
-                               goto fail;
-                       }
+                       n = key_alloc_mbuf(len, M_WAITOK);
+                       KASSERT(n->m_next == NULL);
                        m_copydata(m, mhp->extoff[idx], mhp->extlen[idx],
                            mtod(n, void *));
                } else {
                        n = m_copym(m, mhp->extoff[idx], mhp->extlen[idx],
-                           M_DONTWAIT);
-               }
-               if (n == NULL)
-                       goto fail;
+                           M_WAITOK);
+               }
+               KASSERT(n != NULL);
 
                if (result)
                        m_cat(result, n);
@@ -2089,18 +2084,15 @@
        }
        va_end(ap);
 
-       if (result && (result->m_flags & M_PKTHDR) != 0) {
+       KASSERT(result != NULL);
+       if ((result->m_flags & M_PKTHDR) != 0) {
                result->m_pkthdr.len = 0;
                for (n = result; n; n = n->m_next)
                        result->m_pkthdr.len += n->m_len;
+               KASSERT(result->m_len >= sizeof(struct sadb_msg));
        }
 
        return result;
-
-fail:
-       va_end(ap);
-       m_freem(result);
-       return NULL;
 }
 
 /*
@@ -2278,13 +2270,8 @@
                    SADB_X_EXT_POLICY,
                    SADB_EXT_ADDRESS_SRC, SADB_EXT_ADDRESS_DST);
        }
-       if (!n)
-               return key_senderror(so, m, ENOBUFS);
-
-       n = key_fill_replymsg(n, 0);
-       if (n == NULL)
-               return key_senderror(so, m, ENOBUFS);
-
+
+       key_fill_replymsg(n, 0);
        off = 0;
        mpolicy = m_pulldown(n, PFKEY_ALIGN8(sizeof(struct sadb_msg)),
            sizeof(*xpl), &off);
@@ -2404,18 +2391,30 @@
        /* create new sadb_msg to reply. */
        n = key_gather_mbuf(m, mhp, 1, 4, SADB_EXT_RESERVED,
            SADB_X_EXT_POLICY, SADB_EXT_ADDRESS_SRC, SADB_EXT_ADDRESS_DST);
-       if (!n)
-               return key_senderror(so, m, ENOBUFS);
-
-       n = key_fill_replymsg(n, 0);
-       if (n == NULL)
-               return key_senderror(so, m, ENOBUFS);
-
+       key_fill_replymsg(n, 0);
        m_freem(m);
        return key_sendup_mbuf(so, n, KEY_SENDUP_ALL);
     }
 }
 
+static struct mbuf *
+key_alloc_mbuf_simple(int len, int mflag)
+{
+       struct mbuf *n;
+
+       KASSERT(mflag == M_NOWAIT || (mflag == M_WAITOK && !cpu_softintr_p()));
+
+       MGETHDR(n, mflag, MT_DATA);
+       if (n && len > MHLEN) {
+               MCLGET(n, mflag);
+               if ((n->m_flags & M_EXT) == 0) {
+                       m_freem(n);
+                       n = NULL;
+               }
+       }
+       return n;
+}
+
 /*
  * SADB_SPDDELETE2 processing
  * receive
@@ -2465,17 +2464,7 @@
        /* create new sadb_msg to reply. */
        len = PFKEY_ALIGN8(sizeof(struct sadb_msg));
 
-       MGETHDR(n, M_DONTWAIT, MT_DATA);
-       if (n && len > MHLEN) {
-               MCLGET(n, M_DONTWAIT);
-               if ((n->m_flags & M_EXT) == 0) {



Home | Main Index | Thread Index | Old Index