[PATCH v4] bpf: core: fix shift-out-of-bounds in ___bpf_prog_run
From: Kurt Manucredo <hidden>
Date: 2021-06-05 15:02:21
Also in:
bpf, linux-kernel-mentees, lkml
Subsystem:
bpf [core], bpf [general] (safe dynamic programs and tools), the rest · Maintainers:
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi, Linus Torvalds
Syzbot detects a shift-out-of-bounds in ___bpf_prog_run() kernel/bpf/core.c:1414:2. I propose: In adjust_scalar_min_max_vals() move boundary check up to avoid missing them and return with error when detected. Reported-and-tested-by: syzbot+bed360704c521841c85d@syzkaller.appspotmail.com Signed-off-by: Kurt Manucredo <redacted> --- https://syzkaller.appspot.com/bug?id=edb51be4c9a320186328893287bb30d5eed09231 Changelog: ---------- v4 - Fix shift-out-of-bounds in adjust_scalar_min_max_vals. Fix commit message. v3 - Make it clearer what the fix is for. v2 - Fix shift-out-of-bounds in ___bpf_prog_run() by adding boundary check in check_alu_op() in verifier.c. v1 - Fix shift-out-of-bounds in ___bpf_prog_run() by adding boundary check in ___bpf_prog_run(). thanks kind regards Kurt kernel/bpf/verifier.c | 30 +++++++++--------------------- 1 file changed, 9 insertions(+), 21 deletions(-)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 94ba5163d4c5..ed0eecf20de5 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c@@ -7510,6 +7510,15 @@ static int adjust_scalar_min_max_vals(struct bpf_verifier_env *env, u32_min_val = src_reg.u32_min_value; u32_max_val = src_reg.u32_max_value; + if ((opcode == BPF_LSH || opcode == BPF_RSH || opcode == BPF_ARSH) && + umax_val >= insn_bitness) { + /* Shifts greater than 31 or 63 are undefined. + * This includes shifts by a negative number. + */ + verbose(env, "invalid shift %lld\n", umax_val); + return -EINVAL; + } + if (alu32) { src_known = tnum_subreg_is_const(src_reg.var_off); if ((src_known &&
@@ -7592,39 +7601,18 @@ static int adjust_scalar_min_max_vals(struct bpf_verifier_env *env, scalar_min_max_xor(dst_reg, &src_reg); break; case BPF_LSH: - if (umax_val >= insn_bitness) { - /* Shifts greater than 31 or 63 are undefined. - * This includes shifts by a negative number. - */ - mark_reg_unknown(env, regs, insn->dst_reg); - break; - } if (alu32) scalar32_min_max_lsh(dst_reg, &src_reg); else scalar_min_max_lsh(dst_reg, &src_reg); break; case BPF_RSH: - if (umax_val >= insn_bitness) { - /* Shifts greater than 31 or 63 are undefined. - * This includes shifts by a negative number. - */ - mark_reg_unknown(env, regs, insn->dst_reg); - break; - } if (alu32) scalar32_min_max_rsh(dst_reg, &src_reg); else scalar_min_max_rsh(dst_reg, &src_reg); break; case BPF_ARSH: - if (umax_val >= insn_bitness) { - /* Shifts greater than 31 or 63 are undefined. - * This includes shifts by a negative number. - */ - mark_reg_unknown(env, regs, insn->dst_reg); - break; - } if (alu32) scalar32_min_max_arsh(dst_reg, &src_reg); else
--
2.30.2