Read IPsec forwarding information once.

Fix MP race between reading ip_forwarding in ip_input() and checking
ip_forwarding == 2 in ip_output().  In theory ip_forwarding could
be 2 during ip_input() and later 0 in ip_output().  Then a packet
would be forwarded that was never allowed.  Currently exclusive
netlock in sysctl(2) prevents all races.

Introduce IP_FORWARDING_IPSEC and pass it with the flags parameter
that was introduced for IP_FORWARDING.

Instead of calling m_tag_find(), traversing the list, and comparing
with NULL, just check the PACKET_TAG_IPSEC_IN_DONE bit.  Reading
ipsec_in_use in ip_output() is a performance hack that is not
necessary.  New code only checks tree bits.

OK mvs@

diff --git a/sys/net/pf.c b/sys/net/pf.c
index 2a46228..b967b11 100644 (file)
--- a/sys/net/pf.c
+++ b/sys/net/pf.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: pf.c,v 1.1199 2024/06/21 12:51:29 sashan Exp $ */
+/*     $OpenBSD: pf.c,v 1.1200 2024/07/02 18:33:47 bluhm Exp $ */
 
 /*
  * Copyright (c) 2001 Daniel Hartmeier
@@ -7958,20 +7958,26 @@ done:
        case PF_AFRT:
                if (pf_translate_af(&pd)) {
                        action = PF_DROP;
-                       break;
+                       goto out;
                }
                pd.m->m_pkthdr.pf.flags |= PF_TAG_GENERATED;
                switch (pd.naf) {
                case AF_INET:
                        if (pd.dir == PF_IN) {
-                               int flags;
-
-                               if (ip_forwarding == 0) {
+                               int flags = IP_REDIRECT;
+
+                               switch (ip_forwarding) {
+                               case 2:
+                                       SET(flags, IP_FORWARDING_IPSEC);
+                                       /* FALLTHROUGH */
+                               case 1:
+                                       SET(flags, IP_FORWARDING);
+                                       break;
+                               default:
                                        ipstat_inc(ips_cantforward);
                                        action = PF_DROP;
-                                       break;
+                                       goto out;
                                }
-                               flags = IP_FORWARDING | IP_REDIRECT;
                                if (ip_directedbcast)
                                        SET(flags, IP_ALLOWBROADCAST);
                                ip_forward(pd.m, ifp, NULL, flags);
@@ -7985,7 +7991,7 @@ done:
                                if (ip6_forwarding == 0) {
                                        ip6stat_inc(ip6s_cantforward);
                                        action = PF_DROP;
-                                       break;
+                                       goto out;
                                }
                                flags = IPV6_FORWARDING | IPV6_REDIRECT;
                                ip6_forward(pd.m, NULL, flags);
@@ -7993,10 +7999,8 @@ done:
                                ip6_output(pd.m, NULL, NULL, 0, NULL, NULL);
                        break;
                }
-               if (action != PF_DROP) {
-                       pd.m = NULL;
-                       action = PF_PASS;
-               }
+               pd.m = NULL;
+               action = PF_PASS;
                break;
 #endif /* INET6 */
        case PF_DROP:
@@ -8036,6 +8040,7 @@ done:
                        st->if_index_out = ifp->if_index;
        }
 
+ out:
        *m0 = pd.m;
 
        pf_state_unref(st);

diff --git a/sys/netinet/ip_input.c b/sys/netinet/ip_input.c
index d561a70..21221c1 100644 (file)
--- a/sys/netinet/ip_input.c
+++ b/sys/netinet/ip_input.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: ip_input.c,v 1.396 2024/06/24 12:19:19 bluhm Exp $    */
+/*     $OpenBSD: ip_input.c,v 1.397 2024/07/02 18:33:47 bluhm Exp $    */
 /*     $NetBSD: ip_input.c,v 1.30 1996/03/16 23:53:58 christos Exp $   */
 
 /*
@@ -465,8 +465,14 @@ ip_input_if(struct mbuf **mp, int *offp, int nxt, int af, struct ifnet *ifp)
                SET(flags, IP_REDIRECT);
 #endif
 
-       if (ip_forwarding != 0)
+       switch (ip_forwarding) {
+       case 2:
+               SET(flags, IP_FORWARDING_IPSEC);
+               /* FALLTHROUGH */
+       case 1:
                SET(flags, IP_FORWARDING);
+               break;
+       }
        if (ip_directedbcast)
                SET(flags, IP_ALLOWBROADCAST);
 
@@ -529,7 +535,7 @@ ip_input_if(struct mbuf **mp, int *offp, int nxt, int af, struct ifnet *ifp)
                         * ip_output().)
                         */
                        KERNEL_LOCK();
-                       error = ip_mforward(m, ifp);
+                       error = ip_mforward(m, ifp, flags);
                        KERNEL_UNLOCK();
                        if (error) {
                                ipstat_inc(ips_cantforward);

diff --git a/sys/netinet/ip_mroute.c b/sys/netinet/ip_mroute.c
index 1913b17..e6b7941 100644 (file)
--- a/sys/netinet/ip_mroute.c
+++ b/sys/netinet/ip_mroute.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: ip_mroute.c,v 1.142 2024/04/06 14:23:27 bluhm Exp $   */
+/*     $OpenBSD: ip_mroute.c,v 1.143 2024/07/02 18:33:47 bluhm Exp $   */
 /*     $NetBSD: ip_mroute.c,v 1.85 2004/04/26 01:31:57 matt Exp $      */
 
 /*
@@ -122,7 +122,7 @@ int get_api_support(struct mbuf *);
 int get_api_config(struct mbuf *);
 int socket_send(struct socket *, struct mbuf *,
                            struct sockaddr_in *);
-int ip_mdq(struct mbuf *, struct ifnet *, struct rtentry *);
+int ip_mdq(struct mbuf *, struct ifnet *, struct rtentry *, int);
 struct ifnet *if_lookupbyvif(vifi_t, unsigned int);
 struct rtentry *rt_mcast_add(struct ifnet *, struct sockaddr *,
     struct sockaddr *);
@@ -1080,7 +1080,7 @@ socket_send(struct socket *so, struct mbuf *mm, struct sockaddr_in *src)
 #define TUNNEL_LEN  12  /* # bytes of IP option for tunnel encapsulation  */
 
 int
-ip_mforward(struct mbuf *m, struct ifnet *ifp)
+ip_mforward(struct mbuf *m, struct ifnet *ifp, int flags)
 {
        struct ip *ip = mtod(m, struct ip *);
        struct vif *v;
@@ -1121,7 +1121,7 @@ ip_mforward(struct mbuf *m, struct ifnet *ifp)
 
        /* Entry exists, so forward if necessary */
        if (rt != NULL) {
-               return (ip_mdq(m, ifp, rt));
+               return (ip_mdq(m, ifp, rt, flags));
        } else {
                /*
                 * If we don't have a route for packet's origin,
@@ -1183,7 +1183,7 @@ ip_mforward(struct mbuf *m, struct ifnet *ifp)
  * Packet forwarding routine once entry in the cache is made
  */
 int
-ip_mdq(struct mbuf *m, struct ifnet *ifp0, struct rtentry *rt)
+ip_mdq(struct mbuf *m, struct ifnet *ifp0, struct rtentry *rt, int flags)
 {
        struct ip  *ip = mtod(m, struct ip *);
        struct mfc *mfc = (struct mfc *)rt->rt_llinfo;
@@ -1281,7 +1281,7 @@ ip_mdq(struct mbuf *m, struct ifnet *ifp0, struct rtentry *rt)
                imo.imo_ttl = ip->ip_ttl - IPTTLDEC;
                imo.imo_loop = 1;
 
-               ip_output(mc, NULL, NULL, IP_FORWARDING, &imo, NULL, 0);
+               ip_output(mc, NULL, NULL, flags | IP_FORWARDING, &imo, NULL, 0);
                if_put(ifp);
        } while ((rt = rtable_iterate(rt)) != NULL);
 

diff --git a/sys/netinet/ip_output.c b/sys/netinet/ip_output.c
index b21683a..1f86f31 100644 (file)
--- a/sys/netinet/ip_output.c
+++ b/sys/netinet/ip_output.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: ip_output.c,v 1.400 2024/06/07 18:24:16 bluhm Exp $   */
+/*     $OpenBSD: ip_output.c,v 1.401 2024/07/02 18:33:47 bluhm Exp $   */
 /*     $NetBSD: ip_output.c,v 1.28 1996/02/13 23:43:07 christos Exp $  */
 
 /*
@@ -331,7 +331,7 @@ reroute:
                                int rv;
 
                                KERNEL_LOCK();
-                               rv = ip_mforward(m, ifp);
+                               rv = ip_mforward(m, ifp, flags);
                                KERNEL_UNLOCK();
                                if (rv != 0)
                                        goto bad;
@@ -428,9 +428,8 @@ sendit:
 #endif
 
 #ifdef IPSEC
-       if ((flags & IP_FORWARDING) && ip_forwarding == 2 &&
-           (!ipsec_in_use ||
-           m_tag_find(m, PACKET_TAG_IPSEC_IN_DONE, NULL) == NULL)) {
+       if (ISSET(flags, IP_FORWARDING) && ISSET(flags, IP_FORWARDING_IPSEC) &&
+           !ISSET(m->m_pkthdr.ph_tagsset, PACKET_TAG_IPSEC_IN_DONE)) {
                error = EHOSTUNREACH;
                goto bad;
        }

diff --git a/sys/netinet/ip_var.h b/sys/netinet/ip_var.h
index d4e2163..df6f003 100644 (file)
--- a/sys/netinet/ip_var.h
+++ b/sys/netinet/ip_var.h
@@ -1,4 +1,4 @@
-/*     $OpenBSD: ip_var.h,v 1.118 2024/06/07 18:24:16 bluhm Exp $      */
+/*     $OpenBSD: ip_var.h,v 1.119 2024/07/02 18:33:47 bluhm Exp $      */
 /*     $NetBSD: ip_var.h,v 1.16 1996/02/13 23:43:20 christos Exp $     */
 
 /*
@@ -207,6 +207,7 @@ struct ipoffnxt {
 #define IP_FORWARDING          0x0001  /* most of ip header exists */
 #define IP_RAWOUTPUT           0x0002  /* raw ip header exists */
 #define IP_REDIRECT            0x0004  /* redirected by pf or source route */
+#define IP_FORWARDING_IPSEC    0x0008  /* only packets processed by IPsec */
 #define IP_ALLOWBROADCAST      SO_BROADCAST    /* can send broadcast packets */
 #define IP_MTUDISC             0x0800  /* pmtu discovery, set DF */
 
@@ -246,7 +247,7 @@ int  ip_getmoptions(int, struct ip_moptions *, struct mbuf *);
 void    ip_init(void);
 struct mbuf*
         ip_insertoptions(struct mbuf *, struct mbuf *, int *);
-int     ip_mforward(struct mbuf *, struct ifnet *);
+int     ip_mforward(struct mbuf *, struct ifnet *, int);
 int     ip_optcopy(struct ip *, struct ip *);
 int     ip_output(struct mbuf *, struct mbuf *, struct route *, int,
            struct ip_moptions *, const struct ipsec_level *, u_int32_t);