Skip to content

Commit 4158e5f

Browse files
borkmanngregkh
authored andcommitted
bpf: Improve verifier error messages for users
commit a6aaece upstream. Consolidate all error handling and provide more user-friendly error messages from sanitize_ptr_alu() and sanitize_val_alu(). Signed-off-by: Daniel Borkmann <daniel@iogearbox.net> Reviewed-by: John Fastabend <john.fastabend@gmail.com> Acked-by: Alexei Starovoitov <ast@kernel.org> [fllinden@amazon.com: backport to 5.4] Signed-off-by: Frank van der Linden <fllinden@amazon.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 15de0c5 commit 4158e5f

1 file changed

Lines changed: 62 additions & 22 deletions

File tree

kernel/bpf/verifier.c

Lines changed: 62 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -4263,6 +4263,14 @@ static struct bpf_insn_aux_data *cur_aux(struct bpf_verifier_env *env)
42634263
return &env->insn_aux_data[env->insn_idx];
42644264
}
42654265

4266+
enum {
4267+
REASON_BOUNDS = -1,
4268+
REASON_TYPE = -2,
4269+
REASON_PATHS = -3,
4270+
REASON_LIMIT = -4,
4271+
REASON_STACK = -5,
4272+
};
4273+
42664274
static int retrieve_ptr_limit(const struct bpf_reg_state *ptr_reg,
42674275
const struct bpf_reg_state *off_reg,
42684276
u32 *alu_limit, u8 opcode)
@@ -4274,7 +4282,7 @@ static int retrieve_ptr_limit(const struct bpf_reg_state *ptr_reg,
42744282

42754283
if (!tnum_is_const(off_reg->var_off) &&
42764284
(off_reg->smin_value < 0) != (off_reg->smax_value < 0))
4277-
return -EACCES;
4285+
return REASON_BOUNDS;
42784286

42794287
switch (ptr_reg->type) {
42804288
case PTR_TO_STACK:
@@ -4301,11 +4309,11 @@ static int retrieve_ptr_limit(const struct bpf_reg_state *ptr_reg,
43014309
}
43024310
break;
43034311
default:
4304-
return -EINVAL;
4312+
return REASON_TYPE;
43054313
}
43064314

43074315
if (ptr_limit >= max)
4308-
return -ERANGE;
4316+
return REASON_LIMIT;
43094317
*alu_limit = ptr_limit;
43104318
return 0;
43114319
}
@@ -4325,7 +4333,7 @@ static int update_alu_sanitation_state(struct bpf_insn_aux_data *aux,
43254333
if (aux->alu_state &&
43264334
(aux->alu_state != alu_state ||
43274335
aux->alu_limit != alu_limit))
4328-
return -EACCES;
4336+
return REASON_PATHS;
43294337

43304338
/* Corresponding fixup done in fixup_bpf_calls(). */
43314339
aux->alu_state = alu_state;
@@ -4398,7 +4406,46 @@ static int sanitize_ptr_alu(struct bpf_verifier_env *env,
43984406
ret = push_stack(env, env->insn_idx + 1, env->insn_idx, true);
43994407
if (!ptr_is_dst_reg && ret)
44004408
*dst_reg = tmp;
4401-
return !ret ? -EFAULT : 0;
4409+
return !ret ? REASON_STACK : 0;
4410+
}
4411+
4412+
static int sanitize_err(struct bpf_verifier_env *env,
4413+
const struct bpf_insn *insn, int reason,
4414+
const struct bpf_reg_state *off_reg,
4415+
const struct bpf_reg_state *dst_reg)
4416+
{
4417+
static const char *err = "pointer arithmetic with it prohibited for !root";
4418+
const char *op = BPF_OP(insn->code) == BPF_ADD ? "add" : "sub";
4419+
u32 dst = insn->dst_reg, src = insn->src_reg;
4420+
4421+
switch (reason) {
4422+
case REASON_BOUNDS:
4423+
verbose(env, "R%d has unknown scalar with mixed signed bounds, %s\n",
4424+
off_reg == dst_reg ? dst : src, err);
4425+
break;
4426+
case REASON_TYPE:
4427+
verbose(env, "R%d has pointer with unsupported alu operation, %s\n",
4428+
off_reg == dst_reg ? src : dst, err);
4429+
break;
4430+
case REASON_PATHS:
4431+
verbose(env, "R%d tried to %s from different maps, paths or scalars, %s\n",
4432+
dst, op, err);
4433+
break;
4434+
case REASON_LIMIT:
4435+
verbose(env, "R%d tried to %s beyond pointer bounds, %s\n",
4436+
dst, op, err);
4437+
break;
4438+
case REASON_STACK:
4439+
verbose(env, "R%d could not be pushed for speculative verification, %s\n",
4440+
dst, err);
4441+
break;
4442+
default:
4443+
verbose(env, "verifier internal error: unknown reason (%d)\n",
4444+
reason);
4445+
break;
4446+
}
4447+
4448+
return -EACCES;
44024449
}
44034450

44044451
/* Handles arithmetic on a pointer and a scalar: computes new min/max and var_off.
@@ -4480,10 +4527,9 @@ static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env,
44804527
switch (opcode) {
44814528
case BPF_ADD:
44824529
ret = sanitize_ptr_alu(env, insn, ptr_reg, off_reg, dst_reg);
4483-
if (ret < 0) {
4484-
verbose(env, "R%d tried to add from different maps, paths, or prohibited types\n", dst);
4485-
return ret;
4486-
}
4530+
if (ret < 0)
4531+
return sanitize_err(env, insn, ret, off_reg, dst_reg);
4532+
44874533
/* We can take a fixed offset as long as it doesn't overflow
44884534
* the s32 'off' field
44894535
*/
@@ -4535,10 +4581,9 @@ static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env,
45354581
break;
45364582
case BPF_SUB:
45374583
ret = sanitize_ptr_alu(env, insn, ptr_reg, off_reg, dst_reg);
4538-
if (ret < 0) {
4539-
verbose(env, "R%d tried to sub from different maps, paths, or prohibited types\n", dst);
4540-
return ret;
4541-
}
4584+
if (ret < 0)
4585+
return sanitize_err(env, insn, ret, off_reg, dst_reg);
4586+
45424587
if (dst_reg == off_reg) {
45434588
/* scalar -= pointer. Creates an unknown scalar */
45444589
verbose(env, "R%d tried to subtract pointer from scalar\n",
@@ -4655,7 +4700,6 @@ static int adjust_scalar_min_max_vals(struct bpf_verifier_env *env,
46554700
s64 smin_val, smax_val;
46564701
u64 umin_val, umax_val;
46574702
u64 insn_bitness = (BPF_CLASS(insn->code) == BPF_ALU64) ? 64 : 32;
4658-
u32 dst = insn->dst_reg;
46594703
int ret;
46604704

46614705
if (insn_bitness == 32) {
@@ -4692,10 +4736,8 @@ static int adjust_scalar_min_max_vals(struct bpf_verifier_env *env,
46924736
switch (opcode) {
46934737
case BPF_ADD:
46944738
ret = sanitize_val_alu(env, insn);
4695-
if (ret < 0) {
4696-
verbose(env, "R%d tried to add from different pointers or scalars\n", dst);
4697-
return ret;
4698-
}
4739+
if (ret < 0)
4740+
return sanitize_err(env, insn, ret, NULL, NULL);
46994741
if (signed_add_overflows(dst_reg->smin_value, smin_val) ||
47004742
signed_add_overflows(dst_reg->smax_value, smax_val)) {
47014743
dst_reg->smin_value = S64_MIN;
@@ -4716,10 +4758,8 @@ static int adjust_scalar_min_max_vals(struct bpf_verifier_env *env,
47164758
break;
47174759
case BPF_SUB:
47184760
ret = sanitize_val_alu(env, insn);
4719-
if (ret < 0) {
4720-
verbose(env, "R%d tried to sub from different pointers or scalars\n", dst);
4721-
return ret;
4722-
}
4761+
if (ret < 0)
4762+
return sanitize_err(env, insn, ret, NULL, NULL);
47234763
if (signed_sub_overflows(dst_reg->smin_value, smax_val) ||
47244764
signed_sub_overflows(dst_reg->smax_value, smin_val)) {
47254765
/* Overflow possible, we know nothing */

0 commit comments

Comments
 (0)