Re: [OOPS PATCH 1/1] netfilter: fix OOPS in flush_expectations()

[Patrick McHardy <kaber@trash.net>]

On Fri, Oct 11, 2013 at 04:53:47PM +0200, Holger Eitzenberger wrote:
> 
> > > Which is due to nf_conntrack_expect.lnode hlist entry not being reset
> > > to NULL after being removed from the list in hlist_del(), but instead to
> > > LIST_POISON1.  And because of this hlist_for_each_entry_safe() does
> > > not terminate correctly.
> > > 
> > > Therefore change nf_ct_unlink_expect_report() to use __hlist_del()
> > > instead.
> > 
> > We should be holding the conntrack lock here and in flush_expectations(),
> > Not sure what I'm missing here, but if locking were used correctly, this
> > shouldn't be happening.
> 
> My first impression was that it is something locking related, so I first
> looked at usage of nf_conntrack_lock.  But I didn't find anything.  So
> my understanding is that usage of nf_conntrack_lock is correct.

Well, it has to be, otherwise we couldn't be hitting the seeing
the element in flush_expectations() with LIST_POISON1.

> Still, I think that using hlist_del() together with those
> loop iterators expecting NULL-ness is a dangerous thing to do.

I disagree, its perfectly fine if done correctly. This is just papering
over the underlying issue, which is apparently missing in something 
invoking nf_ct_unlink_expect_report().