Skip to content

Commit 0d0e576

Browse files
borkmanndavem330
authored andcommitted
bpf: don't let ldimm64 leak map addresses on unprivileged
The patch fixes two things at once: 1) It checks the env->allow_ptr_leaks and only prints the map address to the log if we have the privileges to do so, otherwise it just dumps 0 as we would when kptr_restrict is enabled on %pK. Given the latter is off by default and not every distro sets it, I don't want to rely on this, hence the 0 by default for unprivileged. 2) Printing of ldimm64 in the verifier log is currently broken in that we don't print the full immediate, but only the 32 bit part of the first insn part for ldimm64. Thus, fix this up as well; it's okay to access, since we verified all ldimm64 earlier already (including just constants) through replace_map_fd_with_map_ptr(). Fixes: 1be7f75 ("bpf: enable non-root eBPF programs") Fixes: cbd3570 ("bpf: verifier (add ability to receive verification log)") Reported-by: Jann Horn <jannh@google.com> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net> Acked-by: Alexei Starovoitov <ast@kernel.org> Signed-off-by: David S. Miller <davem@davemloft.net>
1 parent 871ff2e commit 0d0e576

File tree

1 file changed

+16
-5
lines changed

1 file changed

+16
-5
lines changed

Diff for: kernel/bpf/verifier.c

+16-5
Original file line numberDiff line numberDiff line change
@@ -298,7 +298,8 @@ static const char *const bpf_jmp_string[16] = {
298298
[BPF_EXIT >> 4] = "exit",
299299
};
300300

301-
static void print_bpf_insn(struct bpf_insn *insn)
301+
static void print_bpf_insn(const struct bpf_verifier_env *env,
302+
const struct bpf_insn *insn)
302303
{
303304
u8 class = BPF_CLASS(insn->code);
304305

@@ -362,9 +363,19 @@ static void print_bpf_insn(struct bpf_insn *insn)
362363
insn->code,
363364
bpf_ldst_string[BPF_SIZE(insn->code) >> 3],
364365
insn->src_reg, insn->imm);
365-
} else if (BPF_MODE(insn->code) == BPF_IMM) {
366-
verbose("(%02x) r%d = 0x%x\n",
367-
insn->code, insn->dst_reg, insn->imm);
366+
} else if (BPF_MODE(insn->code) == BPF_IMM &&
367+
BPF_SIZE(insn->code) == BPF_DW) {
368+
/* At this point, we already made sure that the second
369+
* part of the ldimm64 insn is accessible.
370+
*/
371+
u64 imm = ((u64)(insn + 1)->imm << 32) | (u32)insn->imm;
372+
bool map_ptr = insn->src_reg == BPF_PSEUDO_MAP_FD;
373+
374+
if (map_ptr && !env->allow_ptr_leaks)
375+
imm = 0;
376+
377+
verbose("(%02x) r%d = 0x%llx\n", insn->code,
378+
insn->dst_reg, (unsigned long long)imm);
368379
} else {
369380
verbose("BUG_ld_%02x\n", insn->code);
370381
return;
@@ -2853,7 +2864,7 @@ static int do_check(struct bpf_verifier_env *env)
28532864

28542865
if (log_level) {
28552866
verbose("%d: ", insn_idx);
2856-
print_bpf_insn(insn);
2867+
print_bpf_insn(env, insn);
28572868
}
28582869

28592870
err = ext_analyzer_insn_hook(env, insn_idx, prev_insn_idx);

0 commit comments

Comments
 (0)