consistently means we do the check in pf_rule_copyin() so both
DIOCADDRULE and DIOCCHANGERULE have the prio values checked. this in
turn prevents invalid prio values getting set on a rule via
DIOCCHANGERULE, which in turn stops a kassert in the ifq priq code
firing.

i think this fixes
https://syzkaller.appspot.com/bug?id=c5cf86b2a0fc06f60463e60c02086756747970d4.

ok?

Index: pf_ioctl.c
===================================================================
RCS file: /cvs/src/sys/net/pf_ioctl.c,v
retrieving revision 1.372
diff -u -p -r1.372 pf_ioctl.c
--- pf_ioctl.c  9 Feb 2022 11:42:58 -0000       1.372
+++ pf_ioctl.c  14 Feb 2022 03:22:44 -0000
@@ -1370,15 +1370,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
                        break;
                }
 
-               if (rule->scrub_flags & PFSTATE_SETPRIO &&
-                   (rule->set_prio[0] > IFQ_MAXPRIO ||
-                   rule->set_prio[1] > IFQ_MAXPRIO)) {
-                       error = EINVAL;
-                       pf_rule_free(rule);
-                       rule = NULL;
-                       break;
-               }
-
                NET_LOCK();
                PF_LOCK();
                pr->anchor[sizeof(pr->anchor) - 1] = '\0';
@@ -3070,6 +3061,11 @@ int
 pf_rule_copyin(struct pf_rule *from, struct pf_rule *to)
 {
        int i;
+
+       if (from->scrub_flags & PFSTATE_SETPRIO &&
+           (from->set_prio[0] > IFQ_MAXPRIO ||
+           from->set_prio[1] > IFQ_MAXPRIO))
+               return (EINVAL);
 
        to->src = from->src;
        to->src.addr.p.tbl = NULL;

Reply via email to