svn commit: r309394 - head/sys/netpfil/pf

[Marcel Moolenaar]

> On Dec 8, 2016, at 2:48 PM, Gleb Smirnoff <glebius at FreeBSD.org <mailto:glebius at FreeBSD.org>> wrote:
> 
>  Marcel,
> 
> On Wed, Dec 07, 2016 at 05:06:08PM -0800, Marcel Moolenaar wrote:
> M> >  thanks for the fixes. While the problem with the first chunk
> M> > in pfsync_sendout() is obvious, the problem you are fixing in th
> M> > second chunk in the pfsync_delete_state() is not clear to me.
> M> > Can you please explain what scenario are you fixing there?
> M> 
> M> State updates may be pending for state being deleted. This
> M> means that the state is still sitting on either the PFSYNC_S_UPD
> M> or PFSYNC_S_UPD_C queues. What pfsync(4) does in that case is
> M> simply remove the state from those queues and add it to the
> M> PFSYNC_S_DEL queue.
> M> 
> M> But, pf(4) has already dropped the reference count for state
> M> that’s deleted and the only reference is by pfsync(4) by virtue
> M> of being on the PFSYNC_S_UPD or PFSYNC_S_UPD_C queues. When the
> M> state gets dequeued from those queues, the reference count drops
> M> to 0 and the state is deleted (read: memory freed). But the same
> M> state is subsequently added to the PFSYNC_S_DEL queue — i.e.
> M> after the memory was freed.
> 
> Thanks for explanation, Marcel! Potentially this problem also exists
> in pfsync_update_state() and in pfsync_update_state_req().
> 
> Your patch introduces extra unnecessary atomic operations, so let
> me suggest another patch. It should be applied on top of yours, and
> it also addresses pfsync_update_state() and in pfsync_update_state_req().
> 
> It isn't tested, but is pretty straightforward. 

I’ll give it a spin and commit.

-- 
Marcel Moolenaar
marcel at xcllnt.net <mailto:marcel at xcllnt.net>