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.
Cheers, Phil
