From mboxrd@z Thu Jan 1 00:00:00 1970 From: John Fastabend Subject: Re: [PATCH v2 bpf] bpf: prevent out-of-bounds speculation Date: Fri, 5 Jan 2018 11:38:42 -0800 Message-ID: <6686071e-d533-9b6d-436f-c8895fc04dec@gmail.com> References: <20180105054519.1885023-1-ast@fb.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: Daniel Borkmann , netdev@vger.kernel.org To: Alexei Starovoitov , "David S . Miller" Return-path: Received: from mail-pf0-f194.google.com ([209.85.192.194]:40306 "EHLO mail-pf0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752445AbeAETi6 (ORCPT ); Fri, 5 Jan 2018 14:38:58 -0500 Received: by mail-pf0-f194.google.com with SMTP id v26so2572239pfl.7 for ; Fri, 05 Jan 2018 11:38:58 -0800 (PST) In-Reply-To: <20180105054519.1885023-1-ast@fb.com> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 01/04/2018 09:45 PM, Alexei Starovoitov wrote: > From: Alexei Starovoitov > > Under speculation, CPUs may mis-predict branches in bounds checks. Thus, > memory accesses under a bounds check may be speculated even if the > bounds check fails, providing a primitive for building a side channel. > > To avoid leaking kernel data round up array-based maps and mask the index > after bounds check, so speculated load with out of bounds index will load > either valid value from the array or zero from the padded area. > > To avoid duplicating map_lookup functions for root/unpriv always generate > a sequence of bpf instructions equivalent to map_lookup function for > array and array_of_maps map types when map was created by unpriv user. > And unconditionally mask index for percpu_array, since it's fast enough, > even when max_entries are not rounded to power of 2 for root user, > since percpu_array doesn't have map_gen_lookup callback yet. > > If prog_array map is created by unpriv user replace > bpf_tail_call(ctx, map, index); > with > if (index >= max_entries) { > index &= map->index_mask; > bpf_tail_call(ctx, map, index); > } > (along with roundup to power 2) to prevent out-of-bounds speculation. > There is secondary redundant 'if (index >= max_entries)' in the interpreter > and in all JITs, but they can be optimized later if necessary. > > Other array-like maps (cpumap, devmap, sockmap, perf_event_array, cgroup_array) > cannot be used by unpriv, so no changes there. > > That fixes bpf side of "Variant 1: bounds check bypass (CVE-2017-5753)" on > all architectures with and without JIT. > > Signed-off-by: Alexei Starovoitov > --- LGTM, I'll drop it on my test systems and start running with it. Although I don't have any Variant 1 code to test, but seems that is being covered by others. Thanks! Acked-by: John Fastabend