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;
> + }
>
> NET_LOCK();
> PF_LOCK();
The diff is obviously correct. Those are the only return statements in
that part of pfioctl() and it is clear the must be replaced by goto fail;
Now the problem at hand is that pf91.in is a double anchor and I think
pfctl recursive traversal opens multiple transactions to handle this case.
So my diff is still wrong and a more complex solution needs to be found.
--
:wq Claudio
