From: Florian Westphal <fw@strlen.de>
To: Pablo Neira Ayuso <pablo@netfilter.org>
Cc: Florian Westphal <fw@strlen.de>,
netfilter-devel@vger.kernel.org, kaber@trash.net
Subject: Re: nft: meta l4proto range printing broken on 32bit
Date: Thu, 16 Jul 2015 22:29:49 +0200 [thread overview]
Message-ID: <20150716202949.GM25674@breakpoint.cc> (raw)
In-Reply-To: <20150716165437.GA6590@salvia>
Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Thu, Jul 16, 2015 at 06:54:05PM +0200, Pablo Neira Ayuso wrote:
> > On Thu, Jul 16, 2015 at 03:39:29PM +0200, Florian Westphal wrote:
> > > Hi Pablo
> > >
> > > 09565a4b1ed4863d44c4509a93c50f44efd12771
> > > (netlink_delinearize: consolidate range printing) causes nft
> > > to segfault on 32bit machine when printing l4proto ranges.
> > >
> > > The problem is that meta_expr_pctx_update() assumes that
> > > right is a value, but after this change it can also be a value.
> > >
> > > Thus, expr->value contents are undefined (its union).
> > > On x86_64 this is also broken but by virtue of struct layout
> > > and pointer sizes, value->_mp_size will almost always be 0
> > > so mpz_get_uint8() returns 0.
> > >
> > > But on x86-32 _mp_size will be huge value (contains expr->right pointer
> > > of range), so we crash in libgmp.
> >
> > Good catch. Could you give a try to this patch instead?
> >
> > We shouldn't call pctx_update(), before the transformation we had
> > there a expr->op == { OP_GT, OP_GTE, OP_LT, OP_LTE }. So we never
> > entered that path as the assert in payload_expr_pctx_update()
> > indicates.
>
> Forgot patch, here it comes.
> diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
> index 6d60be3..4226b82 100644
> --- a/src/netlink_delinearize.c
> +++ b/src/netlink_delinearize.c
> @@ -983,6 +983,9 @@ static void meta_match_postprocess(struct rule_pp_ctx *ctx,
>
> switch (expr->op) {
> case OP_EQ:
> + if (expr->right->ops->type == EXPR_RANGE)
> + break;
> +
> expr->left->ops->pctx_update(&ctx->pctx, expr);
>
Right thats better -- and it also avoids the crash.
nft testsuite now passes with a few warnings and no crashes.
Thanks!
prev parent reply other threads:[~2015-07-16 20:29 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-07-16 13:39 nft: meta l4proto range printing broken on 32bit Florian Westphal
2015-07-16 13:42 ` Florian Westphal
2015-07-16 16:54 ` Pablo Neira Ayuso
2015-07-16 16:54 ` Pablo Neira Ayuso
2015-07-16 20:29 ` Florian Westphal [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=20150716202949.GM25674@breakpoint.cc \
--to=fw@strlen.de \
--cc=kaber@trash.net \
--cc=netfilter-devel@vger.kernel.org \
--cc=pablo@netfilter.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.