bpf: prevent out-of-bounds speculation
Subsystems (4)
Alexei Starovoitov
maintainerPATCH v3 1/1 bpf: prevent out-of-bounds speculation
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.
Unconditionally mask index for all array types even when max_entries are not rounded to power of 2 for root user.
When map is created by unpriv user generate a sequence of bpf insns that includes AND operation to make sure that JITed code includes the same 'index & index_mask' operation.
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.
v2->v3:
Daniel noticed that attack potentially can be crafted via syscall commands without loading the program, so add masking to those paths as well.
--- include/linux/bpf.h | 2 ++ kernel/bpf/arraymap.c | 47 ++++++++++++++++++++++++++++++++++++----------- kernel/bpf/verifier.c | 36 ++++++++++++++++++++++++++++++++++++ 3 files changed, 74 insertions(+), 11 deletions(-)
include/linux/bpf.h +2 -0
@@ -52,6 +52,7 @@ struct bpf_map {u32 pages;u32 id;int numa_node; +bool unpriv_array;struct user_struct *user;const struct bpf_map_ops *ops;struct work_struct work;
@@ -221,6 +222,7 @@ struct bpf_prog_aux {struct bpf_array {struct bpf_map map;u32 elem_size; +u32 index_mask;/* 'ownership' of prog_array is claimed by the first program that * is going to use this map or by the first program which FD is stored * in the map to make sure that all callers and callees have the same
kernel/bpf/arraymap.c +36 -11
125 lines not on this page · raw message
kernel/bpf/verifier.c +36 -0
@@ -1729,6 +1729,13 @@ static int check_call(struct bpf_verifier_env *env, int func_id, int insn_idx) err =check_func_arg (env,BPF_REG_2 , fn->arg2_type, &meta);if (err)return err; +if (func_id == BPF_FUNC_tail_call) { +if (meta.map_ptr ==NULL ) { +verbose (env,"verifier bug\n" ); +return -EINVAL ; + } + env->insn_aux_data[insn_idx].map_ptr = meta.map_ptr; + } err =check_func_arg (env,BPF_REG_3 , fn->arg3_type, &meta);if (err)return err;
@@ -4456,6 +4463,35 @@ static int fixup_bpf_calls(struct bpf_verifier_env *env)*/ insn->imm =0 ; insn->code =BPF_JMP |BPF_TAIL_CALL ; + +/* instead of changing every JIT dealing with tail_call +* emit two extra insns: +* if (index >= max_entries) goto out; +* index &= array->index_mask; +* to avoid out-of-bounds cpu speculation +*/ + map_ptr = env->insn_aux_data[i + delta].map_ptr; +if (map_ptr ==BPF_MAP_PTR_POISON ) { +verbose (env,"tail_call obusing map_ptr\n" ); +return -EINVAL ; + } +if (!map_ptr->unpriv_array) +continue ; + insn_buf[0 ] =BPF_JMP_IMM (BPF_JGE ,BPF_REG_3 , + map_ptr->max_entries,2 ); + insn_buf[1 ] =BPF_ALU32_IMM (BPF_AND ,BPF_REG_3 , +container_of (map_ptr, + struct bpf_array, + map)->index_mask); + insn_buf[2 ] = *insn; + cnt =3 ; + new_prog =bpf_patch_insn_data (env, i + delta, insn_buf, cnt); +if (!new_prog) +return -ENOMEM ; + + delta += cnt -1 ; + env->prog = prog = new_prog; + insn = new_prog->insnsi + i + delta;continue ; }
Signature · 1 line
2.9.5
Quoted 31 lines · 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.
Unconditionally mask index for all array types even when
max_entriesare not rounded to power of 2 for root user.
When map is created by unpriv user generate a sequence of bpf insns that includes AND operation to make sure that JITed code includes the same 'index &index_mask' operation.If
prog_arraymap is created by unpriv user replace
bpf_tail_call(ctx, map, index);
withif (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.
Quoted 6 lines
v2->v3:
Daniel noticed that attack potentially can be crafted via syscall commands without loading the program, so add masking to those paths as well.Signed-off-by Alexei Starovoitov <[email protected]>Acked-by John Fastabend <[email protected]>
Applied to bpf tree, thanks Alexei!