On Tue, Aug 02, 2016 at 04:12:14PM +0100, Jakub Kicinski wrote:
> Using per-register incrementing ID can lead to
> find_good_pkt_pointers() confusing registers which
> have completely different values. Consider example:
>
> 0: (bf) r6 = r1
> 1: (61) r8 = *(u32 *)(r6 +76)
> 2: (61) r0 = *(u32 *)(r6 +80)
> 3: (bf) r7 = r8
> 4: (07) r8 += 32
> 5: (2d) if r8 > r0 goto pc+9
> R0=pkt_end R1=ctx R6=ctx R7=pkt(id=0,off=0,r=32) R8=pkt(id=0,off=32,r=32)
> R10=fp
> 6: (bf) r8 = r7
> 7: (bf) r9 = r7
> 8: (71) r1 = *(u8 *)(r7 +0)
> 9: (0f) r8 += r1
> 10: (71) r1 = *(u8 *)(r7 +1)
> 11: (0f) r9 += r1
> 12: (07) r8 += 32
> 13: (2d) if r8 > r0 goto pc+1
> R0=pkt_end R1=inv56 R6=ctx R7=pkt(id=0,off=0,r=32) R8=pkt(id=1,off=32,r=32)
> R9=pkt(id=1,off=0,r=32) R10=fp
> 14: (71) r1 = *(u8 *)(r9 +16)
> 15: (b7) r7 = 0
> 16: (bf) r0 = r7
> 17: (95) exit
>
> We need to get a UNKNOWN_VALUE with imm to force id
> generation so lines 0-5 make r7 a valid packet pointer.
> We then read two different bytes from the packet and
> add them to copies of the constructed packet pointer.
> r8 (line 9) and r9 (line 11) will get the same id of 1,
> independently. When either of them is validated (line
> 13) - find_good_pkt_pointers() will also mark the other
> as safe. This leads to access on line 14 being mistakenly
> considered safe.
>
> Fixes: 969bf05eb3ce ("bpf: direct packet access")
> Signed-off-by: Jakub Kicinski <jakub.kicin...@netronome.com>
Great catch. Thanks!
Acked-by: Alexei Starovoitov <a...@kernel.org>
