Hello Joerg,
your patch looks OK. thank you for testing & fixing.
thanks and
regard
sashan
On Tue, Nov 26, 2019 at 10:42:42AM +0100, Joerg Goltermann wrote:
> Hello,
>
> I tested a kernel with WITH_PF_LOCK and NET_TASKQ > 1 and got a
> kernel panic during switching between the pf statics inside systat.
>
>
> I think I found the problem inside pf_ioctl.c, it looks like
> the problem was introduced with a "mechanical" patch which
> replaced all "breaks;" with "PF_UNLOCK(); break;" This seems
> to be wrong in this particular case:
>
> ...
> case DIOCGETRULESETS: {
>
> ...
> RB_FOREACH(anchor, pf_anchor_node,
> &ruleset->anchor->children)
> if (nr++ == pr->nr) {
> strlcpy(pr->name, anchor->name,
> sizeof(pr->name));
> PF_UNLOCK();
> break;
> }
> ...
> }
>
> below is a diff to fix it.
>
> - Joerg
>
>
> Index: net/pf_ioctl.c
> ===================================================================
> RCS file: /cvs/src/sys/net/pf_ioctl.c,v
> retrieving revision 1.344
> diff -u -p -r1.344 pf_ioctl.c
> --- net/pf_ioctl.c 9 May 2019 14:59:30 -0000 1.344
> +++ net/pf_ioctl.c 25 Nov 2019 21:28:41 -0000
> @@ -1980,7 +1981,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
> if (anchor->parent == NULL && nr++ == pr->nr) {
> strlcpy(pr->name, anchor->name,
> sizeof(pr->name));
> - PF_UNLOCK();
> break;
> }
> } else {
> @@ -1989,13 +1989,12 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
> if (nr++ == pr->nr) {
> strlcpy(pr->name, anchor->name,
> sizeof(pr->name));
> - PF_UNLOCK();
> break;
> }
> }
> + PF_UNLOCK();
> if (!pr->name[0])
> error = EBUSY;
> - PF_UNLOCK();
> break;
> }
>
