ALU sanitization was introduced to ensure that a subsequent ptr access can never go OOBs, even under speculation. This is required because we currently allow speculative scalar confusion. This is the case because Spectre v4 sanitization only adds a nospec after critical stores (e.g., scalar overwritten with a pointer).
If we add a nospec before the ALU op, none of the operands can be subject to scalar confusion. As an ADD/SUB can not introduce scalar confusion itself, the result will also not be subject to scalar confusion. Therefore, the subsequent ptr access is always safe.
For REASON_STACK, we raise the error from push_stack() directly now. This effectively only changes the error code from EACCES to ENOMEM (which arguably also fits). Raising the push_stack() error allows a future commit to fall back to nospec for certain errors from push_stack() if on a speculative path.
Fall back to nospec directly for the remaining sanitization errs even if we are not on a speculative path.
Signed-off-by: Luis Gerhorst luis.gerhorst@fau.de Acked-by: Henriette Herzog henriette.herzog@rub.de Cc: Maximilian Ott ott@cs.fau.de Cc: Milan Stephan milan.stephan@fau.de --- kernel/bpf/verifier.c | 85 ++++++------------- .../selftests/bpf/progs/verifier_bounds.c | 5 +- .../bpf/progs/verifier_bounds_deduction.c | 43 ++++++---- .../selftests/bpf/progs/verifier_map_ptr.c | 12 ++- .../bpf/progs/verifier_value_ptr_arith.c | 44 ++++++---- 5 files changed, 92 insertions(+), 97 deletions(-)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 610f9567af7c..03af82f52a02 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -13809,14 +13809,6 @@ static bool check_reg_sane_offset(struct bpf_verifier_env *env, return true; }
-enum { - REASON_BOUNDS = -1, - REASON_TYPE = -2, - REASON_PATHS = -3, - REASON_LIMIT = -4, - REASON_STACK = -5, -}; - static int retrieve_ptr_limit(const struct bpf_reg_state *ptr_reg, u32 *alu_limit, bool mask_to_left) { @@ -13839,11 +13831,13 @@ static int retrieve_ptr_limit(const struct bpf_reg_state *ptr_reg, ptr_reg->umax_value) + ptr_reg->off; break; default: - return REASON_TYPE; + /* Register has pointer with unsupported alu operation. */ + return -EOPNOTSUPP; }
+ /* Register tried access beyond pointer bounds. */ if (ptr_limit >= max) - return REASON_LIMIT; + return -EOPNOTSUPP; *alu_limit = ptr_limit; return 0; } @@ -13864,8 +13858,12 @@ static int update_alu_sanitation_state(struct bpf_insn_aux_data *aux, */ if (aux->alu_state && (aux->alu_state != alu_state || - aux->alu_limit != alu_limit)) - return REASON_PATHS; + aux->alu_limit != alu_limit)) { + /* Tried to perform alu op from different maps, paths or scalars */ + aux->nospec = true; + aux->alu_state = 0; + return 0; + }
/* Corresponding fixup done in do_misc_fixups(). */ aux->alu_state = alu_state; @@ -13946,16 +13944,24 @@ static int sanitize_ptr_alu(struct bpf_verifier_env *env,
if (!commit_window) { if (!tnum_is_const(off_reg->var_off) && - (off_reg->smin_value < 0) != (off_reg->smax_value < 0)) - return REASON_BOUNDS; + (off_reg->smin_value < 0) != (off_reg->smax_value < 0)) { + /* Register has unknown scalar with mixed signed bounds. */ + aux->nospec = true; + aux->alu_state = 0; + return 0; + }
info->mask_to_left = (opcode == BPF_ADD && off_is_neg) || (opcode == BPF_SUB && !off_is_neg); }
err = retrieve_ptr_limit(ptr_reg, &alu_limit, info->mask_to_left); - if (err < 0) - return err; + if (err) { + WARN_ON_ONCE(err != -EOPNOTSUPP); + aux->nospec = true; + aux->alu_state = 0; + return 0; + }
if (commit_window) { /* In commit phase we narrow the masking window based on @@ -14008,7 +14014,7 @@ static int sanitize_ptr_alu(struct bpf_verifier_env *env, env->insn_idx); if (!ptr_is_dst_reg && !IS_ERR(branch)) *dst_reg = tmp; - return IS_ERR(branch) ? REASON_STACK : 0; + return PTR_ERR_OR_ZERO(branch); }
static void sanitize_mark_insn_seen(struct bpf_verifier_env *env) @@ -14024,45 +14030,6 @@ static void sanitize_mark_insn_seen(struct bpf_verifier_env *env) env->insn_aux_data[env->insn_idx].seen = env->pass_cnt; }
-static int sanitize_err(struct bpf_verifier_env *env, - const struct bpf_insn *insn, int reason, - const struct bpf_reg_state *off_reg, - const struct bpf_reg_state *dst_reg) -{ - static const char *err = "pointer arithmetic with it prohibited for !root"; - const char *op = BPF_OP(insn->code) == BPF_ADD ? "add" : "sub"; - u32 dst = insn->dst_reg, src = insn->src_reg; - - switch (reason) { - case REASON_BOUNDS: - verbose(env, "R%d has unknown scalar with mixed signed bounds, %s\n", - off_reg == dst_reg ? dst : src, err); - break; - case REASON_TYPE: - verbose(env, "R%d has pointer with unsupported alu operation, %s\n", - off_reg == dst_reg ? src : dst, err); - break; - case REASON_PATHS: - verbose(env, "R%d tried to %s from different maps, paths or scalars, %s\n", - dst, op, err); - break; - case REASON_LIMIT: - verbose(env, "R%d tried to %s beyond pointer bounds, %s\n", - dst, op, err); - break; - case REASON_STACK: - verbose(env, "R%d could not be pushed for speculative verification, %s\n", - dst, err); - break; - default: - verbose(env, "verifier internal error: unknown reason (%d)\n", - reason); - break; - } - - return -EACCES; -} - /* check that stack access falls within stack limits and that 'reg' doesn't * have a variable offset. * @@ -14228,7 +14195,7 @@ static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env, ret = sanitize_ptr_alu(env, insn, ptr_reg, off_reg, dst_reg, &info, false); if (ret < 0) - return sanitize_err(env, insn, ret, off_reg, dst_reg); + return ret; }
switch (opcode) { @@ -14356,7 +14323,7 @@ static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env, ret = sanitize_ptr_alu(env, insn, dst_reg, off_reg, dst_reg, &info, true); if (ret < 0) - return sanitize_err(env, insn, ret, off_reg, dst_reg); + return ret; }
return 0; @@ -14950,7 +14917,7 @@ static int adjust_scalar_min_max_vals(struct bpf_verifier_env *env, if (sanitize_needed(opcode)) { ret = sanitize_val_alu(env, insn); if (ret < 0) - return sanitize_err(env, insn, ret, NULL, NULL); + return ret; }
/* Calculate sign/unsigned bounds and tnum for alu32 and alu64 bit ops. diff --git a/tools/testing/selftests/bpf/progs/verifier_bounds.c b/tools/testing/selftests/bpf/progs/verifier_bounds.c index 0488ef05a7a4..ad405a609db4 100644 --- a/tools/testing/selftests/bpf/progs/verifier_bounds.c +++ b/tools/testing/selftests/bpf/progs/verifier_bounds.c @@ -47,9 +47,12 @@ SEC("socket") __description("subtraction bounds (map value) variant 2") __failure __msg("R0 min value is negative, either use unsigned index or do a if (index >=0) check.") -__msg_unpriv("R1 has unknown scalar with mixed signed bounds") +__msg_unpriv("R0 pointer arithmetic of map value goes out of range, prohibited for !root") __naked void bounds_map_value_variant_2(void) { + /* unpriv: nospec inserted to prevent "R1 has unknown scalar with mixed + * signed bounds". + */ asm volatile (" \ r1 = 0; \ *(u64*)(r10 - 8) = r1; \ diff --git a/tools/testing/selftests/bpf/progs/verifier_bounds_deduction.c b/tools/testing/selftests/bpf/progs/verifier_bounds_deduction.c index c506afbdd936..c2b5099fa208 100644 --- a/tools/testing/selftests/bpf/progs/verifier_bounds_deduction.c +++ b/tools/testing/selftests/bpf/progs/verifier_bounds_deduction.c @@ -8,21 +8,21 @@ SEC("socket") __description("check deducing bounds from const, 1") __failure __msg("R0 tried to subtract pointer from scalar") -__msg_unpriv("R1 has pointer with unsupported alu operation") +__failure_unpriv __naked void deducing_bounds_from_const_1(void) { asm volatile (" \ r0 = 1; \ if r0 s>= 1 goto l0_%=; \ -l0_%=: r0 -= r1; \ +l0_%=: /* unpriv: nospec (inserted to prevent `R1 has pointer with unsupported alu operation`) */\ + r0 -= r1; \ exit; \ " ::: __clobber_all); }
SEC("socket") __description("check deducing bounds from const, 2") -__success __failure_unpriv -__msg_unpriv("R1 has pointer with unsupported alu operation") +__success __success_unpriv __retval(1) __naked void deducing_bounds_from_const_2(void) { @@ -32,7 +32,8 @@ __naked void deducing_bounds_from_const_2(void) exit; \ l0_%=: if r0 s<= 1 goto l1_%=; \ exit; \ -l1_%=: r1 -= r0; \ +l1_%=: /* unpriv: nospec (inserted to prevent `R1 has pointer with unsupported alu operation`) */\ + r1 -= r0; \ exit; \ " ::: __clobber_all); } @@ -40,21 +41,21 @@ l1_%=: r1 -= r0; \ SEC("socket") __description("check deducing bounds from const, 3") __failure __msg("R0 tried to subtract pointer from scalar") -__msg_unpriv("R1 has pointer with unsupported alu operation") +__failure_unpriv __naked void deducing_bounds_from_const_3(void) { asm volatile (" \ r0 = 0; \ if r0 s<= 0 goto l0_%=; \ -l0_%=: r0 -= r1; \ +l0_%=: /* unpriv: nospec (inserted to prevent `R1 has pointer with unsupported alu operation`) */\ + r0 -= r1; \ exit; \ " ::: __clobber_all); }
SEC("socket") __description("check deducing bounds from const, 4") -__success __failure_unpriv -__msg_unpriv("R6 has pointer with unsupported alu operation") +__success __success_unpriv __retval(0) __naked void deducing_bounds_from_const_4(void) { @@ -65,7 +66,8 @@ __naked void deducing_bounds_from_const_4(void) exit; \ l0_%=: if r0 s>= 0 goto l1_%=; \ exit; \ -l1_%=: r6 -= r0; \ +l1_%=: /* unpriv: nospec (inserted to prevent `R6 has pointer with unsupported alu operation`) */\ + r6 -= r0; \ exit; \ " ::: __clobber_all); } @@ -73,12 +75,13 @@ l1_%=: r6 -= r0; \ SEC("socket") __description("check deducing bounds from const, 5") __failure __msg("R0 tried to subtract pointer from scalar") -__msg_unpriv("R1 has pointer with unsupported alu operation") +__failure_unpriv __naked void deducing_bounds_from_const_5(void) { asm volatile (" \ r0 = 0; \ if r0 s>= 1 goto l0_%=; \ + /* unpriv: nospec (inserted to prevent `R1 has pointer with unsupported alu operation`) */\ r0 -= r1; \ l0_%=: exit; \ " ::: __clobber_all); @@ -87,14 +90,15 @@ l0_%=: exit; \ SEC("socket") __description("check deducing bounds from const, 6") __failure __msg("R0 tried to subtract pointer from scalar") -__msg_unpriv("R1 has pointer with unsupported alu operation") +__failure_unpriv __naked void deducing_bounds_from_const_6(void) { asm volatile (" \ r0 = 0; \ if r0 s>= 0 goto l0_%=; \ exit; \ -l0_%=: r0 -= r1; \ +l0_%=: /* unpriv: nospec (inserted to prevent `R1 has pointer with unsupported alu operation`) */\ + r0 -= r1; \ exit; \ " ::: __clobber_all); } @@ -102,14 +106,15 @@ l0_%=: r0 -= r1; \ SEC("socket") __description("check deducing bounds from const, 7") __failure __msg("dereference of modified ctx ptr") -__msg_unpriv("R1 has pointer with unsupported alu operation") +__failure_unpriv __flag(BPF_F_ANY_ALIGNMENT) __naked void deducing_bounds_from_const_7(void) { asm volatile (" \ r0 = %[__imm_0]; \ if r0 s>= 0 goto l0_%=; \ -l0_%=: r1 -= r0; \ +l0_%=: /* unpriv: nospec (inserted to prevent `R1 has pointer with unsupported alu operation`) */\ + r1 -= r0; \ r0 = *(u32*)(r1 + %[__sk_buff_mark]); \ exit; \ " : @@ -121,13 +126,14 @@ l0_%=: r1 -= r0; \ SEC("socket") __description("check deducing bounds from const, 8") __failure __msg("negative offset ctx ptr R1 off=-1 disallowed") -__msg_unpriv("R1 has pointer with unsupported alu operation") +__failure_unpriv __flag(BPF_F_ANY_ALIGNMENT) __naked void deducing_bounds_from_const_8(void) { asm volatile (" \ r0 = %[__imm_0]; \ if r0 s>= 0 goto l0_%=; \ + /* unpriv: nospec (inserted to prevent `R1 has pointer with unsupported alu operation`) */\ r1 += r0; \ l0_%=: r0 = *(u32*)(r1 + %[__sk_buff_mark]); \ exit; \ @@ -140,13 +146,14 @@ l0_%=: r0 = *(u32*)(r1 + %[__sk_buff_mark]); \ SEC("socket") __description("check deducing bounds from const, 9") __failure __msg("R0 tried to subtract pointer from scalar") -__msg_unpriv("R1 has pointer with unsupported alu operation") +__failure_unpriv __naked void deducing_bounds_from_const_9(void) { asm volatile (" \ r0 = 0; \ if r0 s>= 0 goto l0_%=; \ -l0_%=: r0 -= r1; \ +l0_%=: /* unpriv: nospec (inserted to prevent `R1 has pointer with unsupported alu operation`) */\ + r0 -= r1; \ exit; \ " ::: __clobber_all); } diff --git a/tools/testing/selftests/bpf/progs/verifier_map_ptr.c b/tools/testing/selftests/bpf/progs/verifier_map_ptr.c index 11a079145966..8d7cd5b82bbd 100644 --- a/tools/testing/selftests/bpf/progs/verifier_map_ptr.c +++ b/tools/testing/selftests/bpf/progs/verifier_map_ptr.c @@ -110,11 +110,13 @@ __naked void ptr_read_ops_field_accepted(void)
SEC("socket") __description("bpf_map_ptr: r = 0, map_ptr = map_ptr + r") -__success __failure_unpriv -__msg_unpriv("R1 has pointer with unsupported alu operation") +__success __success_unpriv __retval(0) __naked void map_ptr_map_ptr_r(void) { + /* unpriv: nospec inserted to prevent "R1 has pointer with unsupported + * alu operation". + */ asm volatile (" \ r0 = 0; \ *(u64*)(r10 - 8) = r0; \ @@ -134,11 +136,13 @@ __naked void map_ptr_map_ptr_r(void)
SEC("socket") __description("bpf_map_ptr: r = 0, r = r + map_ptr") -__success __failure_unpriv -__msg_unpriv("R0 has pointer with unsupported alu operation") +__success __success_unpriv __retval(0) __naked void _0_r_r_map_ptr(void) { + /* unpriv: nospec inserted to prevent "R1 has pointer with unsupported + * alu operation". + */ asm volatile (" \ r0 = 0; \ *(u64*)(r10 - 8) = r0; \ diff --git a/tools/testing/selftests/bpf/progs/verifier_value_ptr_arith.c b/tools/testing/selftests/bpf/progs/verifier_value_ptr_arith.c index 2acd7769bd91..d1be1b6f4674 100644 --- a/tools/testing/selftests/bpf/progs/verifier_value_ptr_arith.c +++ b/tools/testing/selftests/bpf/progs/verifier_value_ptr_arith.c @@ -41,11 +41,13 @@ struct {
SEC("socket") __description("map access: known scalar += value_ptr unknown vs const") -__success __failure_unpriv -__msg_unpriv("R1 tried to add from different maps, paths or scalars") +__success __success_unpriv __retval(1) __naked void value_ptr_unknown_vs_const(void) { + /* unpriv: nospec inserted to prevent "R1 tried to add from different + * maps, paths or scalars". + */ asm volatile (" \ r0 = *(u32*)(r1 + %[__sk_buff_len]); \ r1 = 0; \ @@ -79,11 +81,13 @@ l2_%=: r0 = 1; \
SEC("socket") __description("map access: known scalar += value_ptr const vs unknown") -__success __failure_unpriv -__msg_unpriv("R1 tried to add from different maps, paths or scalars") +__success __success_unpriv __retval(1) __naked void value_ptr_const_vs_unknown(void) { + /* unpriv: nospec inserted to prevent "R1 tried to add from different + * maps, paths or scalars". + */ asm volatile (" \ r0 = *(u32*)(r1 + %[__sk_buff_len]); \ r1 = 0; \ @@ -117,11 +121,13 @@ l2_%=: r0 = 1; \
SEC("socket") __description("map access: known scalar += value_ptr const vs const (ne)") -__success __failure_unpriv -__msg_unpriv("R1 tried to add from different maps, paths or scalars") +__success __success_unpriv __retval(1) __naked void ptr_const_vs_const_ne(void) { + /* unpriv: nospec inserted to prevent "R1 tried to add from different + * maps, paths or scalars". + */ asm volatile (" \ r0 = *(u32*)(r1 + %[__sk_buff_len]); \ r1 = 0; \ @@ -225,8 +231,7 @@ l2_%=: r0 = 1; \
SEC("socket") __description("map access: known scalar += value_ptr unknown vs unknown (lt)") -__success __failure_unpriv -__msg_unpriv("R1 tried to add from different maps, paths or scalars") +__success __success_unpriv __retval(1) __naked void ptr_unknown_vs_unknown_lt(void) { @@ -251,7 +256,8 @@ l1_%=: call %[bpf_map_lookup_elem]; \ l3_%=: r1 = 6; \ r1 = -r1; \ r1 &= 0x7; \ -l4_%=: r1 += r0; \ +l4_%=: /* unpriv: nospec (inserted to prevent `R1 tried to add from different maps, paths or scalars`) */\ + r1 += r0; \ r0 = *(u8*)(r1 + 0); \ l2_%=: r0 = 1; \ exit; \ @@ -265,11 +271,13 @@ l2_%=: r0 = 1; \
SEC("socket") __description("map access: known scalar += value_ptr unknown vs unknown (gt)") -__success __failure_unpriv -__msg_unpriv("R1 tried to add from different maps, paths or scalars") +__success __success_unpriv __retval(1) __naked void ptr_unknown_vs_unknown_gt(void) { + /* unpriv: nospec inserted to prevent "R1 tried to add from different + * maps, paths or scalars". + */ asm volatile (" \ r0 = *(u32*)(r1 + %[__sk_buff_len]); \ r1 = 0; \ @@ -398,11 +406,14 @@ l2_%=: r0 = 1; \
SEC("socket") __description("map access: mixing value pointer and scalar, 1") -__success __failure_unpriv -__msg_unpriv("R2 tried to add from different maps, paths or scalars, pointer arithmetic with it prohibited for !root") +__success __success_unpriv __retval(0) __naked void value_pointer_and_scalar_1(void) { + /* unpriv: nospec inserted to prevent "R2 tried to add from different + * maps, paths or scalars, pointer arithmetic with it prohibited for + * !root". + */ asm volatile (" \ /* load map value pointer into r0 and r2 */ \ r0 = 1; \ @@ -452,11 +463,14 @@ l4_%=: /* fake-dead code; targeted from branch A to \
SEC("socket") __description("map access: mixing value pointer and scalar, 2") -__success __failure_unpriv -__msg_unpriv("R2 tried to add from different maps, paths or scalars, pointer arithmetic with it prohibited for !root") +__success __success_unpriv __retval(0) __naked void value_pointer_and_scalar_2(void) { + /* unpriv: nospec inserted to prevent "R2 tried to add from different + * maps, paths or scalars, pointer arithmetic with it prohibited for + * !root". + */ asm volatile (" \ /* load map value pointer into r0 and r2 */ \ r0 = 1; \