On 05/04/2024 15:42, Carsten Haitzler wrote:
On 3/15/24 1:50 PM, Kevin Brodsky wrote:
In PCuABI, we need compat_ptr() to create a valid capability from an arbitrary compat pointer, to enable uaccess to the underlying memory. However, if the compat pointer is clearly an invalid address, it is preferable to return a null-derived (invalid) capability. This is especially true when the address is null: we clearly want to return the null capability in that case.
This should be a non-functional change (uaccess would fail anyway due to the absence of underlying mapping), but it avoids some confusion when debugging compat64 handling (and reduces the creation of capabilities).
Signed-off-by: Kevin Brodsky kevin.brodsky@arm.com
Review branch:
https://git.morello-project.org/kbrodsky-arm/linux/-/commits/morello/null_co...
arch/arm64/include/asm/compat.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/include/asm/compat.h b/arch/arm64/include/asm/compat.h index 4556bed1704f..acf5487985b5 100644 --- a/arch/arm64/include/asm/compat.h +++ b/arch/arm64/include/asm/compat.h @@ -93,7 +93,9 @@ static inline void __user *compat_ptr(compat_uptr_t uptr) * TODO [Morello] - this should be done using the current user DDC, not * the root user capability. */ - return (void __user *)cheri_address_set(cheri_user_root_allperms_cap, uptr); + return likely(uptr >= PAGE_SIZE && uptr < TASK_SIZE_MAX) ? + (void __user *)cheri_address_set(cheri_user_root_allperms_cap, uptr) : + as_user_ptr(uptr); } #define compat_ptr(uptr) compat_ptr(uptr) #endif
the only issue might be "it's not as fast" as it has to do more, but reality s being more right is more important than more fast here imho, so i see no issue with this - some obvious invalid ptr range checking seems fine.
Yes, that is a fair point. In fact that's part of the reason why I kept the bounds constant (I considered using mmap_min_addr as lower bound at some point, though it wouldn't work for other reasons). As it stands the overhead is probably unnoticeable (just some arithmetic and a couple of branches that are easily predicted).
To do the really correct thing, we should use the current user DDC here (TODO above), but that may be a bigger overhead.
+1
Thanks for the review! Now in next.
Kevin