All of lore.kernel.org
 help / color / mirror / Atom feed
From: will.deacon@arm.com (Will Deacon)
To: linux-arm-kernel@lists.infradead.org
Subject: [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

WARNING: multiple messages have this Message-ID (diff)
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

  reply	other threads:[~2016-01-12 17:17 UTC|newest]

Thread overview: 30+ 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:39 ` Rabin Vincent
2016-01-05 17:55 ` Alexei Starovoitov
2016-01-05 17:55   ` Alexei Starovoitov
2016-01-06 20:31   ` Rabin Vincent
2016-01-06 20:31     ` Rabin Vincent
2016-01-06 22:12     ` Alexei Starovoitov
2016-01-06 22:12       ` Alexei Starovoitov
2016-01-07 11:07       ` David Laight
2016-01-07 11:07         ` David Laight
2016-01-07 12:48         ` Daniel Borkmann
2016-01-07 12:48           ` Daniel Borkmann
2016-01-08 15:58           ` Rabin Vincent
2016-01-08 15:58             ` Rabin Vincent
2016-01-08 16:44             ` Daniel Borkmann
2016-01-08 16:44               ` Daniel Borkmann
2016-01-08 19:18               ` Alexei Starovoitov
2016-01-08 19:18                 ` Alexei Starovoitov
2016-01-08 15:44 ` Will Deacon
2016-01-08 15:44   ` Will Deacon
2016-01-08 19:09   ` Alexei Starovoitov
2016-01-08 19:09     ` Alexei Starovoitov
2016-01-12 17:17     ` Will Deacon [this message]
2016-01-12 17:17       ` Will Deacon
2016-01-12 19:23       ` Alexei Starovoitov
2016-01-12 19:23         ` Alexei Starovoitov
2016-01-13  4:45         ` Z Lim
2016-01-13  4:45           ` Z Lim
2016-01-13 12:08           ` Will Deacon
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=linux-arm-kernel@lists.infradead.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.