bpf: fix undefined behavior in narrow load handling
authorKrzesimir Nowak <krzesimir@kinvolk.io>
Wed, 8 May 2019 16:08:58 +0000 (18:08 +0200)
committerDaniel Borkmann <daniel@iogearbox.net>
Mon, 13 May 2019 00:05:50 +0000 (02:05 +0200)
Commit 31fd85816dbe ("bpf: permits narrower load from bpf program
context fields") made the verifier add AND instructions to clear the
unwanted bits with a mask when doing a narrow load. The mask is
computed with

  (1 << size * 8) - 1

where "size" is the size of the narrow load. When doing a 4 byte load
of a an 8 byte field the verifier shifts the literal 1 by 32 places to
the left. This results in an overflow of a signed integer, which is an
undefined behavior. Typically, the computed mask was zero, so the
result of the narrow load ended up being zero too.

Cast the literal to long long to avoid overflows. Note that narrow
load of the 4 byte fields does not have the undefined behavior,
because the load size can only be either 1 or 2 bytes, so shifting 1
by 8 or 16 places will not overflow it. And reading 4 bytes would not
be a narrow load of a 4 bytes field.

Fixes: 31fd85816dbe ("bpf: permits narrower load from bpf program context fields")
Reviewed-by: Alban Crequy <alban@kinvolk.io>
Reviewed-by: Iago López Galeiras <iago@kinvolk.io>
Signed-off-by: Krzesimir Nowak <krzesimir@kinvolk.io>
Cc: Yonghong Song <yhs@fb.com>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
kernel/bpf/verifier.c

index 7b05e8938d5cc69192b768a78108223d8322010c..95f9354495ad833ae7c981e551a518a478897fd0 100644 (file)
@@ -7599,7 +7599,7 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env)
                                                                        insn->dst_reg,
                                                                        shift);
                                insn_buf[cnt++] = BPF_ALU64_IMM(BPF_AND, insn->dst_reg,
-                                                               (1 << size * 8) - 1);
+                                                               (1ULL << size * 8) - 1);
                        }
                }