Skip to content

Commit

Permalink
bpf: fix sanitation rewrite in case of non-pointers
Browse files Browse the repository at this point in the history
Marek reported that he saw an issue with the below snippet in that
timing measurements where off when loaded as unpriv while results
were reasonable when loaded as privileged:

    [...]
    uint64_t a = bpf_ktime_get_ns();
    uint64_t b = bpf_ktime_get_ns();
    uint64_t delta = b - a;
    if ((int64_t)delta > 0) {
    [...]

Turns out there is a bug where a corner case is missing in the fix
d3bd741 ("bpf: fix sanitation of alu op with pointer / scalar
type from different paths"), namely fixup_bpf_calls() only checks
whether aux has a non-zero alu_state, but it also needs to test for
the case of BPF_ALU_NON_POINTER since in both occasions we need to
skip the masking rewrite (as there is nothing to mask).

Fixes: d3bd741 ("bpf: fix sanitation of alu op with pointer / scalar type from different paths")
Reported-by: Marek Majkowski <marek@cloudflare.com>
Reported-by: Arthur Fabre <afabre@cloudflare.com>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Link: https://lore.kernel.org/netdev/CAJPywTJqP34cK20iLM5YmUMz9KXQOdu1-+BZrGMAGgLuBWz7fg@mail.gmail.com/T/
Acked-by: Song Liu <songliubraving@fb.com>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
  • Loading branch information
Daniel Borkmann authored and Alexei Starovoitov committed Mar 2, 2019
1 parent d1a2930 commit 3612af7
Showing 1 changed file with 2 additions and 1 deletion.
3 changes: 2 additions & 1 deletion kernel/bpf/verifier.c
Original file line number Diff line number Diff line change
Expand Up @@ -6920,7 +6920,8 @@ static int fixup_bpf_calls(struct bpf_verifier_env *env)
u32 off_reg;

aux = &env->insn_aux_data[i + delta];
if (!aux->alu_state)
if (!aux->alu_state ||
aux->alu_state == BPF_ALU_NON_POINTER)
continue;

isneg = aux->alu_state & BPF_ALU_NEG_VALUE;
Expand Down

0 comments on commit 3612af7

Please sign in to comment.