Commit f232326f authored by Piotr Krysiuk's avatar Piotr Krysiuk Committed by Daniel Borkmann

bpf: Prohibit alu ops for pointer types not defining ptr_limit

The purpose of this patch is to streamline error propagation and in particular
to propagate retrieve_ptr_limit() errors for pointer types that are not defining
a ptr_limit such that register-based alu ops against these types can be rejected.

The main rationale is that a gap has been identified by Piotr in the existing
protection against speculatively out-of-bounds loads, for example, in case of
ctx pointers, unprivileged programs can still perform pointer arithmetic. This
can be abused to execute speculatively out-of-bounds loads without restrictions
and thus extract contents of kernel memory.

Fix this by rejecting unprivileged programs that attempt any pointer arithmetic
on unprotected pointer types. The two affected ones are pointer to ctx as well
as pointer to map. Field access to a modified ctx' pointer is rejected at a
later point in time in the verifier, and 7c696732 ("bpf: Permit map_ptr
arithmetic with opcode add and offset 0") only relevant for root-only use cases.
Risk of unprivileged program breakage is considered very low.

Fixes: 7c696732 ("bpf: Permit map_ptr arithmetic with opcode add and offset 0")
Fixes: b2157399 ("bpf: prevent out-of-bounds speculation")
Signed-off-by: default avatarPiotr Krysiuk <piotras@gmail.com>
Co-developed-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
Signed-off-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
Acked-by: default avatarAlexei Starovoitov <ast@kernel.org>
parent 8a141dd7
...@@ -5934,6 +5934,7 @@ static int sanitize_ptr_alu(struct bpf_verifier_env *env, ...@@ -5934,6 +5934,7 @@ static int sanitize_ptr_alu(struct bpf_verifier_env *env,
u32 alu_state, alu_limit; u32 alu_state, alu_limit;
struct bpf_reg_state tmp; struct bpf_reg_state tmp;
bool ret; bool ret;
int err;
if (can_skip_alu_sanitation(env, insn)) if (can_skip_alu_sanitation(env, insn))
return 0; return 0;
...@@ -5949,10 +5950,13 @@ static int sanitize_ptr_alu(struct bpf_verifier_env *env, ...@@ -5949,10 +5950,13 @@ static int sanitize_ptr_alu(struct bpf_verifier_env *env,
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;
if (retrieve_ptr_limit(ptr_reg, &alu_limit, opcode, off_is_neg)) err = retrieve_ptr_limit(ptr_reg, &alu_limit, opcode, off_is_neg);
return 0; if (err < 0)
if (update_alu_sanitation_state(aux, alu_state, alu_limit)) return err;
return -EACCES;
err = update_alu_sanitation_state(aux, alu_state, alu_limit);
if (err < 0)
return err;
do_sim: do_sim:
/* Simulate and find potential out-of-bounds access under /* Simulate and find potential out-of-bounds access under
* speculative execution from truncation as a result of * speculative execution from truncation as a result of
...@@ -6103,7 +6107,7 @@ static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env, ...@@ -6103,7 +6107,7 @@ static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env,
case BPF_ADD: case BPF_ADD:
ret = sanitize_ptr_alu(env, insn, ptr_reg, dst_reg, smin_val < 0); ret = sanitize_ptr_alu(env, insn, ptr_reg, dst_reg, smin_val < 0);
if (ret < 0) { if (ret < 0) {
verbose(env, "R%d tried to add from different maps or paths\n", dst); verbose(env, "R%d tried to add from different maps, paths, or prohibited types\n", dst);
return ret; return ret;
} }
/* We can take a fixed offset as long as it doesn't overflow /* We can take a fixed offset as long as it doesn't overflow
...@@ -6158,7 +6162,7 @@ static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env, ...@@ -6158,7 +6162,7 @@ static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env,
case BPF_SUB: case BPF_SUB:
ret = sanitize_ptr_alu(env, insn, ptr_reg, dst_reg, smin_val < 0); ret = sanitize_ptr_alu(env, insn, ptr_reg, dst_reg, smin_val < 0);
if (ret < 0) { if (ret < 0) {
verbose(env, "R%d tried to sub from different maps or paths\n", dst); verbose(env, "R%d tried to sub from different maps, paths, or prohibited types\n", dst);
return ret; return ret;
} }
if (dst_reg == off_reg) { if (dst_reg == off_reg) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment