Message ID | 1498054412-2495-2-git-send-email-jesse.sung@canonical.com |
---|---|
State | New |
Headers | show |
On 21.06.2017 16:13, Wen-chien Jesse Sung wrote: > From: Daniel Borkmann <daniel@iogearbox.net> > > CVE-2017-9150 > > 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: 1be7f75d1668 ("bpf: enable non-root eBPF programs") > Fixes: cbd357008604 ("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> > (backported from commit 0d0e57697f162da4aa218b5feafe614fb666db07) > Signed-off-by: Wen-chien Jesse Sung <jesse.sung@canonical.com> Acked-by: Stefan Bader <stefan.bader@canonical.com> > --- The backport appear to look right. But this to be targeted only fo X/Y. Any of the derivatives (or backports) will automatically get it when rebasing. Thanks, -Stefan > kernel/bpf/verifier.c | 21 ++++++++++++++++----- > 1 file changed, 16 insertions(+), 5 deletions(-) > > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > index 2cbfba7..85de509 100644 > --- a/kernel/bpf/verifier.c > +++ b/kernel/bpf/verifier.c > @@ -313,7 +313,8 @@ static const char *const bpf_jmp_string[16] = { > [BPF_EXIT >> 4] = "exit", > }; > > -static void print_bpf_insn(struct bpf_insn *insn) > +static void print_bpf_insn(const struct verifier_env *env, > + const struct bpf_insn *insn) > { > u8 class = BPF_CLASS(insn->code); > > @@ -377,9 +378,19 @@ static void print_bpf_insn(struct bpf_insn *insn) > insn->code, > bpf_ldst_string[BPF_SIZE(insn->code) >> 3], > insn->src_reg, insn->imm); > - } else if (BPF_MODE(insn->code) == BPF_IMM) { > - verbose("(%02x) r%d = 0x%x\n", > - insn->code, insn->dst_reg, insn->imm); > + } else if (BPF_MODE(insn->code) == BPF_IMM && > + BPF_SIZE(insn->code) == BPF_DW) { > + /* At this point, we already made sure that the second > + * part of the ldimm64 insn is accessible. > + */ > + u64 imm = ((u64)(insn + 1)->imm << 32) | (u32)insn->imm; > + bool map_ptr = insn->src_reg == BPF_PSEUDO_MAP_FD; > + > + if (map_ptr && !env->allow_ptr_leaks) > + imm = 0; > + > + verbose("(%02x) r%d = 0x%llx\n", insn->code, > + insn->dst_reg, (unsigned long long)imm); > } else { > verbose("BUG_ld_%02x\n", insn->code); > return; > @@ -1758,7 +1769,7 @@ static int do_check(struct verifier_env *env) > > if (log_level) { > verbose("%d: ", insn_idx); > - print_bpf_insn(insn); > + print_bpf_insn(env, insn); > } > > if (class == BPF_ALU || class == BPF_ALU64) { >
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 2cbfba7..85de509 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -313,7 +313,8 @@ static const char *const bpf_jmp_string[16] = { [BPF_EXIT >> 4] = "exit", }; -static void print_bpf_insn(struct bpf_insn *insn) +static void print_bpf_insn(const struct verifier_env *env, + const struct bpf_insn *insn) { u8 class = BPF_CLASS(insn->code); @@ -377,9 +378,19 @@ static void print_bpf_insn(struct bpf_insn *insn) insn->code, bpf_ldst_string[BPF_SIZE(insn->code) >> 3], insn->src_reg, insn->imm); - } else if (BPF_MODE(insn->code) == BPF_IMM) { - verbose("(%02x) r%d = 0x%x\n", - insn->code, insn->dst_reg, insn->imm); + } else if (BPF_MODE(insn->code) == BPF_IMM && + BPF_SIZE(insn->code) == BPF_DW) { + /* At this point, we already made sure that the second + * part of the ldimm64 insn is accessible. + */ + u64 imm = ((u64)(insn + 1)->imm << 32) | (u32)insn->imm; + bool map_ptr = insn->src_reg == BPF_PSEUDO_MAP_FD; + + if (map_ptr && !env->allow_ptr_leaks) + imm = 0; + + verbose("(%02x) r%d = 0x%llx\n", insn->code, + insn->dst_reg, (unsigned long long)imm); } else { verbose("BUG_ld_%02x\n", insn->code); return; @@ -1758,7 +1769,7 @@ static int do_check(struct verifier_env *env) if (log_level) { verbose("%d: ", insn_idx); - print_bpf_insn(insn); + print_bpf_insn(env, insn); } if (class == BPF_ALU || class == BPF_ALU64) {