Hello,
I agree this is a bug.
On Sat, Apr 18, 2026 at 08:33:44PM +0300, Atanas Vladimirov wrote:
</snip>
>
> Index: sys/net/pf_if.c
> ===================================================================
> --- sys/net/pf_if.c
> +++ sys/net/pf_if.c
> @@ -852,8 +852,17 @@ pfi_set_flags(const char *name, int flags)
> } else
> panic("%s pfi_kif_get() returned NULL\n",
> __func__);
> - } else
> + } else {
> p->pfik_flags_new = p->pfik_flags | flags;
> + /*
> + * The kif already existed (created by
> + * pfi_attach_ifnet()/pfi_attach_ifgroup()). Take a
> + * flag ref so it survives a transient disappearance
> + * of the backing ifnet/ifgroup, e.g. vmd(8) destroying
> + * and recreating tap(4) across a VM reboot.
> + */
> + if (ISSET(flags, PFI_IFLAG_SKIP) &&
> + p->pfik_flagrefs == 0)
> + pfi_kif_ref(p, PFI_KIF_REF_FLAG);
> + }
> } else {
> RB_FOREACH(p, pfi_ifhead, &pfi_ifs)
> p->pfik_flags_new = p->pfik_flags | flags;
>
however diff above triggered an assert() on one of my test boxes:
kernel diagnostic assertion "(p->pfik_flagrefs == 0) || (p->pfik_flagref
s == 1)" failed: file "/home/sashan/src.sashan/sys/net/pf_if.c", line 901
Stopped at db_enter+0x14: popq %rbp
TID PID UID PRFLAGS PFLAGS CPU COMMAND
* 30662 18016 0 0x3 0 0K pfctl
db_enter() at db_enter+0x14
panic(ffffffff82672a84) at panic+0xd5
__assert(ffffffff826aeb78,ffffffff8270f923,385,ffffffff82628d6b) at
__assert+0x
29
pfi_clear_flags(ffff80001c8d6200,100) at pfi_clear_flags+0x206
pfioctl(14900,c028445a,ffff80001c8d6200,3,ffff80001c7f14d8) at pfioctl+0xe0f
VOP_IOCTL(fffffd802b6e6cd0,c028445a,ffff80001c8d6200,3,fffffd8004ffda90,ffff800
01c7f14d8) at VOP_IOCTL+0x60
vn_ioctl(fffffd802909ebd0,c028445a,ffff80001c8d6200,ffff80001c7f14d8) at
vn_ioc
tl+0x86
sys_ioctl(ffff80001c7f14d8,ffff80001c8d63b0,ffff80001c8d6330) at
sys_ioctl+0x30
e
syscall(ffff80001c8d63b0) at syscall+0x5f9
Xsyscall() at Xsyscall+0x128
the /etc/pf.conf on that test box contained two 'set skip on' statements
for lo0 interface. Diff below avoids the assert on that box.
Taking a closer look at pfi_set_flags() function I think the branch
with RB_FOREACH() should be removed. The pfctl(8) always fills name
so that code is never reached on behalf of pfctl. Also the RB_FOREACH()
loop body suffers from the same bug we attempt to fix here.
OK ?
thanks and
regards
sashan
--------8<---------------8<---------------8<------------------8<--------
diff --git a/sys/net/pf_if.c b/sys/net/pf_if.c
index 0c6681e757e..bba7237c904 100644
--- a/sys/net/pf_if.c
+++ b/sys/net/pf_if.c
@@ -854,8 +854,22 @@ pfi_set_flags(const char *name, int flags)
} else
panic("%s pfi_kif_get() returned NULL\n",
__func__);
- } else
+ } else {
+ /*
+ * pf.conf may accidentally contain two set skip on ...
+ * statements. For example:
+ * set skip lo
+ * set skip lo
+ * We need to grab reference only when skip flag is
+ * set to avoid tripping assert pfi_clear_flags()
+ */
+ if (ISSET(flags, PFI_IFLAG_SKIP) &&
+ !ISSET(p->pfik_flags_new, PFI_IFLAG_SKIP) &&
+ !ISSET(p->pfik_flags, PFI_IFLAG_SKIP))
+ pfi_kif_ref(p, PFI_KIF_REF_FLAG);
+
p->pfik_flags_new = p->pfik_flags | flags;
+ }
} else {
RB_FOREACH(p, pfi_ifhead, &pfi_ifs)
p->pfik_flags_new = p->pfik_flags | flags;
