netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Pablo Neira Ayuso <pablo@netfilter.org>
To: Phil Sutter <phil@nwl.cc>, Florian Westphal <fw@strlen.de>,
	netfilter-devel@vger.kernel.org
Subject: Re: [PATCH nft 3/3] src: evaluate: return immediately if no op was requested
Date: Tue, 23 Jul 2019 15:52:44 +0200	[thread overview]
Message-ID: <20190723135244.x77dihvyf4nplbku@salvia> (raw)
In-Reply-To: <20190723131142.GN22661@orbyte.nwl.cc>

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@netfilter.org> 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.

  reply	other threads:[~2019-07-23 13:52 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-21  0:14 [PATCH nft 0/3] fix crash bug during rule restore Florian Westphal
2019-07-21  0:14 ` [PATCH nft 1/3] src: erec: fall back to internal location if its null Florian Westphal
2019-07-21 18:46   ` Pablo Neira Ayuso
2019-07-21 18:50     ` Florian Westphal
2019-07-23 19:19   ` Pablo Neira Ayuso
2019-07-21  0:14 ` [PATCH nft 2/3] src: evaluate: don't rely on global chain ctx for error reporting Florian Westphal
2019-07-21  0:14 ` [PATCH nft 3/3] src: evaluate: return immediately if no op was requested Florian Westphal
2019-07-21 18:49   ` Pablo Neira Ayuso
2019-07-21 18:50     ` Florian Westphal
2019-07-22 21:25       ` Pablo Neira Ayuso
2019-07-23 13:11         ` Phil Sutter
2019-07-23 13:52           ` Pablo Neira Ayuso [this message]
2019-07-23 19:18   ` Pablo Neira Ayuso
2019-07-23 22:44     ` Florian Westphal

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190723135244.x77dihvyf4nplbku@salvia \
    --to=pablo@netfilter.org \
    --cc=fw@strlen.de \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=phil@nwl.cc \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).