Re: [PATCH 6.6 000/113] 6.6.91-rc2 review

[Pawan Gupta <pawan.kumar.gupta@linux.intel.com>]

On Mon, May 19, 2025 at 05:27:30PM +0200, Greg Kroah-Hartman wrote:
> On Thu, May 15, 2025 at 08:25:57AM -0700, Pawan Gupta wrote:
> > On Thu, May 15, 2025 at 07:35:26AM +0200, Greg Kroah-Hartman wrote:
> > > On Wed, May 14, 2025 at 01:49:06PM -0700, Guenter Roeck wrote:
> > > > On 5/14/25 13:33, Harshit Mogalapalli wrote:
> > > > > Hi Greg,
> > > > > 
> > > > > On 15/05/25 01:35, Greg Kroah-Hartman wrote:
> > > > > > On Thu, May 15, 2025 at 12:29:40AM +0530, Harshit Mogalapalli wrote:
> > > > > > > Hi Greg,
> > > > > > > On 14/05/25 18:34, Greg Kroah-Hartman wrote:
> > > > > > > > This is the start of the stable review cycle for the 6.6.91 release.
> > > > > > > > There are 113 patches in this series, all will be posted as a response
> > > > > > > > to this one.  If anyone has any issues with these being applied, please
> > > > > > > > let me know.
> > > > > > > > 
> > > > > > > > Responses should be made by Fri, 16 May 2025 12:55:38 +0000.
> > > > > > > > Anything received after that time might be too late.
> > > > > > > 
> > > > > > > ld: vmlinux.o: in function `patch_retpoline':
> > > > > > > alternative.c:(.text+0x3b6f1): undefined reference to `module_alloc'
> > > > > > > make[2]: *** [scripts/Makefile.vmlinux:37: vmlinux] Error 1
> > > > > > > 
> > > > > > > We see this build error in 6.6.91-rc2 tag.
> > > > > > 
> > > > > > What is odd about your .config?  Have a link to it?  I can't duplicate
> > > > > > it here on my builds.
> > > > > > 
> > > > > 
> > > > > So this is a config where CONFIG_MODULES is unset(!=y) -- with that we could reproduce it on defconfig + disabling CONFIG_MODULES as well.
> > > > > 
> > > > 
> > > > Key is the combination of CONFIG_MODULES=n with CONFIG_MITIGATION_ITS=y.
> > > 
> > > Ah, this is due to the change in its_alloc() for 6.6.y and 6.1.y by the
> > > call to module_alloc() instead of execmem_alloc() in the backport of
> > > 872df34d7c51 ("x86/its: Use dynamic thunks for indirect branches").
> > 
> > Sorry for the trouble. I wish I had a test to catch problems like this. The
> > standard config targets defconfig, allyesconfig, allnoconfig, etc. do not
> > expose such issues. The only thing that comes close is randconfig.
> > 
> > CONFIG_MODULES=n is not a common setting, I wonder how people find such
> > issues? (trying to figure out how to prevent such issues in future).
> > 
> > > Pawan, any hints on what should be done here instead?
> > 
> > Since dynamic thunks are not possible without CONFIG_MODULES, one option is
> > to adjust the already in 6.6.91-rc2 patch 9f35e331144a (x86/its: Fix build
> > errors when CONFIG_MODULES=n) to also bring the ITS thunk allocation under
> > CONFIG_MODULES.
> > 
> > I am not seeing any issue with below build and boot test:
> > 
> >   #!/bin/bash -ex
> > 
> >   ./scripts/config --disable CONFIG_MODULES
> >   ./scripts/config --disable CONFIG_MITIGATION_ITS
> >   # https://github.com/arighi/virtme-ng
> >   vng -b
> >   vng -- lscpu
> > 
> >   # main test
> >   ./scripts/config --disable CONFIG_MODULES
> >   ./scripts/config --enable CONFIG_MITIGATION_ITS
> >   vng -b
> >   vng -- lscpu
> > 
> >   ./scripts/config --enable CONFIG_MODULES
> >   ./scripts/config --disable CONFIG_MITIGATION_ITS
> >   vng -b
> >   vng -- lscpu
> > 
> >   ./scripts/config --enable CONFIG_MODULES
> >   ./scripts/config --enable CONFIG_MITIGATION_ITS
> >   vng -b
> >   vng -- lscpu
> > 
> >   echo "PASS"
> > 
> > Similar change is required for 6.1 and 5.15 as well. 6.12 is fine because
> > it uses execmem_alloc().
> > 
> > --- 8< ---
> > From: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
> > Subject: [PATCH 6.6] x86/its: Fix build errors when CONFIG_MODULES=n
> > 
> > From: Eric Biggers <ebiggers@google.com>
> > 
> > commit 9f35e33144ae5377d6a8de86dd3bd4d995c6ac65 upstream.
> > 
> > Fix several build errors when CONFIG_MODULES=n, including the following:
> > 
> > ../arch/x86/kernel/alternative.c:195:25: error: incomplete definition of type 'struct module'
> >   195 |         for (int i = 0; i < mod->its_num_pages; i++) {
> > 
> >   [ pawan: backport: Bring ITS dynamic thunk code under CONFIG_MODULES ]
> > 
> > Fixes: 872df34d7c51 ("x86/its: Use dynamic thunks for indirect branches")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Eric Biggers <ebiggers@google.com>
> > Acked-by: Dave Hansen <dave.hansen@intel.com>
> > Tested-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> > Reviewed-by: Alexandre Chartre <alexandre.chartre@oracle.com>
> > Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > ---
> >  arch/x86/kernel/alternative.c | 12 +++++++++++-
> >  1 file changed, 11 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
> > index 6085919d3b3e..c6d9a3882ec8 100644
> > --- a/arch/x86/kernel/alternative.c
> > +++ b/arch/x86/kernel/alternative.c
> > @@ -129,6 +129,7 @@ const unsigned char * const x86_nops[ASM_NOP_MAX+1] =
> >  
> >  #ifdef CONFIG_MITIGATION_ITS
> >  
> > +#ifdef CONFIG_MODULES
> >  static struct module *its_mod;
> >  static void *its_page;
> >  static unsigned int its_offset;
> > @@ -244,7 +245,16 @@ static void *its_allocate_thunk(int reg)
> >  	return thunk;
> >  }
> >  
> > -#endif
> > +#else /* CONFIG_MODULES */
> > +
> > +static void *its_allocate_thunk(int reg)
> > +{
> > +	return NULL;
> > +}
> > +
> > +#endif /* CONFIG_MODULES */
> > +
> > +#endif /* CONFIG_MITIGATION_ITS */
> >  
> >  /*
> >   * Fill the buffer with a single effective instruction of size @len.
> > -- 
> > 2.34.1
> > 
> 
> This looks to still be causing problems, see these two reports of build
> problems with the latest 6.1 and 6.6 releases with this commit in it:
> 	https://lore.kernel.org/r/20250519164717.18738b4e@ncopa-desktop
> 	https://lore.kernel.org/r/2f1ae598-0339-4e17-8156-03e8525a213d@roeck-us.net

These reports appear to be related to a merge resolution of "x86/its:
FineIBT-paranoid vs ITS" in 6.6 (and 6.1):

https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=v6.6.91&id=772934d9062a0f7297ad4e5bffbd904208655660

which is different from 6.12:

https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=v6.12.29&id=7e78061be78b8593df9b0cd0f21b1fee425035de

Basically its_static_thunk() needs to be under #ifdef CONFIG_MITIGATION_ITS

I guess there needs to be a separate patch to fix this?