[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <vi5eesl223u6s3zqb27kyaojnncsro3725escqp2mcrd2mhcbx@hbmfepqluveh>
Date: Thu, 30 Oct 2025 23:06:25 +0100
From: Mateusz Guzik <mjguzik@...il.com>
To: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: Thomas Gleixner <tglx@...utronix.de>, brauner@...nel.org, 
	viro@...iv.linux.org.uk, jack@...e.cz, linux-kernel@...r.kernel.org, 
	linux-fsdevel@...r.kernel.org, pfalcato@...e.de
Subject: Re: [PATCH v4] fs: hide names_cachep behind runtime access machinery
On Thu, Oct 30, 2025 at 10:39:46PM +0100, Mateusz Guzik wrote:
> On Thu, Oct 30, 2025 at 7:07 PM Linus Torvalds
> <torvalds@...ux-foundation.org> wrote:
> >
> > [ Adding Thomas, because he's been working on our x86 uaccess code,
> > and I actually think we get this all wrong for access_ok() etc ]
> >
> > On Thu, 30 Oct 2025 at 09:35, Mateusz Guzik <mjguzik@...il.com> wrote:
> > >
> > > I don't know if you are suggesting to make the entire thing fail to
> > > compile if included for a module, or to transparently convert
> > > runtime-optimized access into plain access.
> > >
> > > I presume the former.
> >
> > I think *including* it should be ok, because we have things like
> > <asm/uaccess.h> - or your addition to <linux/fs.h> - that use it for
> > core functionality that is then not supported for module use.
> >
> > Yeah, in a perfect world we'd have those things only in "internal"
> > headers and people couldn't include them even by mistake, but that
> > ends up being a pain.
> >
> > So I don't think your
> >
> > +#ifdef MODULE
> > +#error "this functionality is not available for modules"
> > +#endif
> >
> > model works, because I think it might be too painful to fix (but hey,
> > maybe I'm wrong).
> >
> 
> In my proposal the patch which messes with the namei cache address
> would have the following in fs.h:
> #ifndef MODULE
> #include <asm/runtime-const.h>
> #endif
> 
> As in, unless the kernel itself is being compiled, it would pretend
> the runtime machinery does not even exist, which imo is preferable to
> failing later at link time.
> 
> Then whatever functionality using runtime-const is straight up not
> available and code insisting on providing something for modules anyway
> is forced to provide an ifdefed implementation.
> 
Here is a build-tested diff for bzImage itself and M=fs/erofs on the
x86-64 architecture.
It keeps access_ok() inline for demostrative purposes, I have no opinion
what to do with this specific sucker.
diff --git a/arch/x86/include/asm/runtime-const.h b/arch/x86/include/asm/runtime-const.h
index 8d983cfd06ea..dc3273ac2034 100644
--- a/arch/x86/include/asm/runtime-const.h
+++ b/arch/x86/include/asm/runtime-const.h
@@ -2,6 +2,10 @@
 #ifndef _ASM_RUNTIME_CONST_H
 #define _ASM_RUNTIME_CONST_H
 
+#ifdef MODULE
+#error "this functionality is not available for modules"
+#endif
+
 #ifdef __ASSEMBLY__
 
 .macro RUNTIME_CONST_PTR sym reg
diff --git a/arch/x86/include/asm/uaccess_64.h b/arch/x86/include/asm/uaccess_64.h
index c8a5ae35c871..ce8f6be1964e 100644
--- a/arch/x86/include/asm/uaccess_64.h
+++ b/arch/x86/include/asm/uaccess_64.h
@@ -12,13 +12,14 @@
 #include <asm/cpufeatures.h>
 #include <asm/page.h>
 #include <asm/percpu.h>
-#include <asm/runtime-const.h>
 
-/*
- * Virtual variable: there's no actual backing store for this,
- * it can purely be used as 'runtime_const_ptr(USER_PTR_MAX)'
- */
 extern unsigned long USER_PTR_MAX;
+#ifdef MODULE
+#define __USER_PTR_MAX	USER_PTR_MAX
+#else
+#include <asm/runtime-const.h>
+#define __USER_PTR_MAX runtime_const_ptr(USER_PTR_MAX)
+#endif
 
 #ifdef CONFIG_ADDRESS_MASKING
 /*
@@ -54,7 +55,7 @@ static inline unsigned long __untagged_addr_remote(struct mm_struct *mm,
 #endif
 
 #define valid_user_address(x) \
-	likely((__force unsigned long)(x) <= runtime_const_ptr(USER_PTR_MAX))
+	likely((__force unsigned long)(x) <= __USER_PTR_MAX)
 
 /*
  * Masking the user address is an alternative to a conditional
@@ -67,7 +68,7 @@ static inline void __user *mask_user_address(const void __user *ptr)
 	asm("cmp %1,%0\n\t"
 	    "cmova %1,%0"
 		:"=r" (ret)
-		:"r" (runtime_const_ptr(USER_PTR_MAX)),
+		:"r" (__USER_PTR_MAX),
 		 "0" (ptr));
 	return ret;
 }
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 3ff9682d8bc4..5a3d89ed75d1 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -78,6 +78,9 @@
 DEFINE_PER_CPU_READ_MOSTLY(struct cpuinfo_x86, cpu_info);
 EXPORT_PER_CPU_SYMBOL(cpu_info);
 
+unsigned long USER_PTR_MAX __ro_after_init = TASK_SIZE_MAX;
+EXPORT_SYMBOL(USER_PTR_MAX);
+
 u32 elf_hwcap2 __read_mostly;
 
 /* Number of siblings per CPU package */
@@ -2575,8 +2578,6 @@ void __init arch_cpu_finalize_init(void)
 	alternative_instructions();
 
 	if (IS_ENABLED(CONFIG_X86_64)) {
-		unsigned long USER_PTR_MAX = TASK_SIZE_MAX;
-
 		/*
 		 * Enable this when LAM is gated on LASS support
 		if (cpu_feature_enabled(X86_FEATURE_LAM))
Powered by blists - more mailing lists
 
