On Wed, Jun 28, 2023 at 05:46:36PM +0200, Alexandr Nedvedicky wrote:
> Hello,
>
> it looks like we need to use goto fail instead of return.
> this is the diff I'm testing now.
>
> --------8<---------------8<---------------8<------------------8<--------
> diff --git a/sys/net/pf_ioctl.c b/sys/net/pf_ioctl.c
> index 36779cfdfd3..a51df9e6089 100644
> --- a/sys/net/pf_ioctl.c
> +++ b/sys/net/pf_ioctl.c
> @@ -1508,11 +1508,15 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int
> flags, struct proc *p)
> int i;
>
> t = pf_find_trans(minor(dev), pr->ticket);
> - if (t == NULL)
> - return (ENXIO);
> + if (t == NULL) {
> + error = ENXIO;
> + goto fail;
> + }
> KASSERT(t->pft_unit == minor(dev));
> - if (t->pft_type != PF_TRANS_GETRULE)
> - return (EINVAL);
> + if (t->pft_type != PF_TRANS_GETRULE) {
> + error = EINVAL;
> + goto fail;
> + }
That looks right in itself since pfioctl() graps pfioctl_rw early on and
these returns fail to release it in case no transaction was found.
>
> NET_LOCK();
> PF_LOCK();
> On Wed, Jun 28, 2023 at 02:38:00PM +0200, Alexander Bluhm wrote:
> > Hi,
> >
> > Since Jun 26 regress tests panic the kernel.
> >
> > panic: rw_enter: pfioctl_rw locking against myself
But I'm not sure yet that this is enough to reinstate claudio's diff as-is.
> > Stopped at db_enter+0x14: popq %rbp
> > TID PID UID PRFLAGS PFLAGS CPU COMMAND
> > * 19846 58589 0 0x2 0 1K pfctl
> > 343161 43899 0 0x2 0 2 perl
> > db_enter() at db_enter+0x14
> > panic(ffffffff820e7d9d) at panic+0xc3
> > rw_enter(ffffffff82462c60,1) at rw_enter+0x26f
> > pfioctl(24900,cd504407,ffff800000f4b000,1,ffff80002226adc0) at pfioctl+0x2da
> > VOP_IOCTL(fffffd827bfea6e0,cd504407,ffff800000f4b000,1,fffffd827f7e3bc8,ffff80002226adc0)
> > at VOP_IOCTL+0x60
> > vn_ioctl(fffffd823b841d20,cd504407,ffff800000f4b000,ffff80002226adc0) at
> > vn_ioctl+0x79
> > sys_ioctl(ffff80002226adc0,ffff800022458160,ffff8000224581c0) at
> > sys_ioctl+0x2c4
> > syscall(ffff800022458230) at syscall+0x3d4
> > Xsyscall() at Xsyscall+0x128
> > end of kernel
> > end trace frame: 0x77becbc54dd0, count: 6
> > https://www.openbsd.org/ddb.html describes the minimum info required in bug
> > reports. Insufficient info makes it difficult to find and fix bugs.
> > ddb{1}>
> >
> > Triggered by regress/sbin/pfctl
> >
> > ==== pfload ====
> > ...
> > /sbin/pfctl -o none -a regress -f - < /usr/src/regress/sbin/pfctl/pf90.in
> > /sbin/pfctl -o none -a 'regress/*' -gvvsr | sed -e
> > 's/__automatic_[0-9a-f]*_/__automatic_/g' | diff -u
> > /usr/src/regress/sbin/pfctl/pf90.loaded /dev/stdin
> > /sbin/pfctl -o none -a regress -Fr >/dev/null 2>&1
> > /sbin/pfctl -o none -a regress -f - < /usr/src/regress/sbin/pfctl/pf91.in
> > /sbin/pfctl -o none -a 'regress/*' -gvvsr | sed -e
> > 's/__automatic_[0-9a-f]*_/__automatic_/g' | diff -u
> > /usr/src/regress/sbin/pfctl/pf91.loaded /dev/stdin
> > Timeout, server ot6 not responding.
> >
> > bluhm
> >
>
