git: 74c2461386ea - main - pf: cope with missing rpool.cur

From: Kristof Provost <kp_at_FreeBSD.org>
Date: Wed, 04 Oct 2023 09:35:10 UTC
The branch main has been updated by kp:

URL: https://cgit.FreeBSD.org/src/commit/?id=74c2461386ea5eeb41e674df6b16a44b0509a882

commit 74c2461386ea5eeb41e674df6b16a44b0509a882
Author:     Kristof Provost <kp@FreeBSD.org>
AuthorDate: 2023-10-03 15:11:44 +0000
Commit:     Kristof Provost <kp@FreeBSD.org>
CommitDate: 2023-10-04 08:16:03 +0000

    pf: cope with missing rpool.cur
    
    If we're evaluating a pfsync'd state (and have different rules on both
    ends) our state may point to the default rule, which does not have
    rpool.cur set. As a result we can end up dereferencing a NULL pointer.
    
    Explicitly check for this when we try to re-construct the route-to interface.
    
    Also add a test case which can trigger this issue.
    
    MFC after:      3 days
    See also:       https://redmine.pfsense.org/issues/14804
    Sponsored by:   Rubicon Communications, LLC ("Netgate")
---
 sys/netpfil/pf/pf.c            |  9 ++--
 tests/sys/netpfil/pf/pfsync.sh | 96 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 101 insertions(+), 4 deletions(-)

diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c
index 3e1c8d32add9..fae0bd2854f9 100644
--- a/sys/netpfil/pf/pf.c
+++ b/sys/netpfil/pf/pf.c
@@ -6978,7 +6978,7 @@ pf_route(struct mbuf **m, struct pf_krule *r, struct ifnet *oifp,
 			} else {
 				ifp = s->rt_kif ? s->rt_kif->pfik_ifp : NULL;
 				/* If pfsync'd */
-				if (ifp == NULL)
+				if (ifp == NULL && r->rpool.cur != NULL)
 					ifp = r->rpool.cur->kif ?
 					    r->rpool.cur->kif->pfik_ifp : NULL;
 				PF_STATE_UNLOCK(s);
@@ -7035,9 +7035,10 @@ pf_route(struct mbuf **m, struct pf_krule *r, struct ifnet *oifp,
 			    s->rt_addr.v4.s_addr;
 		ifp = s->rt_kif ? s->rt_kif->pfik_ifp : NULL;
 		/* If pfsync'd */
-		if (ifp == NULL)
+		if (ifp == NULL && r->rpool.cur != NULL) {
 			ifp = r->rpool.cur->kif ?
 			    r->rpool.cur->kif->pfik_ifp : NULL;
+		}
 		PF_STATE_UNLOCK(s);
 	}
 
@@ -7191,7 +7192,7 @@ pf_route6(struct mbuf **m, struct pf_krule *r, struct ifnet *oifp,
 			} else {
 				ifp = s->rt_kif ? s->rt_kif->pfik_ifp : NULL;
 				/* If pfsync'd */
-				if (ifp == NULL)
+				if (ifp == NULL && r->rpool.cur != NULL)
 					ifp = r->rpool.cur->kif ?
 					    r->rpool.cur->kif->pfik_ifp : NULL;
 				PF_STATE_UNLOCK(s);
@@ -7249,7 +7250,7 @@ pf_route6(struct mbuf **m, struct pf_krule *r, struct ifnet *oifp,
 			    &s->rt_addr, AF_INET6);
 		ifp = s->rt_kif ? s->rt_kif->pfik_ifp : NULL;
 		/* If pfsync'd */
-		if (ifp == NULL)
+		if (ifp == NULL && r->rpool.cur != NULL)
 			ifp = r->rpool.cur->kif ?
 			    r->rpool.cur->kif->pfik_ifp : NULL;
 	}
diff --git a/tests/sys/netpfil/pf/pfsync.sh b/tests/sys/netpfil/pf/pfsync.sh
index 80b81205b59d..5d30f5b44888 100644
--- a/tests/sys/netpfil/pf/pfsync.sh
+++ b/tests/sys/netpfil/pf/pfsync.sh
@@ -826,6 +826,101 @@ basic_ipv6_cleanup()
 	pfsynct_cleanup
 }
 
+atf_test_case "route_to" "cleanup"
+route_to_head()
+{
+	atf_set descr 'Test route-to with default rule'
+	atf_set require.user root
+	atf_set require.progs scapy
+}
+
+route_to_body()
+{
+	pfsynct_init
+
+	epair_sync=$(vnet_mkepair)
+	epair_one=$(vnet_mkepair)
+	epair_two=$(vnet_mkepair)
+	epair_out_one=$(vnet_mkepair)
+	epair_out_two=$(vnet_mkepair)
+
+	vnet_mkjail one ${epair_one}a ${epair_sync}a ${epair_out_one}a
+	vnet_mkjail two ${epair_two}a ${epair_sync}b ${epair_out_two}a
+
+	# pfsync interface
+	jexec one ifconfig ${epair_sync}a 192.0.2.1/24 up
+	jexec one ifconfig ${epair_one}a 198.51.100.1/24 up
+	jexec one ifconfig ${epair_out_one}a 203.0.113.1/24 up
+	jexec one ifconfig ${epair_out_one}a name outif
+	jexec one sysctl net.inet.ip.forwarding=1
+	jexec one arp -s 203.0.113.254 00:01:02:03:04:05
+	jexec one ifconfig pfsync0 \
+		syncdev ${epair_sync}a \
+		maxupd 1 \
+		up
+
+	jexec two ifconfig ${epair_sync}b 192.0.2.2/24 up
+	jexec two ifconfig ${epair_two}a 198.51.100.2/24 up
+	jexec two ifconfig ${epair_out_two}a 203.0.113.2/24 up
+	#jexec two ifconfig ${epair_out_two}a name outif
+	jexec two sysctl net.inet.ip.forwarding=1
+	jexec two arp -s 203.0.113.254 00:01:02:03:04:05
+	jexec two ifconfig pfsync0 \
+		syncdev ${epair_sync}b \
+		maxupd 1 \
+		up
+
+	# Enable pf!
+	jexec one pfctl -e
+	pft_set_rules one \
+		"set skip on ${epair_sync}a" \
+		"pass out route-to (outif 203.0.113.254)"
+	jexec two pfctl -e
+
+	# Make sure we have different rulesets so the synced state is associated with
+	# V_pf_default_rule
+	pft_set_rules two \
+		"set skip on ${epair_sync}b" \
+		"pass out route-to (outif 203.0.113.254)" \
+		"pass out proto tcp"
+
+	ifconfig ${epair_one}b 198.51.100.254/24 up
+	ifconfig ${epair_two}b 198.51.100.253/24 up
+	route add -net 203.0.113.0/24 198.51.100.1
+	ifconfig ${epair_two}b up
+	ifconfig ${epair_out_one}b up
+	ifconfig ${epair_out_two}b up
+
+	atf_check -s exit:0 env PYTHONPATH=${common_dir} \
+		${common_dir}/pft_ping.py \
+		--sendif ${epair_one}b \
+		--fromaddr 198.51.100.254 \
+		--to 203.0.113.254 \
+		--recvif ${epair_out_one}b
+
+	# Allow time for sync
+	ifconfig ${epair_one}b inet 198.51.100.254 -alias
+	route del -net 203.0.113.0/24 198.51.100.1
+	route add -net 203.0.113.0/24 198.51.100.2
+
+	sleep 2
+
+	# Now try to trigger the state on the other pfsync member
+	env PYTHONPATH=${common_dir} \
+		${common_dir}/pft_ping.py \
+		--sendif ${epair_two}b \
+		--fromaddr 198.51.100.254 \
+		--to 203.0.113.254 \
+		--recvif ${epair_out_two}b
+
+	true
+}
+
+route_to_cleanup()
+{
+	pfsynct_cleanup
+}
+
 atf_init_test_cases()
 {
 	atf_add_test_case "basic"
@@ -838,4 +933,5 @@ atf_init_test_cases()
 	atf_add_test_case "timeout"
 	atf_add_test_case "basic_ipv6_unicast"
 	atf_add_test_case "basic_ipv6"
+	atf_add_test_case "route_to"
 }