Re: [bpf-next PATCH 05/10] bpf: verifier, return value is an int in do_refine_retval_range
From: John Fastabend <john.fastabend@gmail.com>
Date: 2020-03-26 15:52:27
Also in:
bpf
Alexei Starovoitov wrote:
On Tue, Mar 24, 2020 at 10:39:16AM -0700, John Fastabend wrote:quoted
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@gmail.com> --- 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;
OK, I missed the u64 in BPF_CALL_x(). Setting both smax and s32_max looks correct. My logic above is wrong so I'll fix that. Thanks.