Re: [PATCH bpf-next v2 2/3] bpf: Fix bpf_throw warning on 32-bit arch

Kui-Feng Lee
2023-09-18T18:12:33Z

On 9/18/23 10:09, Matthieu Baerts wrote:
> Hi Kumar,
>
> On 18/09/2023 17:52, Kumar Kartikeya Dwivedi wrote:
>> On 32-bit architectures, the pointer width is 32-bit, while we try to
>> cast from a u64 down to it, the compiler complains on mismatch in
>> integer size. Fix this by first casting to long which should match
>> the pointer width on targets supported by Linux.
>
> Thank you for the patch, it fixes the issue on our side!
>
> (Not sure you need a tested by tag but just in case: )
>
> Tested-by: Matthieu Baerts <matthieu.baerts@tessares.net>
>
>> Fixes: ec5290a178b7 ("bpf: Prevent KASAN false positive with bpf_throw")
>> Reported-by: Matthieu Baerts <matthieu.baerts@tessares.net>
>> Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
>> ---
>> kernel/bpf/helpers.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
>> index 7ff2a42f1996..dd1c69ee3375 100644
>> --- a/kernel/bpf/helpers.c
>> +++ b/kernel/bpf/helpers.c
>> @@ -2488,7 +2488,7 @@ __bpf_kfunc void bpf_throw(u64 cookie)
>> * deeper stack depths than ctx.sp as we do not return from bpf_throw,
>> * which skips compiler generated instrumentation to do the same.
>> */
>> - kasan_unpoison_task_stack_below((void *)ctx.sp);
>> + kasan_unpoison_task_stack_below((void *)(long)ctx.sp);
> I never know what's the recommended way to fix such issues: casting it
> to 'long' or 'unsigned long'? But it looks like both are used in the
> kernel and 'long' is more often used than the other one so all good I
> suppose.

Shouldn't we have a macro to do this kind of casting if there is not?
Without any comment, it is difficult to know why this extra casting
is here.

>
>> ctx.aux->bpf_exception_cb(cookie, ctx.sp, ctx.bp);
>> WARN(1, "A call to BPF exception callback should never return\n");
>> }
>
> Cheers,
> Matt