On 16/11/2022 15:12, Tudor Cretu wrote:
On 15-11-2022 09:08, Zachary Leaf wrote:
The bpf_attr union currently stores pointers as u64 addresses, and hence the make_bpfptr helper function takes u64 addresses instead of pointers.
In preparation for changing bpf_attr for architectures where user pointers are longer than u64 (e.g. Morello PCuABI), adapt make_bpfptr to accept __kernel_aligned_uintptr_t. This will remain u64 on architectures
Why __kernel_aligned_uintptr_t and not __kernel_uintptr_t? We're typically using __kernel_aligned_uintptr_t to replace __aligned_u64, but maybe I'm missing something.
Not missing anything - we should probably use the more generic __kernel_uintptr_t version here. It's the exact same thing in PCuABI (__uintcap_t) and otherwise it's __u64 v __aligned_u64 and I don't think alignment matters here.
Will change to __kernel_uintptr_t.
where addresses and pointers are equivalent, but be extended on archs with larger pointer sizes.
Signed-off-by: Zachary Leaf zachary.leaf@arm.com
include/linux/bpfptr.h | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/include/linux/bpfptr.h b/include/linux/bpfptr.h index abb5b3641f5d..627c1c9c0372 100644 --- a/include/linux/bpfptr.h +++ b/include/linux/bpfptr.h @@ -31,6 +31,14 @@ static inline bpfptr_t make_bpfptr(u64 addr, bool is_kernel) return USER_BPFPTR(u64_to_user_ptr(addr)); } +static inline bpfptr_t make_bpfptr_fixed(__kernel_aligned_uintptr_t ptr, bool is_kernel) +{ + if (is_kernel) + return KERNEL_BPFPTR((void *) (uintptr_t) ptr);
a cast to (void *) should be sufficient without going through uintptr_t now.
That results in a CToPtr warning for me:
./include/linux/bpfptr.h:37:24: warning: the following conversion will result in a CToPtr operation; the behaviour of CToPtr can be confusing since using CToPtr on an untagged capability will give 0 instead of the integer value and should therefore be explicitly annotated [-Wcher i-pointer-conversion]
For PCuABI __kernel_aligned_uintptr_t is __uintcap_t (128b), void* is 64b and uintptr_t should be u64.
I guess the warning here is erroneous since in this code path, in theory it should be the kernel calling this with a 64b kernel ptr, so ptr should contain a 64b integer representation in __uintcap_t (__kernel_uintptr_t), with nothing in the top bits.
Explicit cast to uintptr_t then tells the compiler there is no chance this __uintcap_t contains a capability so no CToPtr warning. I'm not sure exactly. Would be interested if anyone understands that differently.
Thanks, Zach
+ else + return USER_BPFPTR((void __user *) ptr); +}
static inline bool bpfptr_is_null(bpfptr_t bpfptr) { if (bpfptr_is_kernel(bpfptr))