Hello,

My colleague hit the following kernel panic when he is doing "psctl
-ss" repeatedly by a script during a performance test.

 uvm_fault(0xfffffd811b869110, 0x10, 0, 1) -> e
 kernel: page fault trap, code=0
 Stopped at      pf_state_export+0x42:   movq    0x10(%rax),%rcx
     TID    PID    UID     PRFLAGS     PFLAGS  CPU  COMMAND
  313402  43614      0         0x3          0    0  awk
 * 66859  46071      0         0x3          0    3K pfctl
   48782  57541      0     0x14000      0x200    1  softnet
 pf_state_export(ffff80003a863218,fffffd81117155a8) at pf_state_export+0x42
 pf_states_get(ffff80003a863680) at pf_states_get+0xe1
 pfioctl(4900,c0104419,ffff80003a863680,1,ffff80003a823cf0) at pfioctl+0x11ba
 
VOP_IOCTL(fffffd811f894770,c0104419,ffff80003a863680,1,fffffd811a4e3ba8,ffff80003a823cf0)
 at VOP_IOCTL+0x57
 vn_ioctl(fffffd812742c168,c0104419,ffff80003a863680,ffff80003a823cf0) at 
vn_ioctl+0x79
 sys_ioctl(ffff80003a823cf0,ffff80003a863790,ffff80003a863780) at 
sys_ioctl+0x2b9
 syscall(ffff80003a863830) at syscall+0x35b
 Xsyscall() at Xsyscall+0xef
 end of kernel
 end trace frame: 0x7f7fffff3960, count: 7
 Running script...
 ddb{3}> 

Look at "pf_state_export+0x42":

 (gdb) l *(pf_state_export+0x42)
 0xffffffff819bb112 is in pf_state_export (/usr/src/sys/net/pf.c:1216).
 1211
 1212            memset(sp, 0, sizeof(struct pfsync_state));
 1213
 1214            /* copy from state key */
 1215            sp->key[PF_SK_WIRE].addr[0] = st->key[PF_SK_WIRE]->addr[0];
 1216            sp->key[PF_SK_WIRE].addr[1] = st->key[PF_SK_WIRE]->addr[1];
 1217            sp->key[PF_SK_WIRE].port[0] = st->key[PF_SK_WIRE]->port[0];
 1218            sp->key[PF_SK_WIRE].port[1] = st->key[PF_SK_WIRE]->port[1];
 1219            sp->key[PF_SK_WIRE].rdomain = 
htons(st->key[PF_SK_WIRE]->rdomain);
 1220            sp->key[PF_SK_WIRE].af = st->key[PF_SK_WIRE]->af;
 (gdb) 

the uvm_fault happened because st->key[PF_SK_WIRE] is NULL.  This is
can happen after pf_state_key_detach() <- pf_state_remove() is called
for the state.

The diff is to take care of that condition.

ok?  comments?


Don't assume pf state keys of pf_state are always there.  Take a
reference of pf state key with a reference counter and access its
members through it.  Found by IIJ.

Index: sys/net/pf.c
===================================================================
RCS file: /cvs/src/sys/net/pf.c,v
retrieving revision 1.1140
diff -u -p -r1.1140 pf.c
--- sys/net/pf.c        3 Sep 2022 19:22:19 -0000       1.1140
+++ sys/net/pf.c        21 Sep 2022 05:50:16 -0000
@@ -1193,26 +1193,37 @@ void
 pf_state_export(struct pfsync_state *sp, struct pf_state *st)
 {
        int32_t expire;
+       struct pf_state_key *skw, *sks;
 
        memset(sp, 0, sizeof(struct pfsync_state));
 
        /* copy from state key */
-       sp->key[PF_SK_WIRE].addr[0] = st->key[PF_SK_WIRE]->addr[0];
-       sp->key[PF_SK_WIRE].addr[1] = st->key[PF_SK_WIRE]->addr[1];
-       sp->key[PF_SK_WIRE].port[0] = st->key[PF_SK_WIRE]->port[0];
-       sp->key[PF_SK_WIRE].port[1] = st->key[PF_SK_WIRE]->port[1];
-       sp->key[PF_SK_WIRE].rdomain = htons(st->key[PF_SK_WIRE]->rdomain);
-       sp->key[PF_SK_WIRE].af = st->key[PF_SK_WIRE]->af;
-       sp->key[PF_SK_STACK].addr[0] = st->key[PF_SK_STACK]->addr[0];
-       sp->key[PF_SK_STACK].addr[1] = st->key[PF_SK_STACK]->addr[1];
-       sp->key[PF_SK_STACK].port[0] = st->key[PF_SK_STACK]->port[0];
-       sp->key[PF_SK_STACK].port[1] = st->key[PF_SK_STACK]->port[1];
-       sp->key[PF_SK_STACK].rdomain = htons(st->key[PF_SK_STACK]->rdomain);
-       sp->key[PF_SK_STACK].af = st->key[PF_SK_STACK]->af;
+       skw = pf_state_key_ref(st->key[PF_SK_WIRE]);
+       if (skw != NULL) {
+               sp->key[PF_SK_WIRE].addr[0] = skw->addr[0];
+               sp->key[PF_SK_WIRE].addr[1] = skw->addr[1];
+               sp->key[PF_SK_WIRE].port[0] = skw->port[0];
+               sp->key[PF_SK_WIRE].port[1] = skw->port[1];
+               sp->key[PF_SK_WIRE].rdomain = htons(skw->rdomain);
+               sp->key[PF_SK_WIRE].af = skw->af;
+               sp->proto = skw->proto;
+               sp->af = skw->af;
+               pf_state_key_unref(skw);
+               skw = NULL;
+       }
+       sks = pf_state_key_ref(st->key[PF_SK_STACK]);
+       if (sks != NULL) {
+               sp->key[PF_SK_STACK].addr[0] = sks->addr[0];
+               sp->key[PF_SK_STACK].addr[1] = sks->addr[1];
+               sp->key[PF_SK_STACK].port[0] = sks->port[0];
+               sp->key[PF_SK_STACK].port[1] = sks->port[1];
+               sp->key[PF_SK_STACK].rdomain = htons(sks->rdomain);
+               sp->key[PF_SK_STACK].af = sks->af;
+               pf_state_key_unref(sks);
+               sks = NULL;
+       }
        sp->rtableid[PF_SK_WIRE] = htonl(st->rtableid[PF_SK_WIRE]);
        sp->rtableid[PF_SK_STACK] = htonl(st->rtableid[PF_SK_STACK]);
-       sp->proto = st->key[PF_SK_WIRE]->proto;
-       sp->af = st->key[PF_SK_WIRE]->af;
 
        /* copy from state */
        strlcpy(sp->ifname, st->kif->pfik_name, sizeof(sp->ifname));

Reply via email to