From mboxrd@z Thu Jan  1 00:00:00 1970
From: alexei.starovoitov@gmail.com (Alexei Starovoitov)
Date: Wed, 6 Jan 2016 14:12:49 -0800
Subject: [PATCH] arm64: net: bpf: don't BUG() on large shifts
In-Reply-To: <20160106203127.GA16059@debian>
References: <1452015543-6790-1-git-send-email-rabin@rab.in>
 <20160105175557.GC83548@ast-mbp.thefacebook.com>
 <20160106203127.GA16059@debian>
Message-ID: <20160106221247.GA95332@ast-mbp.thefacebook.com>
To: linux-arm-kernel@lists.infradead.org
List-Id: linux-arm-kernel.lists.infradead.org

On Wed, Jan 06, 2016 at 09:31:27PM +0100, Rabin Vincent wrote:
> On Tue, Jan 05, 2016 at 09:55:58AM -0800, Alexei Starovoitov wrote:
> > this one is better to be addressed in verifier instead of eBPF JITs.
> > Please reject it in check_alu_op() instead.
> 
> AFAICS the eBPF verifier is not called on the eBPF filters generated by
> the BPF->eBPF conversion in net/core/filter.c, so performing this check
> only in check_alu_op() will be insufficient.  So I think we'd need to
> add this check to bpf_check_classic() too.  Or am I missing something?

correct. the check is needed in bpf_check_classic() too and I believe
it's ok to tighten it up in this case, since >32 shift is
invalid/undefined anyway. We can either accept it as nop or K&=31
or error. I think returning error is more user friendly long term, though
there is a small risk of rejecting previously loadable broken programs.