git: e123e2294cb5 - main - pf: guard against DIOCADDRULE without DIOCXBEGIN

The branch main has been updated by mjg:

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

commit e123e2294cb50deb288916b79a8c05a006f8bca3
Author:     Mateusz Guzik <mjg@FreeBSD.org>
AuthorDate: 2022-03-29 13:17:54 +0000
Commit:     Mateusz Guzik <mjg@FreeBSD.org>
CommitDate: 2022-03-29 19:00:55 +0000

    pf: guard against DIOCADDRULE without DIOCXBEGIN
    
    Possibility to do it was always a bug, but it runs into crashes
    since recent introduction of a per-ruleset RB tree.
    
    Reviewed by:    kp
    Sponsored by:   Rubicon Communications, LLC ("Netgate")
    Reported by:    syzbot+665b700afc6f69f1766a@syzkaller.appspotmail.com
---
 sys/netpfil/pf/pf_ioctl.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/sys/netpfil/pf/pf_ioctl.c b/sys/netpfil/pf/pf_ioctl.c
index ae07fe80fbf8..6012825ead9b 100644
--- a/sys/netpfil/pf/pf_ioctl.c
+++ b/sys/netpfil/pf/pf_ioctl.c
@@ -1364,7 +1364,7 @@ pf_commit_rules(u_int32_t ticket, int rs_num, char *anchor)
 
 	rs->rules[rs_num].inactive.ptr = old_rules;
 	rs->rules[rs_num].inactive.ptr_array = old_array;
-	rs->rules[rs_num].inactive.tree = NULL;
+	rs->rules[rs_num].inactive.tree = NULL; /* important for pf_ioctl_addrule */
 	rs->rules[rs_num].inactive.rcount = old_rcount;
 
 	rs->rules[rs_num].active.ticket =
@@ -2137,6 +2137,16 @@ pf_ioctl_addrule(struct pf_krule *rule, uint32_t ticket,
 		    V_ticket_pabuf));
 		ERROUT(EBUSY);
 	}
+	/*
+	 * XXXMJG hack: there is no mechanism to ensure they started the
+	 * transaction. Ticket checked above may happen to match by accident,
+	 * even if nobody called DIOCXBEGIN, let alone this process.
+	 * Partially work around it by checking if the RB tree got allocated,
+	 * see pf_begin_rules.
+	 */
+	if (ruleset->rules[rs_num].inactive.tree == NULL) {
+		ERROUT(EINVAL);
+	}
 
 	tail = TAILQ_LAST(ruleset->rules[rs_num].inactive.ptr,
 	    pf_krulequeue);