Read IP forwarding variables only once.

Do not assume that ip_forwarding and ip_directedbcast cannot change
while processing one packet.  Read it once and pass down its value
with a flag.  This is necessary for unlocking the sysctl path.
There are a few places where a consistent value does not really
matter, they are unchanged.  Use a proper ip_ prefix for the global
variable.

OK claudio@

diff --git a/sys/net/pf.c b/sys/net/pf.c
index 8591b04..ecc6bfe 100644 (file)
--- a/sys/net/pf.c
+++ b/sys/net/pf.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: pf.c,v 1.1196 2024/05/14 08:26:13 jsg Exp $ */
+/*     $OpenBSD: pf.c,v 1.1197 2024/06/07 18:24:16 bluhm Exp $ */
 
 /*
  * Copyright (c) 2001 Daniel Hartmeier
@@ -7958,12 +7958,17 @@ done:
                switch (pd.naf) {
                case AF_INET:
                        if (pd.dir == PF_IN) {
-                               if (ipforwarding == 0) {
+                               int flags;
+
+                               if (ip_forwarding == 0) {
                                        ipstat_inc(ips_cantforward);
                                        action = PF_DROP;
                                        break;
                                }
-                               ip_forward(pd.m, ifp, NULL, 1);
+                               flags = IP_FORWARDING | IP_REDIRECT;
+                               if (ip_directedbcast)
+                                       SET(flags, IP_ALLOWBROADCAST);
+                               ip_forward(pd.m, ifp, NULL, flags);
                        } else
                                ip_output(pd.m, NULL, NULL, 0, NULL, NULL, 0);
                        break;

diff --git a/sys/netinet/ip_icmp.c b/sys/netinet/ip_icmp.c
index 30bec63..2d42796 100644 (file)
--- a/sys/netinet/ip_icmp.c
+++ b/sys/netinet/ip_icmp.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: ip_icmp.c,v 1.192 2023/09/16 09:33:27 mpi Exp $       */
+/*     $OpenBSD: ip_icmp.c,v 1.193 2024/06/07 18:24:16 bluhm Exp $     */
 /*     $NetBSD: ip_icmp.c,v 1.19 1996/02/13 23:42:22 christos Exp $    */
 
 /*
@@ -589,7 +589,7 @@ reflect:
                struct sockaddr_in ssrc;
                struct rtentry *newrt = NULL;
 
-               if (icmp_rediraccept == 0 || ipforwarding == 1)
+               if (icmp_rediraccept == 0 || ip_forwarding == 1)
                        goto freeit;
                if (code > 3)
                        goto badcode;

diff --git a/sys/netinet/ip_input.c b/sys/netinet/ip_input.c
index ad0455b..38057fc 100644 (file)
--- a/sys/netinet/ip_input.c
+++ b/sys/netinet/ip_input.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: ip_input.c,v 1.394 2024/05/08 13:01:30 bluhm Exp $    */
+/*     $OpenBSD: ip_input.c,v 1.395 2024/06/07 18:24:16 bluhm Exp $    */
 /*     $NetBSD: ip_input.c,v 1.30 1996/03/16 23:53:58 christos Exp $   */
 
 /*
@@ -84,10 +84,10 @@
 #endif
 
 /* values controllable via sysctl */
-int    ipforwarding = 0;
+int    ip_forwarding = 0;
 int    ipmforwarding = 0;
 int    ipmultipath = 0;
-int    ipsendredirects = 1;
+int    ip_sendredirects = 1;
 int    ip_dosourceroute = 0;
 int    ip_defttl = IPDEFTTL;
 int    ip_mtudisc = 1;
@@ -108,8 +108,8 @@ const struct sysctl_bounded_args ipctl_vars[] = {
 #ifdef MROUTING
        { IPCTL_MRTPROTO, &ip_mrtproto, SYSCTL_INT_READONLY },
 #endif
-       { IPCTL_FORWARDING, &ipforwarding, 0, 2 },
-       { IPCTL_SENDREDIRECTS, &ipsendredirects, 0, 1 },
+       { IPCTL_FORWARDING, &ip_forwarding, 0, 2 },
+       { IPCTL_SENDREDIRECTS, &ip_sendredirects, 0, 1 },
        { IPCTL_DEFTTL, &ip_defttl, 0, 255 },
        { IPCTL_DIRECTEDBCAST, &ip_directedbcast, 0, 1 },
        { IPCTL_IPPORT_FIRSTAUTO, &ipport_firstauto, 0, 65535 },
@@ -137,8 +137,8 @@ static struct mbuf_queue    ipsendraw_mq;
 extern struct niqueue          arpinq;
 
 int    ip_ours(struct mbuf **, int *, int, int);
-int    ip_dooptions(struct mbuf *, struct ifnet *);
-int    in_ouraddr(struct mbuf *, struct ifnet *, struct route *);
+int    ip_dooptions(struct mbuf *, struct ifnet *, int);
+int    in_ouraddr(struct mbuf *, struct ifnet *, struct route *, int);
 
 int            ip_fragcheck(struct mbuf **, int *);
 struct mbuf *  ip_reass(struct ipqent *, struct ipq *);
@@ -431,7 +431,7 @@ ip_input_if(struct mbuf **mp, int *offp, int nxt, int af, struct ifnet *ifp)
 #if NPF > 0
        struct in_addr odst;
 #endif
-       int pfrdr = 0;
+       int flags = 0;
 
        KASSERT(*offp == 0);
 
@@ -461,9 +461,15 @@ ip_input_if(struct mbuf **mp, int *offp, int nxt, int af, struct ifnet *ifp)
                goto bad;
 
        ip = mtod(m, struct ip *);
-       pfrdr = odst.s_addr != ip->ip_dst.s_addr;
+       if (odst.s_addr != ip->ip_dst.s_addr)
+               SET(flags, IP_REDIRECT);
 #endif
 
+       if (ip_forwarding != 0)
+               SET(flags, IP_FORWARDING);
+       if (ip_directedbcast)
+               SET(flags, IP_ALLOWBROADCAST);
+
        hlen = ip->ip_hl << 2;
 
        /*
@@ -472,7 +478,7 @@ ip_input_if(struct mbuf **mp, int *offp, int nxt, int af, struct ifnet *ifp)
         * error was detected (causing an icmp message
         * to be sent and the original packet to be freed).
         */
-       if (hlen > sizeof (struct ip) && ip_dooptions(m, ifp)) {
+       if (hlen > sizeof (struct ip) && ip_dooptions(m, ifp, flags)) {
                m = *mp = NULL;
                goto bad;
        }
@@ -483,7 +489,7 @@ ip_input_if(struct mbuf **mp, int *offp, int nxt, int af, struct ifnet *ifp)
                goto out;
        }
 
-       switch(in_ouraddr(m, ifp, &ro)) {
+       switch(in_ouraddr(m, ifp, &ro, flags)) {
        case 2:
                goto bad;
        case 1:
@@ -565,7 +571,7 @@ ip_input_if(struct mbuf **mp, int *offp, int nxt, int af, struct ifnet *ifp)
        /*
         * Not for us; forward if possible and desirable.
         */
-       if (ipforwarding == 0) {
+       if (!ISSET(flags, IP_FORWARDING)) {
                ipstat_inc(ips_cantforward);
                goto bad;
        }
@@ -585,7 +591,7 @@ ip_input_if(struct mbuf **mp, int *offp, int nxt, int af, struct ifnet *ifp)
        }
 #endif /* IPSEC */
 
-       ip_forward(m, ifp, &ro, pfrdr);
+       ip_forward(m, ifp, &ro, flags);
        *mp = NULL;
        rtfree(ro.ro_rt);
        return IPPROTO_DONE;
@@ -807,7 +813,7 @@ ip_deliver(struct mbuf **mp, int *offp, int nxt, int af, int shared)
 #undef IPSTAT_INC
 
 int
-in_ouraddr(struct mbuf *m, struct ifnet *ifp, struct route *ro)
+in_ouraddr(struct mbuf *m, struct ifnet *ifp, struct route *ro, int flags)
 {
        struct rtentry          *rt;
        struct ip               *ip;
@@ -837,7 +843,8 @@ in_ouraddr(struct mbuf *m, struct ifnet *ifp, struct route *ro)
                 * if it is received on the interface with that address.
                 */
                if (ISSET(rt->rt_flags, RTF_BROADCAST) &&
-                   (!ip_directedbcast || rt->rt_ifidx == ifp->if_index)) {
+                   (!ISSET(flags, IP_ALLOWBROADCAST) ||
+                   rt->rt_ifidx == ifp->if_index)) {
                        match = 1;
 
                        /* Make sure M_BCAST is set */
@@ -876,7 +883,8 @@ in_ouraddr(struct mbuf *m, struct ifnet *ifp, struct route *ro)
                                break;
                        }
                }
-       } else if (ipforwarding == 0 && rt->rt_ifidx != ifp->if_index &&
+       } else if (!ISSET(flags, IP_FORWARDING) &&
+           rt->rt_ifidx != ifp->if_index &&
            !((ifp->if_flags & IFF_LOOPBACK) || (ifp->if_type == IFT_ENC) ||
            (m->m_pkthdr.pf.flags & PF_TAG_TRANSLATE_LOCALHOST))) {
                /* received on wrong interface. */
@@ -1150,7 +1158,7 @@ ip_flush(void)
  * 0 if the packet should be processed further.
  */
 int
-ip_dooptions(struct mbuf *m, struct ifnet *ifp)
+ip_dooptions(struct mbuf *m, struct ifnet *ifp, int flags)
 {
        struct ip *ip = mtod(m, struct ip *);
        unsigned int rtableid = m->m_pkthdr.ph_rtableid;
@@ -1371,8 +1379,8 @@ ip_dooptions(struct mbuf *m, struct ifnet *ifp)
                }
        }
        KERNEL_UNLOCK();
-       if (forward && ipforwarding > 0) {
-               ip_forward(m, ifp, NULL, 1);
+       if (forward && ISSET(flags, IP_FORWARDING)) {
+               ip_forward(m, ifp, NULL, flags | IP_REDIRECT);
                return (1);
        }
        return (0);
@@ -1514,7 +1522,7 @@ const u_char inetctlerrmap[PRC_NCMDS] = {
  * of codes and types.
  *
  * If not forwarding, just drop the packet.  This could be confusing
- * if ipforwarding was zero but some routing protocol was advancing
+ * if ip_forwarding was zero but some routing protocol was advancing
  * us as a gateway to somewhere.  However, we must let the routing
  * protocol deal with that.
  *
@@ -1522,7 +1530,7 @@ const u_char inetctlerrmap[PRC_NCMDS] = {
  * via a source route.
  */
 void
-ip_forward(struct mbuf *m, struct ifnet *ifp, struct route *ro, int srcrt)
+ip_forward(struct mbuf *m, struct ifnet *ifp, struct route *ro, int flags)
 {
        struct mbuf mfake, *mcopy;
        struct ip *ip = mtod(m, struct ip *);
@@ -1588,7 +1596,7 @@ ip_forward(struct mbuf *m, struct ifnet *ifp, struct route *ro, int srcrt)
        if ((rt->rt_ifidx == ifp->if_index) &&
            (rt->rt_flags & (RTF_DYNAMIC|RTF_MODIFIED)) == 0 &&
            satosin(rt_key(rt))->sin_addr.s_addr != 0 &&
-           ipsendredirects && !srcrt &&
+           ip_sendredirects && !ISSET(flags, IP_REDIRECT) &&
            !arpproxy(satosin(rt_key(rt))->sin_addr, m->m_pkthdr.ph_rtableid)) {
                if ((ip->ip_src.s_addr & ifatoia(rt->rt_ifa)->ia_netmask) ==
                    ifatoia(rt->rt_ifa)->ia_net) {
@@ -1602,9 +1610,7 @@ ip_forward(struct mbuf *m, struct ifnet *ifp, struct route *ro, int srcrt)
                }
        }
 
-       error = ip_output(m, NULL, ro,
-           (IP_FORWARDING | (ip_directedbcast ? IP_ALLOWBROADCAST : 0)),
-           NULL, NULL, 0);
+       error = ip_output(m, NULL, ro, flags | IP_FORWARDING, NULL, NULL, 0);
        rt = ro->ro_rt;
        if (error)
                ipstat_inc(ips_cantforward);

diff --git a/sys/netinet/ip_output.c b/sys/netinet/ip_output.c
index c0aeb76..b21683a 100644 (file)
--- a/sys/netinet/ip_output.c
+++ b/sys/netinet/ip_output.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: ip_output.c,v 1.399 2024/05/16 13:01:04 bluhm Exp $   */
+/*     $OpenBSD: ip_output.c,v 1.400 2024/06/07 18:24:16 bluhm Exp $   */
 /*     $NetBSD: ip_output.c,v 1.28 1996/02/13 23:43:07 christos Exp $  */
 
 /*
@@ -428,7 +428,7 @@ sendit:
 #endif
 
 #ifdef IPSEC
-       if ((flags & IP_FORWARDING) && ipforwarding == 2 &&
+       if ((flags & IP_FORWARDING) && ip_forwarding == 2 &&
            (!ipsec_in_use ||
            m_tag_find(m, PACKET_TAG_IPSEC_IN_DONE, NULL) == NULL)) {
                error = EHOSTUNREACH;

diff --git a/sys/netinet/ip_var.h b/sys/netinet/ip_var.h
index a963798..d4e2163 100644 (file)
--- a/sys/netinet/ip_var.h
+++ b/sys/netinet/ip_var.h
@@ -1,4 +1,4 @@
-/*     $OpenBSD: ip_var.h,v 1.117 2024/04/17 20:48:51 bluhm Exp $      */
+/*     $OpenBSD: ip_var.h,v 1.118 2024/06/07 18:24:16 bluhm Exp $      */
 /*     $NetBSD: ip_var.h,v 1.16 1996/02/13 23:43:20 christos Exp $     */
 
 /*
@@ -204,10 +204,11 @@ struct ipoffnxt {
 };
 
 /* flags passed to ip_output */
-#define        IP_FORWARDING           0x1             /* most of ip header exists */
-#define        IP_RAWOUTPUT            0x2             /* raw ip header exists */
-#define        IP_ALLOWBROADCAST       SO_BROADCAST    /* can send broadcast packets */
-#define        IP_MTUDISC              0x0800          /* pmtu discovery, set DF */
+#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_ALLOWBROADCAST      SO_BROADCAST    /* can send broadcast packets */
+#define IP_MTUDISC             0x0800  /* pmtu discovery, set DF */
 
 extern struct ipstat ipstat;
 extern int ip_defttl;                  /* default IP ttl */
@@ -221,11 +222,12 @@ extern int ipport_firstauto;              /* min port for port allocation */
 extern int ipport_lastauto;            /* max port for port allocation */
 extern int ipport_hifirstauto;         /* min dynamic/private port number */
 extern int ipport_hilastauto;          /* max dynamic/private port number */
-extern int ipforwarding;               /* enable IP forwarding */
+extern int ip_forwarding;              /* enable IP forwarding */
 #ifdef MROUTING
 extern int ipmforwarding;              /* enable multicast forwarding */
 #endif
 extern int ipmultipath;                        /* enable multipath routing */
+extern int ip_directedbcast;           /* accept all broadcast packets */
 extern unsigned int la_hold_total;
 
 extern const struct pr_usrreqs rip_usrreqs;

diff --git a/sys/netinet6/ip6_forward.c b/sys/netinet6/ip6_forward.c
index 913fb64..13e6037 100644 (file)
--- a/sys/netinet6/ip6_forward.c
+++ b/sys/netinet6/ip6_forward.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: ip6_forward.c,v 1.117 2024/04/16 12:56:39 bluhm Exp $ */
+/*     $OpenBSD: ip6_forward.c,v 1.118 2024/06/07 18:24:16 bluhm Exp $ */
 /*     $KAME: ip6_forward.c,v 1.75 2001/06/29 12:42:13 jinmei Exp $    */
 
 /*
@@ -75,7 +75,7 @@
  * of codes and types.
  *
  * If not forwarding, just drop the packet.  This could be confusing
- * if ipforwarding was zero but some routing protocol was advancing
+ * if ip6_forwarding was zero but some routing protocol was advancing
  * us as a gateway to somewhere.  However, we must let the routing
  * protocol deal with that.
  *