pf: fix NULL dereference in pfr_set_addrs() with feedback

Since DIOCRSETADDRS was converted to netlink, pf_handle_table_set_addrs()
calls pfr_set_addrs() with a NULL size2, as the netlink interface has no
buffer to return the deleted addresses in. pfr_set_addrs() only checked
size2 for NULL at the end of the function; with PFR_FLAG_FEEDBACK set it
dereferenced it unconditionally first. pfctl sets PFR_FLAG_FEEDBACK
when run with -v, so "pfctl -v -t foo -T replace ..." panicked the
kernel with a NULL pointer dereference. To reproduce:

pfctl -e
pfctl -t foo -T add 192.0.2.1
pfctl -v -t foo -T replace 192.0.2.2

Check size2 for NULL before dereferencing it, as is already done at the
end of the function. The per-address feedback for added and changed
addresses is still copied back as before; only the list of deleted
addresses, which the netlink caller has no room for, is skipped.

While here, compare size2 against NULL explicitly in the second check as
well, per style(9).

Add a regression test.

Approved by:		kp (mentor)
Fixes:			08ed87a4a2 ("pf: convert DIOCRSETADDRS to netlink")
MFC after:		1 week
Differential Revision:	https://reviews.freebsd.org/D60096

sys/netpfil/pf/pf_table.c

@@ -464,7 +464,7 @@ _skip:
 	}
 	if (flags & PFR_FLAG_DONE)
 		pfr_enqueue_addrs(kt, &delq, &xdel, ENQUEUE_UNMARKED_ONLY);
-	if ((flags & PFR_FLAG_FEEDBACK) && *size2) {
+	if ((flags & PFR_FLAG_FEEDBACK) && size2 != NULL && *size2) {
 		if (*size2 < size+xdel) {
 			*size2 = size+xdel;
 			senderr(0);
@@ -490,7 +490,7 @@ _skip:
 		*ndel = xdel;
 	if (nchange != NULL)
 		*nchange = xchange;
-	if ((flags & PFR_FLAG_FEEDBACK) && size2)
+	if ((flags & PFR_FLAG_FEEDBACK) && size2 != NULL)
 		*size2 = size+xdel;
 	pfr_destroy_ktable(tmpkt, 0);
 	return (0);

tests/sys/netpfil/pf/table.sh

@@ -812,6 +812,39 @@ replace_cleanup()
 	pft_cleanup
 }
 
+atf_test_case "replace_verbose" "cleanup"
+replace_verbose_head()
+{
+	atf_set descr 'Test table replace command, asked to be verbose'
+	atf_set require.user root
+}
+
+replace_verbose_body()
+{
+	pft_init
+
+	vnet_mkjail alcatraz
+	jexec alcatraz pfctl -e
+
+	pft_set_rules alcatraz \
+	    "table <foo> { 192.0.2.1, 192.0.2.2 }" \
+	    "pass in from <foo> to any"
+
+	# This used to panic: pfr_set_addrs() dereferenced a NULL size2
+	# when asked for feedback over netlink.
+	atf_check -s exit:0 -e "match:1 addresses added." \
+	    -e "match:1 addresses deleted." \
+	    jexec alcatraz pfctl -v -t foo -T replace 192.0.2.2 192.0.2.3
+	atf_check -s exit:0 -o "match:192.0.2.2" -o "match:192.0.2.3" \
+	    -o "not-match:192.0.2.1" \
+	    jexec alcatraz pfctl -t foo -T show
+}
+
+replace_verbose_cleanup()
+{
+	pft_cleanup
+}
+
 atf_test_case "load" "cleanup"
 load_head()
 {
@@ -902,6 +935,7 @@ atf_init_test_cases()
 	atf_add_test_case "show_recursive"
 	atf_add_test_case "in_anchor"
 	atf_add_test_case "replace"
+	atf_add_test_case "replace_verbose"
 	atf_add_test_case "load"
 	atf_add_test_case "test"
 }