For the now raised REASON_STACK, this allows us to later fall back to
nospec for certain errors from push_stack() if we are on a speculative
path.
Fall back to nospec_result directly for the remaining sanitization errs
even if we are not on a speculative path. We must prevent a following
mem-access from using the result of the alu op speculatively. Therefore,
insert a nospec after the alu insn.
The latter requires us to modify the nospec_result patching code to work
not only for write-type insns.
Signed-off-by: Luis Gerhorst <luis.gerho...@fau.de>
Acked-by: Henriette Herzog <henriette.her...@rub.de>
Cc: Maximilian Ott <o...@cs.fau.de>
Cc: Milan Stephan <milan.step...@fau.de>
---
kernel/bpf/verifier.c | 122 +++++++++++++++---------------------------
1 file changed, 42 insertions(+), 80 deletions(-)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 406294bcd5ce..033780578966 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -13572,14 +13572,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)
{
@@ -13602,11 +13594,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 -ENOTSUPP;
}
+ /* Register tried access beyond pointer bounds. */
if (ptr_limit >= max)
- return REASON_LIMIT;
+ return -ENOTSUPP;
*alu_limit = ptr_limit;
return 0;
}
@@ -13625,8 +13619,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_result = true;
+ aux->alu_state = 0;
+ return 0;
+ }
/* Corresponding fixup done in do_misc_fixups(). */
aux->alu_state = alu_state;
@@ -13707,16 +13705,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_result = 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 != -ENOTSUPP);
+ aux->nospec_result = true;
+ aux->alu_state = 0;
+ return 0;
+ }
if (commit_window) {
/* In commit phase we narrow the masking window based on
@@ -13769,7 +13775,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)
@@ -13785,45 +13791,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.
*
@@ -13989,7 +13956,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) {
@@ -14117,7 +14084,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;
@@ -14711,7 +14678,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.
@@ -20515,6 +20482,22 @@ static int convert_ctx_accesses(struct
bpf_verifier_env *env)
*/
}
+ if (env->insn_aux_data[i + delta].nospec_result) {
+ struct bpf_insn patch[] = {
+ *insn,
+ BPF_ST_NOSPEC(),
+ };
+
+ cnt = ARRAY_SIZE(patch);
+ new_prog = bpf_patch_insn_data(env, i + delta, patch,
cnt);
+ if (!new_prog)
+ return -ENOMEM;
+
+ delta += cnt - 1;
+ env->prog = new_prog;
+ insn = new_prog->insnsi + i + delta;
+ }
+
if (insn->code == (BPF_LDX | BPF_MEM | BPF_B) ||
insn->code == (BPF_LDX | BPF_MEM | BPF_H) ||
insn->code == (BPF_LDX | BPF_MEM | BPF_W) ||
@@ -20561,27 +20544,6 @@ static int convert_ctx_accesses(struct
bpf_verifier_env *env)
continue;
}
- if (type == BPF_WRITE &&
- env->insn_aux_data[i + delta].nospec_result) {
- /* nospec_result is only used to mitigate Spectre v4 and
- * to limit verification-time for Spectre v1.
- */
- struct bpf_insn patch[] = {
- *insn,
- BPF_ST_NOSPEC(),
- };
-
- cnt = ARRAY_SIZE(patch);
- new_prog = bpf_patch_insn_data(env, i + delta, patch,
cnt);
- if (!new_prog)
- return -ENOMEM;
-
- delta += cnt - 1;
- env->prog = new_prog;
- insn = new_prog->insnsi + i + delta;
- continue;
- }
-
switch ((int)env->insn_aux_data[i + delta].ptr_type) {
case PTR_TO_CTX:
if (!ops->convert_ctx_access)
--
2.48.1
