git: 6a88e22728d2 - main - pfctl: pfik_ifp is always NULL

The branch main has been updated by kp:

URL: https://cgit.FreeBSD.org/src/commit/?id=6a88e22728d285c4df17216515ce2b8d1e5a6835

commit 6a88e22728d285c4df17216515ce2b8d1e5a6835
Author:     Kristof Provost <kp@FreeBSD.org>
AuthorDate: 2024-08-16 12:55:31 +0000
Commit:     Kristof Provost <kp@FreeBSD.org>
CommitDate: 2024-08-19 16:02:15 +0000

    pfctl: pfik_ifp is always NULL
    
    The pfik_ifp field is not provided by the kernel, it is always NULL. Do not
    check for it. This caused us to not clear the skip flag on interfaces, leading
    to unexpected behaviour when a 'set skip' was removed.
    
    PR:             280834
    Sponsored by:   Rubicon Communications, LLC ("Netgate")
    Differential Revision:  https://reviews.freebsd.org/D46311
---
 sbin/pfctl/pfctl.c               |  7 +----
 tests/sys/netpfil/pf/set_skip.sh | 61 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 62 insertions(+), 6 deletions(-)

diff --git a/sbin/pfctl/pfctl.c b/sbin/pfctl/pfctl.c
index b60e64fba338..45bfdf31f8dc 100644
--- a/sbin/pfctl/pfctl.c
+++ b/sbin/pfctl/pfctl.c
@@ -394,8 +394,6 @@ pfctl_check_skip_ifaces(char *ifname)
 				continue;
 
 			for (n = h; n != NULL; n = n->next) {
-				if (p->pfik_ifp == NULL)
-					continue;
 				if (strncmp(p->pfik_name, ifname, IFNAMSIZ))
 					continue;
 
@@ -422,9 +420,6 @@ pfctl_adjust_skip_ifaces(struct pfctl *pf)
 
 		for (n = h; n != NULL; n = n->next)
 			PFRB_FOREACH(pp, &skip_b) {
-				if (pp->pfik_ifp == NULL)
-					continue;
-
 				if (strncmp(pp->pfik_name, n->ifname, IFNAMSIZ))
 					continue;
 
@@ -437,7 +432,7 @@ pfctl_adjust_skip_ifaces(struct pfctl *pf)
 	}
 
 	PFRB_FOREACH(p, &skip_b) {
-		if (p->pfik_ifp == NULL || ! (p->pfik_flags & PFI_IFLAG_SKIP))
+		if (! (p->pfik_flags & PFI_IFLAG_SKIP))
 			continue;
 
 		pfctl_set_interface_flags(pf, p->pfik_name, PFI_IFLAG_SKIP, 0);
diff --git a/tests/sys/netpfil/pf/set_skip.sh b/tests/sys/netpfil/pf/set_skip.sh
index e5b1440360e9..e984377721b8 100644
--- a/tests/sys/netpfil/pf/set_skip.sh
+++ b/tests/sys/netpfil/pf/set_skip.sh
@@ -26,6 +26,50 @@
 
 . $(atf_get_srcdir)/utils.subr
 
+atf_test_case "unset" "cleanup"
+unset_head()
+{
+	atf_set descr 'Unset set skip test'
+	atf_set require.user root
+}
+
+unset_body()
+{
+	pft_init
+
+	vnet_mkjail alcatraz
+	jexec alcatraz ifconfig lo0 127.0.0.1/8 up
+	jexec alcatraz pfctl -e
+	pft_set_rules alcatraz "set skip on lo0" \
+		"block in proto icmp"
+
+	echo "set skip"
+	jexec alcatraz pfctl -v -sI
+
+	jexec alcatraz ifconfig
+	atf_check -s exit:0 -o ignore jexec alcatraz ping -c 1 127.0.0.1
+
+	# Unset the skip on the group
+	pft_set_rules noflush alcatraz \
+	    "block in proto icmp"
+
+	echo "No setskip"
+	jexec alcatraz pfctl -v -sI
+
+	# Do flush states
+	jexec alcatraz pfctl -Fs
+
+	# And now our ping is blocked
+	atf_check -s exit:2 -o ignore jexec alcatraz ping -c 1 127.0.0.1
+
+	jexec alcatraz pfctl -v -sI
+}
+
+unset_cleanup()
+{
+	pft_cleanup
+}
+
 atf_test_case "set_skip_group" "cleanup"
 set_skip_group_head()
 {
@@ -45,8 +89,24 @@ set_skip_group_body()
 	pft_set_rules alcatraz "set skip on foo" \
 		"block in proto icmp"
 
+	echo "set skip"
+	jexec alcatraz pfctl -v -sI
+
 	jexec alcatraz ifconfig
 	atf_check -s exit:0 -o ignore jexec alcatraz ping -c 1 127.0.0.1
+
+	# Unset the skip on the group
+	pft_set_rules noflush alcatraz \
+	    "block in proto icmp"
+
+	# Do flush states
+	jexec alcatraz pfctl -Fs
+
+	# And now our ping is blocked
+	atf_check -s exit:2 -o ignore jexec alcatraz ping -c 1 127.0.0.1
+
+	echo "No setskip"
+	jexec alcatraz pfctl -v -sI
 }
 
 set_skip_group_cleanup()
@@ -163,6 +223,7 @@ pr255852_cleanup()
 
 atf_init_test_cases()
 {
+	atf_add_test_case "unset"
 	atf_add_test_case "set_skip_group"
 	atf_add_test_case "set_skip_group_lo"
 	atf_add_test_case "set_skip_dynamic"