On Wed, Oct 16, 2019 at 03:44:25PM +0100, Dave P Martin wrote:
On Wed, Oct 16, 2019 at 05:29:33AM +0100, Will Deacon wrote:
diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h index b61b50bf68b1..c23c47360664 100644 --- a/arch/arm64/include/asm/memory.h +++ b/arch/arm64/include/asm/memory.h @@ -215,12 +215,18 @@ static inline unsigned long kaslr_offset(void)
- up with a tagged userland pointer. Clear the tag to get a sane pointer to
- pass on to access_ok(), for instance.
*/ -#define untagged_addr(addr) \ +#define __untagged_addr(addr) \ ((__force __typeof__(addr))sign_extend64((__force u64)(addr), 55)) +#define untagged_addr(addr) ({ \
Having the same informal name ("untagged") for two different address representations seems like a recipe for confusion. Can we rename one of them? (Say, untagged_address_if_user()?)
I agree it's confusing. We can rename the __* one but the other is spread around the kernel (it can be done, though as a subsequent exercise; untagged_uaddr?).
- __addr &= __untagged_addr(__addr); \
- (__force __typeof__(addr))__addr; \
+})
Is there any reason why we can't just have
#define untagged_addr ((__force __typeof__(addr))( \ (__force u64)(addr) & GENMASK_ULL(63, 56)))
I guess you meant ~GENMASK_ULL(63,56) or GENMASK_ULL(55,0).
This changes the overflow case for mlock() which would return -ENOMEM instead of -EINVAL (not sure we have a test for it though). Does it matter?