> On Dec 8, 2016, at 2:48 PM, Gleb Smirnoff <gleb...@freebsd.org > <mailto:gleb...@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 mar...@xcllnt.net <mailto:mar...@xcllnt.net> _______________________________________________ svn-src-all@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
