bpf: Fix pointer arithmetic mask tightening under state pruning

commit e042aa532c84d18ff13291d00620502ce7a38dda upstream.

In 7fedb63a8307 ("bpf: Tighten speculative pointer arithmetic mask") we
narrowed the offset mask for unprivileged pointer arithmetic in order to
mitigate a corner case where in the speculative domain it is possible to
advance, for example, the map value pointer by up to value_size-1 out-of-
bounds in order to leak kernel memory via side-channel to user space.

The verifier's state pruning for scalars leaves one corner case open
where in the first verification path R_x holds an unknown scalar with an
aux->alu_limit of e.g. 7, and in a second verification path that same
register R_x, here denoted as R_x', holds an unknown scalar which has
tighter bounds and would thus satisfy range_within(R_x, R_x') as well as
tnum_in(R_x, R_x') for state pruning, yielding an aux->alu_limit of 3:
Given the second path fits the register constraints for pruning, the final
generated mask from aux->alu_limit will remain at 7. While technically
not wrong for the non-speculative domain, it would however be possible
to craft similar cases where the mask would be too wide as in 7fedb63a8307.

One way to fix it is to detect the presence of unknown scalar map pointer
arithmetic and force a deeper search on unknown scalars to ensure that
we do not run into a masking mismatch.

Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This commit is contained in:
Daniel Borkmann 2021-07-16 09:18:21 +00:00 committed by Greg Kroah-Hartman
parent ffb9d5c48b
commit be561c0154
2 changed files with 18 additions and 10 deletions

View File

@ -397,6 +397,7 @@ struct bpf_verifier_env {
struct bpf_map *used_maps[MAX_USED_MAPS]; /* array of map's used by eBPF program */ struct bpf_map *used_maps[MAX_USED_MAPS]; /* array of map's used by eBPF program */
u32 used_map_cnt; /* number of used maps */ u32 used_map_cnt; /* number of used maps */
u32 id_gen; /* used to generate unique reg IDs */ u32 id_gen; /* used to generate unique reg IDs */
bool explore_alu_limits;
bool allow_ptr_leaks; bool allow_ptr_leaks;
bool allow_uninit_stack; bool allow_uninit_stack;
bool allow_ptr_to_map_access; bool allow_ptr_to_map_access;

View File

@ -5792,6 +5792,12 @@ static int sanitize_ptr_alu(struct bpf_verifier_env *env,
alu_state |= off_is_imm ? BPF_ALU_IMMEDIATE : 0; alu_state |= off_is_imm ? BPF_ALU_IMMEDIATE : 0;
alu_state |= ptr_is_dst_reg ? alu_state |= ptr_is_dst_reg ?
BPF_ALU_SANITIZE_SRC : BPF_ALU_SANITIZE_DST; BPF_ALU_SANITIZE_SRC : BPF_ALU_SANITIZE_DST;
/* Limit pruning on unknown scalars to enable deep search for
* potential masking differences from other program paths.
*/
if (!off_is_imm)
env->explore_alu_limits = true;
} }
err = update_alu_sanitation_state(aux, alu_state, alu_limit); err = update_alu_sanitation_state(aux, alu_state, alu_limit);
@ -9088,8 +9094,8 @@ static void clean_live_states(struct bpf_verifier_env *env, int insn,
} }
/* Returns true if (rold safe implies rcur safe) */ /* Returns true if (rold safe implies rcur safe) */
static bool regsafe(struct bpf_reg_state *rold, struct bpf_reg_state *rcur, static bool regsafe(struct bpf_verifier_env *env, struct bpf_reg_state *rold,
struct bpf_id_pair *idmap) struct bpf_reg_state *rcur, struct bpf_id_pair *idmap)
{ {
bool equal; bool equal;
@ -9115,6 +9121,8 @@ static bool regsafe(struct bpf_reg_state *rold, struct bpf_reg_state *rcur,
return false; return false;
switch (rold->type) { switch (rold->type) {
case SCALAR_VALUE: case SCALAR_VALUE:
if (env->explore_alu_limits)
return false;
if (rcur->type == SCALAR_VALUE) { if (rcur->type == SCALAR_VALUE) {
if (!rold->precise && !rcur->precise) if (!rold->precise && !rcur->precise)
return true; return true;
@ -9204,9 +9212,8 @@ static bool regsafe(struct bpf_reg_state *rold, struct bpf_reg_state *rcur,
return false; return false;
} }
static bool stacksafe(struct bpf_func_state *old, static bool stacksafe(struct bpf_verifier_env *env, struct bpf_func_state *old,
struct bpf_func_state *cur, struct bpf_func_state *cur, struct bpf_id_pair *idmap)
struct bpf_id_pair *idmap)
{ {
int i, spi; int i, spi;
@ -9251,9 +9258,8 @@ static bool stacksafe(struct bpf_func_state *old,
continue; continue;
if (old->stack[spi].slot_type[0] != STACK_SPILL) if (old->stack[spi].slot_type[0] != STACK_SPILL)
continue; continue;
if (!regsafe(&old->stack[spi].spilled_ptr, if (!regsafe(env, &old->stack[spi].spilled_ptr,
&cur->stack[spi].spilled_ptr, &cur->stack[spi].spilled_ptr, idmap))
idmap))
/* when explored and current stack slot are both storing /* when explored and current stack slot are both storing
* spilled registers, check that stored pointers types * spilled registers, check that stored pointers types
* are the same as well. * are the same as well.
@ -9310,10 +9316,11 @@ static bool func_states_equal(struct bpf_verifier_env *env, struct bpf_func_stat
memset(env->idmap_scratch, 0, sizeof(env->idmap_scratch)); memset(env->idmap_scratch, 0, sizeof(env->idmap_scratch));
for (i = 0; i < MAX_BPF_REG; i++) for (i = 0; i < MAX_BPF_REG; i++)
if (!regsafe(&old->regs[i], &cur->regs[i], env->idmap_scratch)) if (!regsafe(env, &old->regs[i], &cur->regs[i],
env->idmap_scratch))
return false; return false;
if (!stacksafe(old, cur, env->idmap_scratch)) if (!stacksafe(env, old, cur, env->idmap_scratch))
return false; return false;
if (!refsafe(old, cur)) if (!refsafe(old, cur))