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.

Reply via email to