On 15/03/2024 14:50, 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
Just a note that I had to amend and reland this patch. I had (once again) forgotten that user addresses cannot be directly compared in this manner - untagged_addr() must be used first to mask out the top byte. Android likes to insert dummy tags in the top byte (when not using MTE), and this patch crashes Android very early without the appropriate masking. The new diff can be found below.
Kevin
-------8<-------
diff --git a/arch/arm64/include/asm/compat.h b/arch/arm64/include/asm/compat.h index 4556bed1704f..4f6d7eb0106e 100644 --- a/arch/arm64/include/asm/compat.h +++ b/arch/arm64/include/asm/compat.h @@ -89,11 +89,15 @@ struct compat_statfs { #ifdef CONFIG_CHERI_PURECAP_UABI static inline void __user *compat_ptr(compat_uptr_t uptr) { + ptraddr_t addr = untagged_addr(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(addr >= PAGE_SIZE && addr < 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