netdev - Re: [PATCH v2] bpf: fix a bug in verification logic when SUB operation taken on FRAME

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 18 Jun 2015 09:00:19 -0700
From:	Alexei Starovoitov <alexei.starovoitov@...il.com>
To:	Wang Nan <wangnan0@...wei.com>
Cc:	ast@...nel.org, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org, lizefan@...wei.com, pi3orama@....com
Subject: Re: [PATCH v2] bpf: fix a bug in verification logic when SUB
 operation taken on FRAME_PTR

On Thu, Jun 18, 2015 at 08:31:45AM +0000, Wang Nan wrote:
> Original code has a problem, cause following code failed to pass verifier:
> 
>  r1 <- r10
>  r1 -= 8
>  r2 = 8
>  r3 = unsafe pointer
>  call BPF_FUNC_probe_read  <-- R1 type=inv expected=fp
> 
> However, by replacing 'r1 -= 8' to 'r1 += -8' the above program can be
> loaded successfully.
> 
> This is because the verifier allows only BPF_ADD instruction on a
> FRAME_PTR reigster to forge PTR_TO_STACK register, but makes BPF_SUB
> on FRAME_PTR reigster to get a UNKNOWN_VALUE register.
> 
> This patch fix it by adding BPF_SUB in stack_relative checking.

It's not a bug. It's catching ADD only by design.
If we let it recognize SUB then one might argue we should let it
recognize multiply, shifts and all other arithmetic on pointers.
verifier will be getting bigger and bigger. Where do we stop?
llvm only emits canonical ADD. If you've seen llvm doing SUB,
let's fix it there.
So what piece generated this 'r1 -= 8' ?

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html