diff mbox series

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

Message ID 20230918155233.297024-3-memxor@gmail.com (mailing list archive)
State Accepted
Commit 00b7e8f4c02d75fa3a34a39a591afd951cfce837
Delegated to: BPF
Headers show
Series Fixes for Exceptions | expand

Checks

Context Check Description
bpf/vmtest-bpf-next-VM_Test-0 success Logs for ShellCheck
bpf/vmtest-bpf-next-VM_Test-5 success Logs for set-matrix
bpf/vmtest-bpf-next-VM_Test-1 success Logs for build for aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-3 success Logs for build for x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-4 success Logs for build for x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-2 success Logs for build for s390x with gcc
bpf/vmtest-bpf-next-VM_Test-27 success Logs for test_verifier on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-6 success Logs for test_maps on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-8 success Logs for test_maps on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-9 success Logs for test_maps on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-12 fail Logs for test_progs on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-13 success Logs for test_progs on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-14 success Logs for test_progs_no_alu32 on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-16 success Logs for test_progs_no_alu32 on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-18 success Logs for test_progs_no_alu32_parallel on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-17 success Logs for test_progs_no_alu32 on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-19 success Logs for test_progs_no_alu32_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-20 success Logs for test_progs_no_alu32_parallel on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-21 success Logs for test_progs_parallel on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-22 success Logs for test_progs_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-23 success Logs for test_progs_parallel on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-24 success Logs for test_verifier on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-25 success Logs for test_verifier on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-26 success Logs for test_verifier on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-28 success Logs for veristat
bpf/vmtest-bpf-next-VM_Test-10 success Logs for test_progs on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-15 success Logs for test_progs_no_alu32 on s390x with gcc
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for bpf-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1384 this patch: 1383
netdev/cc_maintainers fail 1 blamed authors not CCed: andreyknvl@gmail.com; 11 maintainers not CCed: andreyknvl@gmail.com martin.lau@linux.dev jolsa@kernel.org haoluo@google.com daniel@iogearbox.net sdf@google.com john.fastabend@gmail.com yonghong.song@linux.dev andrii@kernel.org kpsingh@kernel.org song@kernel.org
netdev/build_clang success Errors and warnings before: 1364 this patch: 1364
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 1406 this patch: 1406
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 8 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
bpf/vmtest-bpf-next-VM_Test-11 success Logs for test_progs on s390x with gcc
bpf/vmtest-bpf-next-PR fail PR summary
bpf/vmtest-bpf-next-VM_Test-7 success Logs for test_maps on s390x with gcc

Commit Message

Kumar Kartikeya Dwivedi Sept. 18, 2023, 3:52 p.m. UTC
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.

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(-)

Comments

Matthieu Baerts Sept. 18, 2023, 5:09 p.m. UTC | #1
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.

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

Cheers,
Matt
Kui-Feng Lee Sept. 18, 2023, 6:12 p.m. UTC | #2
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
diff mbox series

Patch

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);
 	ctx.aux->bpf_exception_cb(cookie, ctx.sp, ctx.bp);
 	WARN(1, "A call to BPF exception callback should never return\n");
 }