On Tue, Jul 23, 2019 at 03:11:42PM +0200, Phil Sutter wrote:
> On Mon, Jul 22, 2019 at 11:25:56PM +0200, Pablo Neira Ayuso wrote:
> > On Sun, Jul 21, 2019 at 08:50:40PM +0200, Florian Westphal wrote:
> > > Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote:
> > > > On Sun, Jul 21, 2019 at 02:14:07AM +0200, Florian Westphal wrote:
> > > > > This makes nft behave like 0.9.0 -- the ruleset
> > > > >
> > > > > flush ruleset
> > > > > table inet filter {
> > > > > }
> > > > > table inet filter {
> > > > > chain test {
> > > > > counter
> > > > > }
> > > > > }
> > > > >
> > > > > loads again without generating an error message.
> > > > > I've added a test case for this, without this it will create an error,
> > > > > and with a checkout of the 'fixes' tag we get crash.
> > > > >
> > > > > Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1351
> > > > > Fixes: e5382c0d08e3c ("src: Support intra-transaction rule references")
> > > >
> > > > This one is causing the cache corruption, right?
> > >
> > > There is no cache corruption. This patch makes us enter a code
> > > path that we did not take before.
> >
> > Sorry, I mean, this is a cache bug :-)
> >
> > cache_flush() is cheating, it sets flags to CACHE_FULL, hence this
> > enters this codepath we dit not take before. This propagates from the
> > previous logic, as a workaround.
> >
> > I made this patch, to skip the cache in case "flush ruleset" is
> > requested.
> >
> > This breaks testcases/transactions/0024rule_0, which is a recent test
> > from Phil to check for intra-transaction references, I don't know yet
> > what makes this code unhappy with my changes.
> >
> > Phil, would you help me have a look at what assumption breaks? Thanks.
>
> Sorry, I don't get it. What is happening in the first place? Florian
> writes, a lookup happens in the wrong table and it seems
> chain_evaluate() doesn't add the chain to cache. Yet I don't understand
> why given patch fixes the problem.
Just sent a patch for this.
