netdev - Re: [PATCH] arm64: net: bpf: don't BUG() on large shifts

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160112171710.GK15737@arm.com>
Date:	Tue, 12 Jan 2016 17:17:10 +0000
From:	Will Deacon <will.deacon@....com>
To:	Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc:	Rabin Vincent <rabin@....in>, davem@...emloft.net,
	netdev@...r.kernel.org, zlim.lnx@...il.com, yang.shi@...aro.org,
	catalin.marinas@....com, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH] arm64: net: bpf: don't BUG() on large shifts

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