Re: [GIT PULL] x86/mm for 6.4

[Dave Hansen <dave.hansen@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 1937 bytes --]

On 5/3/23 23:28, Kirill A. Shutemov wrote:
>> Untagging a kernel address will "corrupt" it, but it will stay a
>> kernel address (well, it will stay a "high bit set" address), which is
>> all we care about anyway.
> The interesting case to consider is untagging kernel pointer when LAM_U48
> is enabled (not part of current LAM enabling). LAM_U48 would make the
> untagging mask wider -- ~GENMASK(62, 48). With 5-level paging and LAM_SUP
> enabled (also not supported yet) untagging kernel may transform it to
> other valid kernel pointer.
> 
> So we cannot rely on #GP as backstop here. The kernel has to exclude
> kernel pointer by other means. It can be fun to debug.

Yeah, I have the feeling that we're really going to need a pair of
untagging functions once we get to doing kernel LAM for a _bunch_ of
reasons.

Just as a practical matter, I think we should OR bits into the mask on
the kernel side, effectively:

unsigned long untag_kernel_addr(unsigned long addr)
{
	return addr | kernel_untag_mask;
}

and kernel_untag_mask should have bit 63 *clear*.

That way the pointers that have gone through untagging won't look silly.
 If you untag VMALLOC_END or something, it'll still look like the
addresses we have in mm.rst.

Also, it'll be impossible to have the mask turn a userspace address into
a kernel one.

Last, we can add some debugging in there, probably conditional on some
mm debugging options like:

	if (WARN_ON_ONCE(!valid_user_address(addr)))
		return 0;

It's kinda like "void __user *" versus "void *".  The __user ones can
*absolutely* point anywhere, user or kernel.  That's why we can't WARN()
in the untagged_addr() function that takes user pointers.

But "void *" can only point to the kernel.  It has totally different rules.

We should probably also do something like the attached patch sooner
rather than later.  'untag_mask' really is a misleading name for a mask
that gets applied only to user addresses.

[-- Attachment #2: lam.patch --]
[-- Type: text/x-patch, Size: 6944 bytes --]

diff --git a/arch/x86/include/asm/mmu.h b/arch/x86/include/asm/mmu.h
index 0da5c227f490..adee70ee398c 100644
--- a/arch/x86/include/asm/mmu.h
+++ b/arch/x86/include/asm/mmu.h
@@ -51,7 +51,7 @@ typedef struct {
 	unsigned long lam_cr3_mask;
 
 	/* Significant bits of the virtual address. Excludes tag bits. */
-	u64 untag_mask;
+	u64 user_untag_mask;
 #endif
 
 	struct mutex lock;
diff --git a/arch/x86/include/asm/mmu_context.h b/arch/x86/include/asm/mmu_context.h
index 1d29dc791f5a..b4bb4b1a36e4 100644
--- a/arch/x86/include/asm/mmu_context.h
+++ b/arch/x86/include/asm/mmu_context.h
@@ -94,18 +94,18 @@ static inline unsigned long mm_lam_cr3_mask(struct mm_struct *mm)
 static inline void dup_lam(struct mm_struct *oldmm, struct mm_struct *mm)
 {
 	mm->context.lam_cr3_mask = oldmm->context.lam_cr3_mask;
-	mm->context.untag_mask = oldmm->context.untag_mask;
+	mm->context.user_untag_mask = oldmm->context.user_untag_mask;
 }
 
 #define mm_untag_mask mm_untag_mask
 static inline unsigned long mm_untag_mask(struct mm_struct *mm)
 {
-	return mm->context.untag_mask;
+	return mm->context.user_untag_mask;
 }
 
 static inline void mm_reset_untag_mask(struct mm_struct *mm)
 {
-	mm->context.untag_mask = -1UL;
+	mm->context.user_untag_mask = -1UL;
 }
 
 #define arch_pgtable_dma_compat arch_pgtable_dma_compat
diff --git a/arch/x86/include/asm/tlbflush.h b/arch/x86/include/asm/tlbflush.h
index 75bfaa421030..9fc6a2ac5318 100644
--- a/arch/x86/include/asm/tlbflush.h
+++ b/arch/x86/include/asm/tlbflush.h
@@ -55,11 +55,11 @@ static inline void cr4_clear_bits(unsigned long mask)
 }
 
 #ifdef CONFIG_ADDRESS_MASKING
-DECLARE_PER_CPU(u64, tlbstate_untag_mask);
+DECLARE_PER_CPU(u64, tlbstate_user_untag_mask);
 
-static inline u64 current_untag_mask(void)
+static inline u64 current_user_untag_mask(void)
 {
-	return this_cpu_read(tlbstate_untag_mask);
+	return this_cpu_read(tlbstate_user_untag_mask);
 }
 #endif
 
@@ -389,7 +389,7 @@ static inline void set_tlbstate_lam_mode(struct mm_struct *mm)
 {
 	this_cpu_write(cpu_tlbstate.lam,
 		       mm->context.lam_cr3_mask >> X86_CR3_LAM_U57_BIT);
-	this_cpu_write(tlbstate_untag_mask, mm->context.untag_mask);
+	this_cpu_write(tlbstate_user_untag_mask, mm->context.user_untag_mask);
 }
 
 #else
diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
index 123135d60f72..044d82e98b27 100644
--- a/arch/x86/include/asm/uaccess.h
+++ b/arch/x86/include/asm/uaccess.h
@@ -36,16 +36,16 @@ static inline unsigned long __untagged_addr(unsigned long addr)
 	long sign;
 
 	/*
-	 * Refer tlbstate_untag_mask directly to avoid RIP-relative relocation
-	 * in alternative instructions. The relocation gets wrong when gets
-	 * copied to the target place.
+	 * Refer tlbstate_user_untag_mask directly to avoid RIP-relative
+	 * relocation in alternative instructions. The relocation gets wrong when
+	 * gets copied to the target place.
 	 */
 	asm (ALTERNATIVE("",
 			 "sar $63, %[sign]\n\t" /* user_ptr ? 0 : -1UL */
-			 "or %%gs:tlbstate_untag_mask, %[sign]\n\t"
+			 "or %%gs:tlbstate_user_untag_mask, %[sign]\n\t"
 			 "and %[sign], %[addr]\n\t", X86_FEATURE_LAM)
 	     : [addr] "+r" (addr), [sign] "=r" (sign)
-	     : "m" (tlbstate_untag_mask), "[sign]" (addr));
+	     : "m" (tlbstate_user_untag_mask), "[sign]" (addr));
 
 	return addr;
 }
@@ -55,20 +55,20 @@ static inline unsigned long __untagged_addr(unsigned long addr)
 	(__force __typeof__(addr))__untagged_addr(__addr);		\
 })
 
-static inline unsigned long __untagged_addr_remote(struct mm_struct *mm,
-						   unsigned long addr)
+static inline unsigned long __untag_remote_user_addr(struct mm_struct *mm,
+						     unsigned long addr)
 {
 	long sign = addr >> 63;
 
 	mmap_assert_locked(mm);
-	addr &= (mm)->context.untag_mask | sign;
+	addr &= (mm)->context.user_untag_mask | sign;
 
 	return addr;
 }
 
-#define untagged_addr_remote(mm, addr)	({				\
+#define untag_remote_user_addr(mm, addr)	({				\
 	unsigned long __addr = (__force unsigned long)(addr);		\
-	(__force __typeof__(addr))__untagged_addr_remote(mm, __addr);	\
+	(__force __typeof__(addr))__untag_remote_user_addr(mm, __addr);	\
 })
 
 #else
diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index 3d4dd9420c30..929152b28b8c 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -580,7 +580,7 @@ static int vaddr_get_pfns(struct mm_struct *mm, unsigned long vaddr,
 		goto done;
 	}
 
-	vaddr = untagged_addr_remote(mm, vaddr);
+	vaddr = untag_remote_user_addr(mm, vaddr);
 
 retry:
 	vma = vma_lookup(mm, vaddr);
diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index 420510f6a545..7ccd3f341af4 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -1692,7 +1692,7 @@ static ssize_t pagemap_read(struct file *file, char __user *buf,
 		ret = mmap_read_lock_killable(mm);
 		if (ret)
 			goto out_free;
-		start_vaddr = untagged_addr_remote(mm, svpfn << PAGE_SHIFT);
+		start_vaddr = untag_remote_user_addr(mm, svpfn << PAGE_SHIFT);
 		mmap_read_unlock(mm);
 	}
 
diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h
index 3064314f4832..8c956172252f 100644
--- a/include/linux/uaccess.h
+++ b/include/linux/uaccess.h
@@ -25,8 +25,8 @@
 #define untagged_addr(addr) (addr)
 #endif
 
-#ifndef untagged_addr_remote
-#define untagged_addr_remote(mm, addr)	({		\
+#ifndef untag_remote_user_addr
+#define untag_remote_user_addr(mm, addr)	({		\
 	mmap_assert_locked(mm);				\
 	untagged_addr(addr);				\
 })
diff --git a/mm/gup.c b/mm/gup.c
index bbe416236593..d22bad9147d7 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -1085,7 +1085,7 @@ static long __get_user_pages(struct mm_struct *mm,
 	if (!nr_pages)
 		return 0;
 
-	start = untagged_addr_remote(mm, start);
+	start = untag_remote_user_addr(mm, start);
 
 	VM_BUG_ON(!!pages != !!(gup_flags & (FOLL_GET | FOLL_PIN)));
 
@@ -1259,7 +1259,7 @@ int fixup_user_fault(struct mm_struct *mm,
 	struct vm_area_struct *vma;
 	vm_fault_t ret;
 
-	address = untagged_addr_remote(mm, address);
+	address = untag_remote_user_addr(mm, address);
 
 	if (unlocked)
 		fault_flags |= FAULT_FLAG_ALLOW_RETRY | FAULT_FLAG_KILLABLE;
diff --git a/mm/madvise.c b/mm/madvise.c
index b5ffbaf616f5..ffb5d76a9481 100644
--- a/mm/madvise.c
+++ b/mm/madvise.c
@@ -1421,7 +1421,7 @@ int do_madvise(struct mm_struct *mm, unsigned long start, size_t len_in, int beh
 		mmap_read_lock(mm);
 	}
 
-	start = untagged_addr_remote(mm, start);
+	start = untag_remote_user_addr(mm, start);
 	end = start + len;
 
 	blk_start_plug(&plug);
diff --git a/mm/migrate.c b/mm/migrate.c
index 01cac26a3127..7647c0a6ff30 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -2109,7 +2109,7 @@ static int add_page_for_migration(struct mm_struct *mm, const void __user *p,
 	bool isolated;
 
 	mmap_read_lock(mm);
-	addr = (unsigned long)untagged_addr_remote(mm, p);
+	addr = (unsigned long)untag_remote_user_addr(mm, p);
 
 	err = -EFAULT;
 	vma = vma_lookup(mm, addr);