bpf: fix replace_map_fd_with_map_ptr's ldimm64 second imm field

Non-zero imm value in the second part of the ldimm64 instruction for
BPF_PSEUDO_MAP_FD is invalid, and thus must be rejected. The map fd
only ever sits in the first instructions' imm field. None of the BPF
loaders known to us are using it, so risk of regression is minimal.
For clarity and consistency, the few insn->{src_reg,imm} occurrences
are rewritten into insn[0].{src_reg,imm}. Add a test case to the BPF
selftest suite as well.

Fixes: 0246e64d9a5f ("bpf: handle pseudo BPF_LD_IMM64 insn")
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Song Liu <songliubraving@fb.com>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index a7b96bf0e6546c077c47799547aaf9b9df5dbe19..ce166a002d161a08eff6bdbb77158886dbba8012 100644 (file)
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -6678,17 +6678,17 @@ static int replace_map_fd_with_map_ptr(struct bpf_verifier_env *env)
                                /* valid generic load 64-bit imm */
                                goto next_insn;
 
-                       if (insn->src_reg != BPF_PSEUDO_MAP_FD) {
-                               verbose(env,
-                                       "unrecognized bpf_ld_imm64 insn\n");
+                       if (insn[0].src_reg != BPF_PSEUDO_MAP_FD ||
+                           insn[1].imm != 0) {
+                               verbose(env, "unrecognized bpf_ld_imm64 insn\n");
                                return -EINVAL;
                        }
 
-                       f = fdget(insn->imm);
+                       f = fdget(insn[0].imm);
                        map = __bpf_map_get(f);
                        if (IS_ERR(map)) {
                                verbose(env, "fd %d is not pointing to valid bpf_map\n",
-                                       insn->imm);
+                                       insn[0].imm);
                                return PTR_ERR(map);
                        }
 

diff --git a/tools/testing/selftests/bpf/verifier/ld_imm64.c b/tools/testing/selftests/bpf/verifier/ld_imm64.c
index 28b8c805a293289ce295d50ead796b83e5324ae5..3856dba733e93f23922962bdd00bf05e0d720366 100644 (file)
--- a/tools/testing/selftests/bpf/verifier/ld_imm64.c
+++ b/tools/testing/selftests/bpf/verifier/ld_imm64.c
@@ -122,7 +122,7 @@
        .insns = {
        BPF_MOV64_IMM(BPF_REG_1, 0),
        BPF_RAW_INSN(BPF_LD | BPF_IMM | BPF_DW, 0, BPF_REG_1, 0, 1),
-       BPF_RAW_INSN(0, 0, 0, 0, 1),
+       BPF_RAW_INSN(0, 0, 0, 0, 0),
        BPF_EXIT_INSN(),
        },
        .errstr = "not pointing to valid bpf_map",
@@ -139,3 +139,16 @@
        .errstr = "invalid bpf_ld_imm64 insn",
        .result = REJECT,
 },
+{
+       "test14 ld_imm64: reject 2nd imm != 0",
+       .insns = {
+       BPF_MOV64_IMM(BPF_REG_0, 0),
+       BPF_RAW_INSN(BPF_LD | BPF_IMM | BPF_DW, BPF_REG_1,
+                    BPF_PSEUDO_MAP_FD, 0, 0),
+       BPF_RAW_INSN(0, 0, 0, 0, 0xfefefe),
+       BPF_EXIT_INSN(),
+       },
+       .fixup_map_hash_48b = { 1 },
+       .errstr = "unrecognized bpf_ld_imm64 insn",
+       .result = REJECT,
+},