From: Patrick McHardy <kaber@trash.net>
To: Pablo Neira Ayuso <pablo@netfilter.org>
Cc: Denys Fedoryshchenko <nuclearcat@nuclearcat.com>,
Netdev <netdev@vger.kernel.org>,
netfilter-devel@vger.kernel.org, stable@vger.kernel.org
Subject: Re: nft 0.4, crash on list
Date: Sun, 22 Mar 2015 19:29:45 +0000 [thread overview]
Message-ID: <20150322192944.GO7910@acer.localdomain> (raw)
In-Reply-To: <20150322192900.GA14885@salvia>
On 22.03, Pablo Neira Ayuso wrote:
> On Sun, Mar 22, 2015 at 10:05:10AM +0200, Denys Fedoryshchenko wrote:
> > On 2015-03-22 07:33, Patrick McHardy wrote:
> > >On 22.03, Denys Fedoryshchenko wrote:
> > >>Sorry for noise, seems git version working fine!
> > >
> > >Still this shouldn't be happening. Just to confirm, you were using an
> > >unpatched kernel and by git you mean nftables git?
> >
> > Yes, correct. I tested on 3.18.8 and 3.19.2 vanilla kernels (x86_64).
> > On nftables 0.4 it does crash, on nftables git it doesn't.
>
> I sent this fix to -stable by March 10th but this doesn't show up in
> 3.18.x and 3.19.x yet.
>
> [ upstream commit 02263db00b6cb98701332aa257c07ca549c2324b ]
I think this is actually a different problem. We're using set->dtype
for uninit of the element's data, but unless it's NFT_DATA_VERDICT,
its holding the user encoding of the type.
Basically all the types except NFT_DATA_RESERVED_MASK map to
NFT_DATA_VALUE, and it seems we're not properly handling it in
that path.
>
> We have several problems in this path:
>
> 1) There is a use-after-free when removing individual elements from
> the commit path.
>
> 2) We have to uninit() the data part of the element from the abort
> path to avoid a chain refcount leak.
>
> 3) We have to check for set->flags to see if there's a mapping,
> instead
> of the element flags.
>
> 4) We have to check for !(flags & NFT_SET_ELEM_INTERVAL_END) to skip
> elements that are part of the interval that have no data part, so
> they don't need to be uninit().
>
> Cc: <stable@vger.kernel.org> # 3.18.x
> Cc: <stable@vger.kernel.org> # 3.19.x
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
>
> > >>On 2015-03-22 00:49, Denys Fedoryshchenko wrote:
> > >>>Additionally, if i will do "nft flush table mangle" , with this table
> > >>>added i will get this:
> > >>>[ 42.800078] ------------[ cut here ]------------
> > >>>[ 42.800092] WARNING: CPU: 3 PID: 2868 at
> > >>>net/netfilter/nf_tables_api.c:4122 nft_data_uninit+0x35/0x50
> > >>>[nf_tables]()
>
prev parent reply other threads:[~2015-03-22 19:29 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-03-21 22:32 nft 0.4, crash on list Denys Fedoryshchenko
2015-03-21 22:49 ` Denys Fedoryshchenko
2015-03-21 23:40 ` Denys Fedoryshchenko
2015-03-22 5:33 ` Patrick McHardy
2015-03-22 8:05 ` Denys Fedoryshchenko
2015-03-22 19:29 ` Pablo Neira Ayuso
2015-03-22 19:29 ` Patrick McHardy [this message]
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=20150322192944.GO7910@acer.localdomain \
--to=kaber@trash.net \
--cc=netdev@vger.kernel.org \
--cc=netfilter-devel@vger.kernel.org \
--cc=nuclearcat@nuclearcat.com \
--cc=pablo@netfilter.org \
--cc=stable@vger.kernel.org \
/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