ACK: [SRU Groovy, Hirsute, Focal/linux-oem-5.10 1/1] bpf: Fix propagation of 32 bit unsigned bounds from 64 bit bounds

[Krzysztof Kozlowski]

On 24/05/2021 18:56, Thadeu Lima de Souza Cascardo wrote:
> From: Daniel Borkmann <daniel at iogearbox.net>
> 
> Similarly as b02709587ea3 ("bpf: Fix propagation of 32-bit signed bounds
> from 64-bit bounds."), we also need to fix the propagation of 32 bit
> unsigned bounds from 64 bit counterparts. That is, really only set the
> u32_{min,max}_value when /both/ {umin,umax}_value safely fit in 32 bit
> space. For example, the register with a umin_value == 1 does /not/ imply
> that u32_min_value is also equal to 1, since umax_value could be much
> larger than 32 bit subregister can hold, and thus u32_min_value is in
> the interval [0,1] instead.
> 
> Before fix, invalid tracking result of R2_w=inv1:
> 
>   [...]
>   5: R0_w=inv1337 R1=ctx(id=0,off=0,imm=0) R2_w=inv(id=0) R10=fp0
>   5: (35) if r2 >= 0x1 goto pc+1
>   [...] // goto path
>   7: R0=inv1337 R1=ctx(id=0,off=0,imm=0) R2=inv(id=0,umin_value=1) R10=fp0
>   7: (b6) if w2 <= 0x1 goto pc+1
>   [...] // goto path
>   9: R0=inv1337 R1=ctx(id=0,off=0,imm=0) R2=inv(id=0,smin_value=-9223372036854775807,smax_value=9223372032559808513,umin_value=1,umax_value=18446744069414584321,var_off=(0x1; 0xffffffff00000000),s32_min_value=1,s32_max_value=1,u32_max_value=1) R10=fp0
>   9: (bc) w2 = w2
>   10: R0=inv1337 R1=ctx(id=0,off=0,imm=0) R2_w=inv1 R10=fp0
>   [...]
> 
> After fix, correct tracking result of R2_w=inv(id=0,umax_value=1,var_off=(0x0; 0x1)):
> 
>   [...]
>   5: R0_w=inv1337 R1=ctx(id=0,off=0,imm=0) R2_w=inv(id=0) R10=fp0
>   5: (35) if r2 >= 0x1 goto pc+1
>   [...] // goto path
>   7: R0=inv1337 R1=ctx(id=0,off=0,imm=0) R2=inv(id=0,umin_value=1) R10=fp0
>   7: (b6) if w2 <= 0x1 goto pc+1
>   [...] // goto path
>   9: R0=inv1337 R1=ctx(id=0,off=0,imm=0) R2=inv(id=0,smax_value=9223372032559808513,umax_value=18446744069414584321,var_off=(0x0; 0xffffffff00000001),s32_min_value=0,s32_max_value=1,u32_max_value=1) R10=fp0
>   9: (bc) w2 = w2
>   10: R0=inv1337 R1=ctx(id=0,off=0,imm=0) R2_w=inv(id=0,umax_value=1,var_off=(0x0; 0x1)) R10=fp0
>   [...]
> 
> Thus, same issue as in b02709587ea3 holds for unsigned subregister tracking.
> Also, align __reg64_bound_u32() similarly to __reg64_bound_s32() as done in
> b02709587ea3 to make them uniform again.
> 
> Fixes: 3f50f132d840 ("bpf: Verifier, do explicit ALU32 bounds tracking")
> Reported-by: Manfred Paul (@_manfp)
> Signed-off-by: Daniel Borkmann <daniel at iogearbox.net>
> Reviewed-by: John Fastabend <john.fastabend at gmail.com>
> Acked-by: Alexei Starovoitov <ast at kernel.org>
> (cherry picked from commit 10bf4e83167cc68595b85fd73bb91e8f2c086e36)
> CVE-2021-31440
> Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo at canonical.com>
> ---
>  kernel/bpf/verifier.c                               | 8 +++-----
>  tools/testing/selftests/bpf/verifier/array_access.c | 2 +-
>  2 files changed, 4 insertions(+), 6 deletions(-)
> 


Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski at canonical.com>


Best regards,
Krzysztof