On Tue, Mar 24, 2020 at 10:39:16AM -0700, John Fastabend wrote:
> Mark 32-bit subreg region with max value because do_refine_retval_range()
> catches functions with int return type (We will assume here that int is
> a 32-bit type). Marking 64-bit region could be dangerous if upper bits
> are not zero which could be possible.
>
> Two reasons to pull this out of original patch. First it makes the original
> fix impossible to backport. And second I've not seen this as being problematic
> in practice unlike the other case.
>
> Fixes: 849fa50662fbc ("bpf/verifier: refine retval R0 state for bpf_get_stack helper")
> Signed-off-by: John Fastabend <john.fastabend@xxxxxxxxx>
> ---
> kernel/bpf/verifier.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 6372fa4..3731109 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -4328,7 +4328,7 @@ static void do_refine_retval_range(struct bpf_reg_state *regs, int ret_type,
> func_id != BPF_FUNC_probe_read_str))
> return;
>
> - ret_reg->smax_value = meta->msize_max_value;
> + ret_reg->s32_max_value = meta->msize_max_value;
I think this is not correct.
These two special helpers are invoked via BPF_CALL_x() which has u64 return value.
So despite having 'int' return in bpf_helper_defs.h the upper 32-bit will be correct.
I think this patch should do:
ret_reg->smax_value = meta->msize_max_value;
ret_reg->s32_max_value = meta->msize_max_value;
