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  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Sat, 13 Jan 2018 02:01:18 +0100
From:   Daniel Borkmann <daniel@...earbox.net>
To:     Edward Cree <ecree@...arflare.com>, ast@...com
Cc:     netdev@...r.kernel.org
Subject: Re: [PATCH bpf] bpf: do not modify min/max bounds on scalars with
 constant values

On 01/12/2018 08:52 PM, Edward Cree wrote:
> On 12/01/18 19:23, 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,
> reg_set_min_max() refines the existing bounds, it doesn't replace
>  them, so all that's happened is that the jsle handling has
>  demonstrated that this branch can't be taken.

Correct, on the one hand one could argue that for known constants
it cannot get any more refined than this already since we know
the precise constant value the register holds (thus we make the
range worse in this case fwiw), on the other hand why should they
be treated any different than registers with unknown scalars.
True that smin > smax is a result that the branch won't be taken
at runtime.

> That AFAICT isn't confined to known constants, one could e.g.
>  obtain inconsistent bounds with two js* insns.  Updating the
>  bounds in reg_set_min_max() is right, it's where we try to use
>  those sbounds in adjust_ptr_min_max_vals() that's wrong imho;
>  instead the 'known' paths should be using off_reg->var_off.value
>  rather than smin_val everywhere.

Ok, I'll check further on the side-effects and evaluate this
option of using var_off.value for a v2 as well, thanks!

> Alternatively we could consider not following jumps/lack-thereof
>  that produce inconsistent bounds, but that can make insns
>  unreachable that previously weren't and thus reject programs
>  that we previously considered valid, so we probably can't get
>  away with that.

Agree.

Thanks,
Daniel

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux - Powered by OpenVZ