On Thu, Jan 12, 2017 at 5:20 PM, Tobias Klauser <tklau...@distanz.ch> wrote: > On 2017-01-12 at 15:54:31 +0100, Vadim Kochan <vadi...@gmail.com> wrote: >> On Thu, Jan 12, 2017 at 4:28 PM, Tobias Klauser <tklau...@distanz.ch> wrote: > [...] >> >> enum flow_direction { >> >> @@ -355,15 +357,15 @@ static inline struct flow_entry >> >> *flow_entry_xalloc(void) >> >> static inline void flow_entry_xfree(struct flow_entry *n) >> >> { >> >> if (n->ct) >> >> - nfct_destroy(n->ct); >> >> + xfree(n->ct); >> > >> > This would leak memory allocated internally in struct nf_contrack, no? >> > What's the reason for this change? >> >> nfct_destroy(ct) may fail if we free entry after nfct event processing >> (our free is defered now because of RCU), >> so using just free(x) is safer because we do just nfct_clone(ct) which >> just allocated another nfct_conntrack entry for us, >> but ofcourse it would be better to have something nfct_free(x), so I >> agree this is not nice but safer. > > As long as it causes memory to be leaked IMO it is equally bad. > > If flow_entry_xfree() and thus nfct_destroy() is called via the > call_rcu() wrapper I proposed, it should no longer be unsafe to call > nfct_destroy()
Hm, I just looked at the libnetfilter_conntrack sources callback.c, and I see that I need to return NFCT_CB_STOLEN in case if I want to manually destroy the entry, otherwise nfct API will do it automatically after event handler (callback which was registered from flowtop.c) execution. -- You received this message because you are subscribed to the Google Groups "netsniff-ng" group. To unsubscribe from this group and stop receiving emails from it, send an email to netsniff-ng+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.