Hello, I'll commit one-liner diff on Tuesday morning (Jun 6th) Prague time without explicit OK, unless there will be no objection.
regards sashan On Sun, Jun 05, 2022 at 09:44:45AM +0100, Stuart Henderson wrote: > I don't know this code well enough to give a meaningful OK, but this > should probably get committed. > > > On 2022/06/01 09:16, Alexandr Nedvedicky wrote: > > Hello, > > > > </snip> > > > r420-1# rcctl -f start relayd > > > relayd(ok) > > > r420-1# uvm_fault(0xfffffd862f82f990, 0x0, 0, 1) -> e > > > kernel: page fault trap, code=0 > > > Stopped at pf_find_or_create_ruleset+0x1c: movb 0(%rdi),%al > > > TID PID UID PRFLAGS PFLAGS CPU COMMAND > > > 431388 19003 0 0x2 0 5 relayd > > > 174608 32253 89 0x1000012 0 2 relayd > > > 395415 12468 0 0x2 0 4 relayd > > > 493579 11904 0 0x2 0 3 relayd > > > *101082 14967 89 0x1100012 0 0K relayd > > > pf_find_or_create_ruleset(0) at pf_find_or_create_ruleset+0x1c > > > pfr_add_tables(832d7cca800,1,ffff800000eaf43c,10000000) at > > > pfr_add_tables+0x6ae > > > > > > pfioctl(4900,c450443d,ffff800000eaf000,3,ffff80002272e7f0) at > > > pfioctl+0x1d9f > > > VOP_IOCTL(fffffd8551f82dd0,c450443d,ffff800000eaf000,3,fffffd862f7d60c0,ffff800 > > > 02272e7f0) at VOP_IOCTL+0x5c > > > vn_ioctl(fffffd855ecec1e8,c450443d,ffff800000eaf000,ffff80002272e7f0) at > > > vn_ioctl+0x75 > > > sys_ioctl(ffff80002272e7f0,ffff8000227d9980,ffff8000227d99d0) at > > > sys_ioctl+0x2c4 > > > syscall(ffff8000227d9a40) at syscall+0x374 > > > Xsyscall() at Xsyscall+0x128 > > > end of kernel > > > > it looks like we are dying here at line 239 due to NULL pointer > > deference: > > > > 232 struct pf_ruleset * > > 233 pf_find_or_create_ruleset(const char *path) > > 234 { > > 235 char *p, *aname, *r; > > 236 struct pf_ruleset *ruleset; > > 237 struct pf_anchor *anchor; > > 238 > > 239 if (path[0] == 0) > > 240 return (&pf_main_ruleset); > > 241 > > 242 while (*path == '/') > > 243 path++; > > 244 > > > > I've followed the same steps to reproduce the issue to check if > > diff below resolves the issue. The bug has been introduced by > > my recent change to pf_table.c [1] from May 10th: > > > > Modified files: > > sys/net : pf_ioctl.c pf_table.c > > > > Log message: > > move memory allocations in pfr_add_tables() out of > > NET_LOCK()/PF_LOCK() scope. bluhm@ helped a lot > > to put this diff into shape. > > > > besides using a regression test I've also did simple testing > > using a 'load anchor': > > > > netlock# cat /tmp/anchor.conf > > > > load anchor "test" from "/tmp/pf.conf" > > netlock# > > netlock# cat /tmp/pf.conf > > > > table <try> { 192.168.1.1 } > > pass from <try> > > netlock# > > netlock# pfctl -sA > > test > > netlock# pfctl -a test -sT > > try > > netlock# pfctl -a test -t try -T show > > 192.168.1.1 > > > > OK to commit fix below? > > > > thanks and > > regards > > sashan > > > > [1] > > https://urldefense.com/v3/__https://marc.info/?l=openbsd-cvs&m=165222430111103&w=2__;!!ACWV5N9M2RV99hQ!LsTJPPsMku6N_u9xzJu6Tj6XpZWyLzLWPmbWr-Z-p845Y8r6LH4Ul8PyX8EmqI6alhF0JqadpBBF4mn53v-rQdY$ > > > > > > --------8<---------------8<---------------8<------------------8<-------- > > diff --git a/sys/net/pf_table.c b/sys/net/pf_table.c > > index 8315ea5dd3a..dfc49de5efe 100644 > > --- a/sys/net/pf_table.c > > +++ b/sys/net/pf_table.c > > @@ -1628,8 +1628,7 @@ pfr_add_tables(struct pfr_table *tbl, int size, int > > *nadd, int flags) > > if (r != NULL) > > continue; > > > > - q->pfrkt_rs = pf_find_or_create_ruleset( > > - q->pfrkt_root->pfrkt_anchor); > > + q->pfrkt_rs = > > pf_find_or_create_ruleset(q->pfrkt_anchor); > > /* > > * root tables are attached to main ruleset, > > * because ->pfrkt_anchor[0] == '\0' > >