Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote: > I already told you, your patchset works in my testbed here. > > My only doubt still here is the need for the extra bit. I don't find > the scenario that will trigger the problem yet. Some comments: Its not obvious but you should be able to produce crashes in your tests when you only look for dying-bit-not-set, however... > > OR it can mean > > > > b) 'This conntrack is being allocated/setup as new connection, the > > flag field was already cleared'. > > In this case, the conntrack is placed in the unconfirmed list or the > hashes. Not necessarily. conntracks are placed on the dying list, when refcnt reaches zero they're deleted from the dying list. But we could have picked it up right before. I *think* you're right in that we could avoid the extra bit if we hold the dying list spinlock. Then, if we pick it up before removal, dying bit is always set since no recycling can happen underneath. > either a) release the entry whose event was already delivered or b) > decrement the use counter. > > > I've found no way to tell these two conditions apart except via new bit. > > I believe the rule: "all dead conntracks have the dying bit set" > always fulfills. Looking at it again I think I need to rework the patch to use the appropriate lock during traversal instead of rcu, as the dying list manipulation routines don't use the _rcu versions for hlist manipulation anyway... I'll send a v3 soon, after doing short sanity test. Since 3.15 has been released already we should simply aim this patch for 3.17. No harm done. Thanks for reviewing and testing! -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
