From: Will Deacon <will.deacon@arm.com>
To: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: Rabin Vincent <rabin@rab.in>,
davem@davemloft.net, netdev@vger.kernel.org, zlim.lnx@gmail.com,
yang.shi@linaro.org, catalin.marinas@arm.com,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] arm64: net: bpf: don't BUG() on large shifts
Date: Tue, 12 Jan 2016 17:17:10 +0000 [thread overview]
Message-ID: <20160112171710.GK15737@arm.com> (raw)
In-Reply-To: <20160108190943.GA11561@ast-mbp.thefacebook.com>
On Fri, Jan 08, 2016 at 11:09:44AM -0800, Alexei Starovoitov wrote:
> On Fri, Jan 08, 2016 at 03:44:23PM +0000, Will Deacon wrote:
> > On Tue, Jan 05, 2016 at 06:39:03PM +0100, Rabin Vincent wrote:
> > > Attempting to generate UBFM/SBFM instructions with shifts that can't be
> > > encoded in the immediate fields of the opcodes leads to a trigger of a
> > > BUG() in the instruction generation code. As the ARMv8 ARM says: "The
> > > shift amounts must be in the range 0 to one less than the register width
> > > of the instruction, inclusive." Make the JIT reject unencodable shifts
> > > instead of crashing.
> >
> > I moaned about those BUG_ONs when they were introduced:
> >
> > https://lkml.org/lkml/2014/7/17/438
> >
> > The response then was that the verifier would catch these issues so
> > there was nothing to worry about. Has something changed so that is no
> > longer the case? Do we need to consider a different way of rejecting
> > invalid instructions at the encoding stage rather than bringing down the
> > kernel?
>
> that discussion lead to replacement of all BUG_ONs in
> arch/arm64/net/bpf_jit_comp.c with pr_err_once(), but looks like
> arch/arm64/kernel/insn.c wasn't addressed.
> The amount of BUG_ONs there is indeed overkill regardless of what
> verifier and other JITs do. btw, x64 JIT doesn't have runtime BUG_ONs.
Maybe, but insn.c is also used by the alternatives patching code, so we
really need a way to communicate failure back to the BPF JIT when passed
an invalid instruction description.
Will
next prev parent reply other threads:[~2016-01-12 17:17 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-01-05 17:39 [PATCH] arm64: net: bpf: don't BUG() on large shifts Rabin Vincent
2016-01-05 17:55 ` Alexei Starovoitov
2016-01-06 20:31 ` Rabin Vincent
2016-01-06 22:12 ` Alexei Starovoitov
2016-01-07 11:07 ` David Laight
2016-01-07 12:48 ` Daniel Borkmann
2016-01-08 15:58 ` Rabin Vincent
2016-01-08 16:44 ` Daniel Borkmann
2016-01-08 19:18 ` Alexei Starovoitov
2016-01-08 15:44 ` Will Deacon
2016-01-08 19:09 ` Alexei Starovoitov
2016-01-12 17:17 ` Will Deacon [this message]
2016-01-12 19:23 ` Alexei Starovoitov
2016-01-13 4:45 ` Z Lim
2016-01-13 12:08 ` Will Deacon
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=20160112171710.GK15737@arm.com \
--to=will.deacon@arm.com \
--cc=alexei.starovoitov@gmail.com \
--cc=catalin.marinas@arm.com \
--cc=davem@davemloft.net \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=netdev@vger.kernel.org \
--cc=rabin@rab.in \
--cc=yang.shi@linaro.org \
--cc=zlim.lnx@gmail.com \
/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).