Re: CVS: cvs.openbsd.org: src

[Sasha Nedvedicky]

Hello Anton,

On Thu, Oct 22, 2020 at 08:11:41AM +0200, Anton Lindqvist wrote:
> Hi,
> 
> On Wed, Oct 21, 2020 at 02:08:05AM -0600, Alexandr Nedvedicky wrote:
> > CVSROOT:    /cvs
> > Module name:        src
> > Changes by: sas...@cvs.openbsd.org  2020/10/21 02:08:05
> > 
> > Modified files:
> >     sys/net        : pf_ioctl.c pf_osfp.c 
> > 
> > Log message:
> > - move NET_LOCK() further down in pf_ioctl.c. Also move memory allocations
> > outside of NET_LOCK()/PF_LOCK() scope in easy spots.
> > 
> > OK kn@
> 
> Did see the following panic that was found by syzkaller after this
> commit? Looks related.
> 
> https://syzkaller.appspot.com/bug?id=69aa91faa159a435b46d162e23268acd07fc8fb3

    yes, this is mine. forgotten NET_UNLOCK() in error path.
    according to reproduce patch below will make syzkaller happy.

    thanks for keeping syzkaller alive.

    I'm going to send diff below to hackers as a confession of my
    sloppiness.

regards
sashan

--------8<---------------8<---------------8<------------------8<--------
diff --git a/sys/net/pf_ioctl.c b/sys/net/pf_ioctl.c
index 3a117a7d1c7..58628519216 100644
--- a/sys/net/pf_ioctl.c
+++ b/sys/net/pf_ioctl.c
@@ -1783,16 +1783,17 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, 
struct proc *p)
                        if (state->timeout != PFTM_UNLINKED) {
                                if ((nr+1) * sizeof(*p) > ps->ps_len)
                                        break;
                                pf_state_export(pstore, state);
                                error = copyout(pstore, p, sizeof(*p));
                                if (error) {
                                        free(pstore, M_TEMP, sizeof(*pstore));
                                        PF_STATE_EXIT_READ();
+                                       NET_UNLOCK();
                                        goto fail;
                                }
                                p++;
                                nr++;
                        }
                        state = TAILQ_NEXT(state, entry_list);
                }
                PF_STATE_EXIT_READ();