Re: [PATCH 4/5] x86/its: explicitly manage permissions for ITS pages

[Chuck Zmudzinski <brchuckz@netscape.net>]

On 6/3/25 10:36 AM, Mike Rapoport wrote:
> On Tue, Jun 03, 2025 at 03:58:45PM +0200, Peter Zijlstra wrote:
> > On Tue, Jun 03, 2025 at 02:14:44PM +0300, Mike Rapoport wrote:
> > > From: "Peter Zijlstra (Intel)" <peterz@infradead.org>
> > > 
> > > execmem_alloc() sets permissions differently depending on the kernel
> > > configuration, CPU support for PSE and whether a page is allocated
> > > before or after mark_rodata_ro().
> > > 
> > > Add tracking for pages allocated for ITS when patching the core kernel
> > > and make sure the permissions for ITS pages are explicitly managed for
> > > both kernel and module allocations.
> > > 
> > > Fixes: 872df34d7c51 ("x86/its: Use dynamic thunks for indirect branches")
> > > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> > > Co-developed-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
> > > Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
> > > ---
> > 
> > How about something like this on top?
>
> Works for me :)
>
> > --- a/arch/x86/kernel/alternative.c
> > +++ b/arch/x86/kernel/alternative.c
> > @@ -121,7 +121,6 @@ struct its_array its_pages;
> >  static void *__its_alloc(struct its_array *pages)
> >  {
> >  	void *page __free(execmem) = execmem_alloc(EXECMEM_MODULE_TEXT, PAGE_SIZE);
> > -
> >  	if (!page)
> >  		return NULL;
> >  
> > @@ -172,6 +171,9 @@ static void *its_init_thunk(void *thunk,
> >  
> >  static void its_pages_protect(struct its_array *pages)
> >  {
> > +	if (!IS_ENABLED(CONFIG_STRICT_KERNEL_RWX))
> > +		return;
> > +
>
> But modules generally use STRICT_MODULE_RWX.
> Do you want to make the its pages stricter than normal module text?
>
> >  	for (int i = 0; i < pages->num; i++) {
> >  		void *page = pages->pages[i];
> >  		execmem_restore_rox(page, PAGE_SIZE);
> > @@ -180,8 +182,7 @@ static void its_pages_protect(struct its
> >  
> >  static void its_fini_core(void)
> >  {
> > -	if (IS_ENABLED(CONFIG_STRICT_KERNEL_RWX))
> > -		its_pages_protect(&its_pages);
> > +	its_pages_protect(&its_pages);
> >  	kfree(its_pages.pages);
> >  }
> >  
> > @@ -207,8 +208,7 @@ void its_fini_mod(struct module *mod)
> >  	its_page = NULL;
> >  	mutex_unlock(&text_mutex);
> >  
> > -	if (IS_ENABLED(CONFIG_STRICT_MODULE_RWX))
> > -		its_pages_protect(&mod->arch.its_pages);
> > +	its_pages_protect(&mod->arch.its_pages);
> >  }
>

I tested the 5-patch series without this 6th patch on top of 6.15.2 released yesterday
and it fixes a crash I get in 6.15.2 in my Xen PV dom0 without this patch set.

I also  tried this 6th patch (including Mike's suggested removal of the three lines from
Peter's add-on 6th patch ) but I got a kernel panic in Xen PV dom0 with this extra patch.

Chuck Zmudzinski