From 0e25137aacafd0b8445eef84d77c82f326686a18 Mon Sep 17 00:00:00 2001 From: bluhm Date: Tue, 2 Jul 2024 18:33:47 +0000 Subject: [PATCH] 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@ --- sys/net/pf.c | 29 +++++++++++++++++------------ sys/netinet/ip_input.c | 12 +++++++++--- sys/netinet/ip_mroute.c | 12 ++++++------ sys/netinet/ip_output.c | 9 ++++----- sys/netinet/ip_var.h | 5 +++-- 5 files changed, 39 insertions(+), 28 deletions(-) diff --git a/sys/net/pf.c b/sys/net/pf.c index 2a462280b3e..b967b11aa48 100644 --- 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 d561a703488..21221c149d7 100644 --- 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 1913b1767a1..e6b7941f895 100644 --- 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 b21683a11f3..1f86f31e223 100644 --- 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 d4e2163446c..df6f003d8b6 100644 --- 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); -- 2.20.1