> On 6 Jun 2022, at 18:08, Claudio Jeker <cje...@diehard.n-r-g.com> wrote:
>
> On Mon, Jun 06, 2022 at 12:03:06AM +0200, Alexandr Nedvedicky wrote:
>> Hello,
>>
>> I'll commit one-liner diff on Tuesday morning (Jun 6th) Prague time without
>> explicit OK, unless there will be no objection.
>
> The diff is OK claudio@.
ok by me too.
>
>> 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'
>>>>
>>
>
> --
> :wq Claudio
