bpf, verifier: enforce larger zero range for pkt on overloading stack buffs
authorDaniel Borkmann <daniel@iogearbox.net>
Mon, 19 Sep 2016 22:26:12 +0000 (00:26 +0200)
committerDavid S. Miller <davem@davemloft.net>
Wed, 21 Sep 2016 03:32:11 +0000 (23:32 -0400)
Current contract for the following two helper argument types is:

  * ARG_CONST_STACK_SIZE: passed argument pair must be (ptr, >0).
  * ARG_CONST_STACK_SIZE_OR_ZERO: passed argument pair can be either
    (NULL, 0) or (ptr, >0).

With 6841de8b0d03 ("bpf: allow helpers access the packet directly"), we can
pass also raw packet data to helpers, so depending on the argument type
being PTR_TO_PACKET, we now either assert memory via check_packet_access()
or check_stack_boundary(). As a result, the tests in check_packet_access()
currently allow more than intended with regards to reg->imm.

Back in 969bf05eb3ce ("bpf: direct packet access"), check_packet_access()
was fine to ignore size argument since in check_mem_access() size was
bpf_size_to_bytes() derived and prior to the call to check_packet_access()
guaranteed to be larger than zero.

However, for the above two argument types, it currently means, we can have
a <= 0 size and thus breaking current guarantees for helpers. Enforce a
check for size <= 0 and bail out if so.

check_stack_boundary() doesn't have such an issue since it already tests
for access_size <= 0 and bails out, resp. access_size == 0 in case of NULL
pointer passed when allowed.

Fixes: 6841de8b0d03 ("bpf: allow helpers access the packet directly")
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
kernel/bpf/verifier.c

index 90493a66dddd397c5354cb0ac8ce157dafae212d..bc138f34e38c4e9a0b58acf9169f9bc6a548b496 100644 (file)
@@ -671,7 +671,7 @@ static int check_packet_access(struct verifier_env *env, u32 regno, int off,
        struct reg_state *reg = &regs[regno];
 
        off += reg->off;
-       if (off < 0 || off + size > reg->range) {
+       if (off < 0 || size <= 0 || off + size > reg->range) {
                verbose("invalid access to packet, off=%d size=%d, R%d(id=%d,off=%d,r=%d)\n",
                        off, size, regno, reg->id, reg->off, reg->range);
                return -EACCES;