From: Roberto Sassu roberto.sassu@huawei.com
BPF LSM needs a reliable source of information to determine if the return value given by eBPF programs is acceptable or not. At the moment, choosing either the 64 bit or the 32 bit one does not seem to be an option (selftests fail).
If we choose the 64 bit one, the following happens.
14: 61 10 00 00 00 00 00 00 r0 = *(u32 *)(r1 + 0) 15: 74 00 00 00 15 00 00 00 w0 >>= 21 16: 54 00 00 00 01 00 00 00 w0 &= 1 17: 04 00 00 00 ff ff ff ff w0 += -1
This is the last part of test_deny_namespace. After #16, the register values are:
smin_value = 0x0, smax_value = 0x1, s32_min_value = 0x0, s32_max_value = 0x1,
After #17, they become:
smin_value = 0x0, smax_value = 0xffffffff, s32_min_value = 0xffffffff, s32_max_value = 0x0
where only the 32 bit values are correct.
If we choose the 32 bit ones, the following happens.
0000000000000000 <check_access>: 0: 79 12 00 00 00 00 00 00 r2 = *(u64 *)(r1 + 0) 1: 79 10 08 00 00 00 00 00 r0 = *(u64 *)(r1 + 8) 2: 67 00 00 00 3e 00 00 00 r0 <<= 62 3: c7 00 00 00 3f 00 00 00 r0 s>>= 63
This is part of test_libbpf_get_fd_by_id_opts (no_alu32 version). In this case, 64 bit register values should be used (for the 32 bit ones, there is no precise information from the verifier).
As the examples above suggest that which register values to use depends on the specific case, mark ALU32 operations in bpf_reg_state structure, so that BPF LSM can choose the proper ones.
Signed-off-by: Roberto Sassu roberto.sassu@huawei.com --- include/linux/bpf_verifier.h | 1 + kernel/bpf/verifier.c | 10 +++++++++- 2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h index 70d06a99f0b8..29c9cf6b0d01 100644 --- a/include/linux/bpf_verifier.h +++ b/include/linux/bpf_verifier.h @@ -181,6 +181,7 @@ struct bpf_reg_state { enum bpf_reg_liveness live; /* if (!precise && SCALAR_VALUE) min/max/tnum don't affect safety */ bool precise; + bool alu32; };
enum bpf_stack_slot_type { diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 8c5f0adbbde3..edce85c425a2 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -10524,9 +10524,13 @@ static int adjust_scalar_min_max_vals(struct bpf_verifier_env *env, break; }
+ dst_reg->alu32 = false; + /* ALU32 ops are zero extended into 64bit register */ - if (alu32) + if (alu32) { zext_32_to_64(dst_reg); + dst_reg->alu32 = true; + } reg_bounds_sync(dst_reg); return 0; } @@ -10700,6 +10704,7 @@ static int check_alu_op(struct bpf_verifier_env *env, struct bpf_insn *insn) *dst_reg = *src_reg; dst_reg->live |= REG_LIVE_WRITTEN; dst_reg->subreg_def = DEF_NOT_SUBREG; + dst_reg->alu32 = false; } else { /* R1 = (u32) R2 */ if (is_pointer_value(env, insn->src_reg)) { @@ -10716,6 +10721,7 @@ static int check_alu_op(struct bpf_verifier_env *env, struct bpf_insn *insn) dst_reg->id = 0; dst_reg->live |= REG_LIVE_WRITTEN; dst_reg->subreg_def = env->insn_idx + 1; + dst_reg->alu32 = true; } else { mark_reg_unknown(env, regs, insn->dst_reg); @@ -10733,9 +10739,11 @@ static int check_alu_op(struct bpf_verifier_env *env, struct bpf_insn *insn) if (BPF_CLASS(insn->code) == BPF_ALU64) { __mark_reg_known(regs + insn->dst_reg, insn->imm); + regs[insn->dst_reg].alu32 = false; } else { __mark_reg_known(regs + insn->dst_reg, (u32)insn->imm); + regs[insn->dst_reg].alu32 = true; } }