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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <10895316-e3fd-4483-9986-a6bcb217b20a@netscape.net>
Date: Wed, 11 Jun 2025 17:09:39 -0400
From: Chuck Zmudzinski <brchuckz@...scape.net>
To: Mike Rapoport <rppt@...nel.org>, Peter Zijlstra <peterz@...radead.org>
Cc: Borislav Petkov <bp@...en8.de>, Dave Hansen
 <dave.hansen@...ux.intel.com>, Ingo Molnar <mingo@...hat.com>,
 J�rgen Gro <jgross@...e.com>,
 "H. Peter Anvin" <hpa@...or.com>, Thomas Gleixner <tglx@...utronix.de>,
 Xin Li <xin@...or.com>, linux-kernel@...r.kernel.org,
 stable@...r.kernel.org, x86@...nel.org
Subject: Re: [PATCH 4/5] x86/its: explicitly manage permissions for ITS pages

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@...radead.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@...radead.org>
> > > Co-developed-by: Mike Rapoport (Microsoft) <rppt@...nel.org>
> > > Signed-off-by: Mike Rapoport (Microsoft) <rppt@...nel.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