On Thu, Jan 18, 2018 at 01:15:21AM +0100, Daniel Borkmann wrote:
> syzkaller generated a BPF proglet and triggered a warning with
> the following:
>
> 0: (b7) r0 = 0
> 1: (d5) if r0 s<= 0x0 goto pc+0
> R0=inv0 R1=ctx(id=0,off=0,imm=0) R10=fp0
> 2: (1f) r0 -= r1
> R0=inv0 R1=ctx(id=0,off=0,imm=0) R10=fp0
> verifier internal error: known but bad sbounds
>
> What happens is that in the first insn, r0's min/max value
> are both 0 due to the immediate assignment, later in the jsle
> test the bounds are updated for the min value in the false
> path, meaning, they yield smin_val = 1, smax_val = 0, and when
> ctx pointer is subtracted from r0, verifier bails out with the
> internal error and throwing a WARN since smin_val != smax_val
> for the known constant.
>
> For min_val > max_val scenario it means that reg_set_min_max()
> and reg_set_min_max_inv() (which both refine existing bounds)
> demonstrated that such branch cannot be taken at runtime.
>
> In above scenario for the case where it will be taken, the
> existing [0, 0] bounds are kept intact. Meaning, the rejection
> is not due to a verifier internal error, and therefore the
> WARN() is not necessary either.
>
> We could just reject such cases in adjust_{ptr,scalar}_min_max_vals()
> when either known scalars have smin_val != smax_val or
> umin_val != umax_val or any scalar reg with bounds
> smin_val > smax_val or umin_val > umax_val. However, there
> may be a small risk of breakage of buggy programs, so handle
> this more gracefully and in adjust_{ptr,scalar}_min_max_vals()
> just taint the dst reg as unknown scalar when we see ops with
> such kind of src reg.
>
> Reported-by: [email protected]
> Signed-off-by: Daniel Borkmann <[email protected]>
there are several ways to fix it and this approach looks
to be the safest way to do it since we're so late into the release.
Applied, Thank you Daniel.
