[PATCH 0/3] x86: Fix some bugs related to ITS mitigation

[PATCH 0/3] x86: Fix some bugs related to ITS mitigation

[Juergen Gross]

Running as a Xen PV guest uncovered some bugs when ITS mitigation is
active.

Juergen Gross (3):
  x86/execmem: don't use PAGE_KERNEL protection for code pages
  x86/mm/pat: don't collapse pages without PSE set
  x86/alternative: make kernel ITS thunks read-only

 arch/x86/kernel/alternative.c | 16 ++++++++++++++++
 arch/x86/mm/init.c            |  2 +-
 arch/x86/mm/pat/set_memory.c  |  3 +++
 3 files changed, 20 insertions(+), 1 deletion(-)

-- 
2.43.0

[PATCH 1/3] x86/execmem: don't use PAGE_KERNEL protection for code pages

[Juergen Gross]

In case X86_FEATURE_PSE isn't available (e.g. when running as a Xen
PV guest), execmem_arch_setup() will fall back to use PAGE_KERNEL
protection for the EXECMEM_MODULE_TEXT range.

This will result in attempts to execute code with the NX bit set in
case of ITS mitigation being applied.

Avoid this problem by using PAGE_KERNEL_EXEC protection instead,
which will not set the NX bit.

Cc: <stable@vger.kernel.org>
Reported-by: Xin Li <xin@zytor.com>
Fixes: 5185e7f9f3bd ("x86/module: enable ROX caches for module text on 64 bit")
Signed-off-by: Juergen Gross <jgross@suse.com>
---
 arch/x86/mm/init.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
index 7456df985d96..f5012ae31d8b 100644
--- a/arch/x86/mm/init.c
+++ b/arch/x86/mm/init.c
@@ -1089,7 +1089,7 @@ struct execmem_info __init *execmem_arch_setup(void)
 		pgprot = PAGE_KERNEL_ROX;
 		flags = EXECMEM_KASAN_SHADOW | EXECMEM_ROX_CACHE;
 	} else {
-		pgprot = PAGE_KERNEL;
+		pgprot = PAGE_KERNEL_EXEC;
 		flags = EXECMEM_KASAN_SHADOW;
 	}
 
-- 
2.43.0

Re: [PATCH 1/3] x86/execmem: don't use PAGE_KERNEL protection for code pages

[Mike Rapoport]

On Wed, May 28, 2025 at 02:35:55PM +0200, Juergen Gross wrote:
> In case X86_FEATURE_PSE isn't available (e.g. when running as a Xen
> PV guest), execmem_arch_setup() will fall back to use PAGE_KERNEL
> protection for the EXECMEM_MODULE_TEXT range.
> 
> This will result in attempts to execute code with the NX bit set in
> case of ITS mitigation being applied.
> 
> Avoid this problem by using PAGE_KERNEL_EXEC protection instead,
> which will not set the NX bit.
> 
> Cc: <stable@vger.kernel.org>
> Reported-by: Xin Li <xin@zytor.com>
> Fixes: 5185e7f9f3bd ("x86/module: enable ROX caches for module text on 64 bit")
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
>  arch/x86/mm/init.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
> index 7456df985d96..f5012ae31d8b 100644
> --- a/arch/x86/mm/init.c
> +++ b/arch/x86/mm/init.c
> @@ -1089,7 +1089,7 @@ struct execmem_info __init *execmem_arch_setup(void)
>  		pgprot = PAGE_KERNEL_ROX;
>  		flags = EXECMEM_KASAN_SHADOW | EXECMEM_ROX_CACHE;
>  	} else {
> -		pgprot = PAGE_KERNEL;
> +		pgprot = PAGE_KERNEL_EXEC;

Please don't. Everything except ITS can work with PAGE_KENREL so the fix
should be on ITS side. 

>  		flags = EXECMEM_KASAN_SHADOW;
>  	}
>  
> -- 
> 2.43.0
> 

-- 
Sincerely yours,
Mike.

Re: [PATCH 1/3] x86/execmem: don't use PAGE_KERNEL protection for code pages

[Jürgen Groß]

[-- Attachment #1.1.1: Type: text/plain, Size: 1493 bytes --]

On 28.05.25 19:27, Mike Rapoport wrote:
> On Wed, May 28, 2025 at 02:35:55PM +0200, Juergen Gross wrote:
>> In case X86_FEATURE_PSE isn't available (e.g. when running as a Xen
>> PV guest), execmem_arch_setup() will fall back to use PAGE_KERNEL
>> protection for the EXECMEM_MODULE_TEXT range.
>>
>> This will result in attempts to execute code with the NX bit set in
>> case of ITS mitigation being applied.
>>
>> Avoid this problem by using PAGE_KERNEL_EXEC protection instead,
>> which will not set the NX bit.
>>
>> Cc: <stable@vger.kernel.org>
>> Reported-by: Xin Li <xin@zytor.com>
>> Fixes: 5185e7f9f3bd ("x86/module: enable ROX caches for module text on 64 bit")
>> Signed-off-by: Juergen Gross <jgross@suse.com>
>> ---
>>   arch/x86/mm/init.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
>> index 7456df985d96..f5012ae31d8b 100644
>> --- a/arch/x86/mm/init.c
>> +++ b/arch/x86/mm/init.c
>> @@ -1089,7 +1089,7 @@ struct execmem_info __init *execmem_arch_setup(void)
>>   		pgprot = PAGE_KERNEL_ROX;
>>   		flags = EXECMEM_KASAN_SHADOW | EXECMEM_ROX_CACHE;
>>   	} else {
>> -		pgprot = PAGE_KERNEL;
>> +		pgprot = PAGE_KERNEL_EXEC;
> 
> Please don't. Everything except ITS can work with PAGE_KENREL so the fix
> should be on ITS side.

Hmm, maybe adding another element to execmem_info[] with the new
type EXECMEM_KERNEL_TEXT, specifying PAGE_KERNEL_EXEC if !PSE?


Juergen

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3743 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

Re: [PATCH 1/3] x86/execmem: don't use PAGE_KERNEL protection for code pages

[Peter Zijlstra]

On Wed, May 28, 2025 at 08:27:19PM +0300, Mike Rapoport wrote:
> On Wed, May 28, 2025 at 02:35:55PM +0200, Juergen Gross wrote:
> > In case X86_FEATURE_PSE isn't available (e.g. when running as a Xen
> > PV guest), execmem_arch_setup() will fall back to use PAGE_KERNEL
> > protection for the EXECMEM_MODULE_TEXT range.
> > 
> > This will result in attempts to execute code with the NX bit set in
> > case of ITS mitigation being applied.
> > 
> > Avoid this problem by using PAGE_KERNEL_EXEC protection instead,
> > which will not set the NX bit.
> > 
> > Cc: <stable@vger.kernel.org>
> > Reported-by: Xin Li <xin@zytor.com>
> > Fixes: 5185e7f9f3bd ("x86/module: enable ROX caches for module text on 64 bit")
> > Signed-off-by: Juergen Gross <jgross@suse.com>
> > ---
> >  arch/x86/mm/init.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
> > index 7456df985d96..f5012ae31d8b 100644
> > --- a/arch/x86/mm/init.c
> > +++ b/arch/x86/mm/init.c
> > @@ -1089,7 +1089,7 @@ struct execmem_info __init *execmem_arch_setup(void)
> >  		pgprot = PAGE_KERNEL_ROX;
> >  		flags = EXECMEM_KASAN_SHADOW | EXECMEM_ROX_CACHE;
> >  	} else {
> > -		pgprot = PAGE_KERNEL;
> > +		pgprot = PAGE_KERNEL_EXEC;
> 
> Please don't. Everything except ITS can work with PAGE_KENREL so the fix
> should be on ITS side. 

Well, this is early vs post make_ro again.

Does something like so work for you?

---
diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
index 7456df985d96..f5012ae31d8b 100644
--- a/arch/x86/mm/init.c
+++ b/arch/x86/mm/init.c
@@ -1089,7 +1089,7 @@ struct execmem_info __init *execmem_arch_setup(void)
 		pgprot = PAGE_KERNEL_ROX;
 		flags = EXECMEM_KASAN_SHADOW | EXECMEM_ROX_CACHE;
 	} else {
-		pgprot = PAGE_KERNEL;
+		pgprot = PAGE_KERNEL_EXEC;
 		flags = EXECMEM_KASAN_SHADOW;
 	}
 
diff --git a/mm/execmem.c b/mm/execmem.c
index 6f7a2653b280..dbe2eedea0e6 100644
--- a/mm/execmem.c
+++ b/mm/execmem.c
@@ -258,6 +258,7 @@ static bool execmem_cache_rox = false;
 
 void execmem_cache_make_ro(void)
 {
+	struct execmem_range *module_text = &execmem_info->ranges[EXECMEM_MODULE_TEXT];
 	struct maple_tree *free_areas = &execmem_cache.free_areas;
 	struct maple_tree *busy_areas = &execmem_cache.busy_areas;
 	MA_STATE(mas_free, free_areas, 0, ULONG_MAX);
@@ -269,6 +270,9 @@ void execmem_cache_make_ro(void)
 
 	mutex_lock(mutex);
 
+	if (!(module_text->flags & EXECMEM_ROX_CACHE))
+		module_text->pgprot = PAGE_KERNEL;
+
 	mas_for_each(&mas_free, area, ULONG_MAX) {
 		unsigned long pages = mas_range_len(&mas_free) >> PAGE_SHIFT;
 		set_memory_ro(mas_free.index, pages);

[PATCH 2/3] x86/mm/pat: don't collapse pages without PSE set

[Juergen Gross]

Collapsing pages to a leaf PMD or PUD should be done only if
X86_FEATURE_PSE is available, which is not the case when running e.g.
as a Xen PV guest.

Cc: <stable@vger.kernel.org>
Fixes: 41d88484c71c ("x86/mm/pat: restore large ROX pages after fragmentation")
Signed-off-by: Juergen Gross <jgross@suse.com>
---
 arch/x86/mm/pat/set_memory.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/x86/mm/pat/set_memory.c b/arch/x86/mm/pat/set_memory.c
index 30ab4aced761..e6a6ac8cdc1d 100644
--- a/arch/x86/mm/pat/set_memory.c
+++ b/arch/x86/mm/pat/set_memory.c
@@ -1257,6 +1257,9 @@ static int collapse_pmd_page(pmd_t *pmd, unsigned long addr,
 	pgprot_t pgprot;
 	int i = 0;
 
+	if (!cpu_feature_enabled(X86_FEATURE_PSE))
+		return 0;
+
 	addr &= PMD_MASK;
 	pte = pte_offset_kernel(pmd, addr);
 	first = *pte;
-- 
2.43.0

[tip: x86/urgent] x86/mm/pat: don't collapse pages without PSE set

[tip-bot2 for Juergen Gross]

The following commit has been merged into the x86/urgent branch of tip:

Commit-ID:     1dbf30fdb5e57fb2c39f17f35f2b544d5de34397
Gitweb:        https://git.kernel.org/tip/1dbf30fdb5e57fb2c39f17f35f2b544d5de34397
Author:        Juergen Gross <jgross@suse.com>
AuthorDate:    Tue, 03 Jun 2025 14:14:41 +03:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Wed, 11 Jun 2025 11:20:51 +02:00

x86/mm/pat: don't collapse pages without PSE set

Collapsing pages to a leaf PMD or PUD should be done only if
X86_FEATURE_PSE is available, which is not the case when running e.g.
as a Xen PV guest.

Fixes: 41d88484c71c ("x86/mm/pat: restore large ROX pages after fragmentation")
Signed-off-by: Juergen Gross <jgross@suse.com>
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/r/20250528123557.12847-3-jgross@suse.com
---
 arch/x86/mm/pat/set_memory.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/x86/mm/pat/set_memory.c b/arch/x86/mm/pat/set_memory.c
index 46edc11..8834c76 100644
--- a/arch/x86/mm/pat/set_memory.c
+++ b/arch/x86/mm/pat/set_memory.c
@@ -1257,6 +1257,9 @@ static int collapse_pmd_page(pmd_t *pmd, unsigned long addr,
 	pgprot_t pgprot;
 	int i = 0;
 
+	if (!cpu_feature_enabled(X86_FEATURE_PSE))
+		return 0;
+
 	addr &= PMD_MASK;
 	pte = pte_offset_kernel(pmd, addr);
 	first = *pte;

[PATCH 3/3] x86/alternative: make kernel ITS thunks read-only

[Juergen Gross]

When allocating memory pages for kernel ITS thunks, make them read-only
after having written the last thunk.

This will be needed when X86_FEATURE_PSE isn't available, as the thunk
memory will have PAGE_KERNEL_EXEC protection, which is including the
write permission.

Cc: <stable@vger.kernel.org>
Fixes: 5185e7f9f3bd ("x86/module: enable ROX caches for module text on 64 bit")
Signed-off-by: Juergen Gross <jgross@suse.com>
---
 arch/x86/kernel/alternative.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index ecfe7b497cad..bd974a0ac88a 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -217,6 +217,15 @@ static void *its_alloc(void)
 	return no_free_ptr(page);
 }
 
+static void its_set_kernel_ro(void *addr)
+{
+#ifdef CONFIG_MODULES
+	if (its_mod)
+		return;
+#endif
+	execmem_restore_rox(addr, PAGE_SIZE);
+}
+
 static void *its_allocate_thunk(int reg)
 {
 	int size = 3 + (reg / 8);
@@ -234,6 +243,8 @@ static void *its_allocate_thunk(int reg)
 #endif
 
 	if (!its_page || (its_offset + size - 1) >= PAGE_SIZE) {
+		if (its_page)
+			its_set_kernel_ro(its_page);
 		its_page = its_alloc();
 		if (!its_page) {
 			pr_err("ITS page allocation failed\n");
@@ -2338,6 +2349,11 @@ void __init alternative_instructions(void)
 	apply_retpolines(__retpoline_sites, __retpoline_sites_end);
 	apply_returns(__return_sites, __return_sites_end);
 
+	/* Make potential last thunk page read-only. */
+	if (its_page)
+		its_set_kernel_ro(its_page);
+	its_page = NULL;
+
 	/*
 	 * Adjust all CALL instructions to point to func()-10, including
 	 * those in .altinstr_replacement.
-- 
2.43.0

Re: [PATCH 3/3] x86/alternative: make kernel ITS thunks read-only

[Peter Zijlstra]

On Wed, May 28, 2025 at 02:35:57PM +0200, Juergen Gross wrote:
> When allocating memory pages for kernel ITS thunks, make them read-only
> after having written the last thunk.
> 
> This will be needed when X86_FEATURE_PSE isn't available, as the thunk
> memory will have PAGE_KERNEL_EXEC protection, which is including the
> write permission.
> 
> Cc: <stable@vger.kernel.org>
> Fixes: 5185e7f9f3bd ("x86/module: enable ROX caches for module text on 64 bit")
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
>  arch/x86/kernel/alternative.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
> index ecfe7b497cad..bd974a0ac88a 100644
> --- a/arch/x86/kernel/alternative.c
> +++ b/arch/x86/kernel/alternative.c
> @@ -217,6 +217,15 @@ static void *its_alloc(void)
>  	return no_free_ptr(page);
>  }
>  
> +static void its_set_kernel_ro(void *addr)
> +{
> +#ifdef CONFIG_MODULES
> +	if (its_mod)
> +		return;
> +#endif
> +	execmem_restore_rox(addr, PAGE_SIZE);
> +}
> +
>  static void *its_allocate_thunk(int reg)
>  {
>  	int size = 3 + (reg / 8);
> @@ -234,6 +243,8 @@ static void *its_allocate_thunk(int reg)
>  #endif
>  
>  	if (!its_page || (its_offset + size - 1) >= PAGE_SIZE) {
> +		if (its_page)
> +			its_set_kernel_ro(its_page);
>  		its_page = its_alloc();
>  		if (!its_page) {
>  			pr_err("ITS page allocation failed\n");
> @@ -2338,6 +2349,11 @@ void __init alternative_instructions(void)
>  	apply_retpolines(__retpoline_sites, __retpoline_sites_end);
>  	apply_returns(__return_sites, __return_sites_end);
>  
> +	/* Make potential last thunk page read-only. */
> +	if (its_page)
> +		its_set_kernel_ro(its_page);
> +	its_page = NULL;
> +
>  	/*
>  	 * Adjust all CALL instructions to point to func()-10, including
>  	 * those in .altinstr_replacement.

No, this is all sorts of wrong. Execmem API should ensure this.

Re: [PATCH 3/3] x86/alternative: make kernel ITS thunks read-only

[Jürgen Groß]

[-- Attachment #1.1.1: Type: text/plain, Size: 2237 bytes --]

On 28.05.25 15:10, Peter Zijlstra wrote:
> On Wed, May 28, 2025 at 02:35:57PM +0200, Juergen Gross wrote:
>> When allocating memory pages for kernel ITS thunks, make them read-only
>> after having written the last thunk.
>>
>> This will be needed when X86_FEATURE_PSE isn't available, as the thunk
>> memory will have PAGE_KERNEL_EXEC protection, which is including the
>> write permission.
>>
>> Cc: <stable@vger.kernel.org>
>> Fixes: 5185e7f9f3bd ("x86/module: enable ROX caches for module text on 64 bit")
>> Signed-off-by: Juergen Gross <jgross@suse.com>
>> ---
>>   arch/x86/kernel/alternative.c | 16 ++++++++++++++++
>>   1 file changed, 16 insertions(+)
>>
>> diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
>> index ecfe7b497cad..bd974a0ac88a 100644
>> --- a/arch/x86/kernel/alternative.c
>> +++ b/arch/x86/kernel/alternative.c
>> @@ -217,6 +217,15 @@ static void *its_alloc(void)
>>   	return no_free_ptr(page);
>>   }
>>   
>> +static void its_set_kernel_ro(void *addr)
>> +{
>> +#ifdef CONFIG_MODULES
>> +	if (its_mod)
>> +		return;
>> +#endif
>> +	execmem_restore_rox(addr, PAGE_SIZE);
>> +}
>> +
>>   static void *its_allocate_thunk(int reg)
>>   {
>>   	int size = 3 + (reg / 8);
>> @@ -234,6 +243,8 @@ static void *its_allocate_thunk(int reg)
>>   #endif
>>   
>>   	if (!its_page || (its_offset + size - 1) >= PAGE_SIZE) {
>> +		if (its_page)
>> +			its_set_kernel_ro(its_page);
>>   		its_page = its_alloc();
>>   		if (!its_page) {
>>   			pr_err("ITS page allocation failed\n");
>> @@ -2338,6 +2349,11 @@ void __init alternative_instructions(void)
>>   	apply_retpolines(__retpoline_sites, __retpoline_sites_end);
>>   	apply_returns(__return_sites, __return_sites_end);
>>   
>> +	/* Make potential last thunk page read-only. */
>> +	if (its_page)
>> +		its_set_kernel_ro(its_page);
>> +	its_page = NULL;
>> +
>>   	/*
>>   	 * Adjust all CALL instructions to point to func()-10, including
>>   	 * those in .altinstr_replacement.
> 
> No, this is all sorts of wrong. Execmem API should ensure this.

You are aware that this patch is basically mirroring the work which is
already done for modules in alternative.c?


Juergen

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3743 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

Re: [PATCH 3/3] x86/alternative: make kernel ITS thunks read-only

[Peter Zijlstra]

On Wed, May 28, 2025 at 03:19:24PM +0200, Jürgen Groß wrote:
> On 28.05.25 15:10, Peter Zijlstra wrote:
> > On Wed, May 28, 2025 at 02:35:57PM +0200, Juergen Gross wrote:
> > > When allocating memory pages for kernel ITS thunks, make them read-only
> > > after having written the last thunk.
> > > 
> > > This will be needed when X86_FEATURE_PSE isn't available, as the thunk
> > > memory will have PAGE_KERNEL_EXEC protection, which is including the
> > > write permission.
> > > 
> > > Cc: <stable@vger.kernel.org>
> > > Fixes: 5185e7f9f3bd ("x86/module: enable ROX caches for module text on 64 bit")
> > > Signed-off-by: Juergen Gross <jgross@suse.com>
> > > ---
> > >   arch/x86/kernel/alternative.c | 16 ++++++++++++++++
> > >   1 file changed, 16 insertions(+)
> > > 
> > > diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
> > > index ecfe7b497cad..bd974a0ac88a 100644
> > > --- a/arch/x86/kernel/alternative.c
> > > +++ b/arch/x86/kernel/alternative.c
> > > @@ -217,6 +217,15 @@ static void *its_alloc(void)
> > >   	return no_free_ptr(page);
> > >   }
> > > +static void its_set_kernel_ro(void *addr)
> > > +{
> > > +#ifdef CONFIG_MODULES
> > > +	if (its_mod)
> > > +		return;
> > > +#endif
> > > +	execmem_restore_rox(addr, PAGE_SIZE);
> > > +}
> > > +
> > >   static void *its_allocate_thunk(int reg)
> > >   {
> > >   	int size = 3 + (reg / 8);
> > > @@ -234,6 +243,8 @@ static void *its_allocate_thunk(int reg)
> > >   #endif
> > >   	if (!its_page || (its_offset + size - 1) >= PAGE_SIZE) {
> > > +		if (its_page)
> > > +			its_set_kernel_ro(its_page);
> > >   		its_page = its_alloc();
> > >   		if (!its_page) {
> > >   			pr_err("ITS page allocation failed\n");
> > > @@ -2338,6 +2349,11 @@ void __init alternative_instructions(void)
> > >   	apply_retpolines(__retpoline_sites, __retpoline_sites_end);
> > >   	apply_returns(__return_sites, __return_sites_end);
> > > +	/* Make potential last thunk page read-only. */
> > > +	if (its_page)
> > > +		its_set_kernel_ro(its_page);
> > > +	its_page = NULL;
> > > +
> > >   	/*
> > >   	 * Adjust all CALL instructions to point to func()-10, including
> > >   	 * those in .altinstr_replacement.
> > 
> > No, this is all sorts of wrong. Execmem API should ensure this.
> 
> You are aware that this patch is basically mirroring the work which is
> already done for modules in alternative.c?

I am having trouble parsing that -- where does alternative.c do this to
modules?

Re: [PATCH 3/3] x86/alternative: make kernel ITS thunks read-only

[Jürgen Groß]

[-- Attachment #1.1.1: Type: text/plain, Size: 2586 bytes --]

On 28.05.25 15:22, Peter Zijlstra wrote:
> On Wed, May 28, 2025 at 03:19:24PM +0200, Jürgen Groß wrote:
>> On 28.05.25 15:10, Peter Zijlstra wrote:
>>> On Wed, May 28, 2025 at 02:35:57PM +0200, Juergen Gross wrote:
>>>> When allocating memory pages for kernel ITS thunks, make them read-only
>>>> after having written the last thunk.
>>>>
>>>> This will be needed when X86_FEATURE_PSE isn't available, as the thunk
>>>> memory will have PAGE_KERNEL_EXEC protection, which is including the
>>>> write permission.
>>>>
>>>> Cc: <stable@vger.kernel.org>
>>>> Fixes: 5185e7f9f3bd ("x86/module: enable ROX caches for module text on 64 bit")
>>>> Signed-off-by: Juergen Gross <jgross@suse.com>
>>>> ---
>>>>    arch/x86/kernel/alternative.c | 16 ++++++++++++++++
>>>>    1 file changed, 16 insertions(+)
>>>>
>>>> diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
>>>> index ecfe7b497cad..bd974a0ac88a 100644
>>>> --- a/arch/x86/kernel/alternative.c
>>>> +++ b/arch/x86/kernel/alternative.c
>>>> @@ -217,6 +217,15 @@ static void *its_alloc(void)
>>>>    	return no_free_ptr(page);
>>>>    }
>>>> +static void its_set_kernel_ro(void *addr)
>>>> +{
>>>> +#ifdef CONFIG_MODULES
>>>> +	if (its_mod)
>>>> +		return;
>>>> +#endif
>>>> +	execmem_restore_rox(addr, PAGE_SIZE);
>>>> +}
>>>> +
>>>>    static void *its_allocate_thunk(int reg)
>>>>    {
>>>>    	int size = 3 + (reg / 8);
>>>> @@ -234,6 +243,8 @@ static void *its_allocate_thunk(int reg)
>>>>    #endif
>>>>    	if (!its_page || (its_offset + size - 1) >= PAGE_SIZE) {
>>>> +		if (its_page)
>>>> +			its_set_kernel_ro(its_page);
>>>>    		its_page = its_alloc();
>>>>    		if (!its_page) {
>>>>    			pr_err("ITS page allocation failed\n");
>>>> @@ -2338,6 +2349,11 @@ void __init alternative_instructions(void)
>>>>    	apply_retpolines(__retpoline_sites, __retpoline_sites_end);
>>>>    	apply_returns(__return_sites, __return_sites_end);
>>>> +	/* Make potential last thunk page read-only. */
>>>> +	if (its_page)
>>>> +		its_set_kernel_ro(its_page);
>>>> +	its_page = NULL;
>>>> +
>>>>    	/*
>>>>    	 * Adjust all CALL instructions to point to func()-10, including
>>>>    	 * those in .altinstr_replacement.
>>>
>>> No, this is all sorts of wrong. Execmem API should ensure this.
>>
>> You are aware that this patch is basically mirroring the work which is
>> already done for modules in alternative.c?
> 
> I am having trouble parsing that -- where does alternative.c do this to
> modules?

Have a look at its_fini_mod().


Juergen

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3743 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

Re: [PATCH 3/3] x86/alternative: make kernel ITS thunks read-only

[Peter Zijlstra]

On Wed, May 28, 2025 at 03:30:33PM +0200, Jürgen Groß wrote:

> Have a look at its_fini_mod().

Oh, that's what you mean. But this still isn't very nice, you now have
restore_rox() without make_temp_rw(), which was the intended usage
pattern.

Bah, I hate how execmem works different for !PSE, Mike, you see a sane
way to fix this?

Anyway, if we have to do something like this, then I would prefer it
shaped something like so:

---
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index ecfe7b497cad..33d4d139cb50 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -111,9 +111,8 @@ static bool cfi_paranoid __ro_after_init;
 
 #ifdef CONFIG_MITIGATION_ITS
 
-#ifdef CONFIG_MODULES
 static struct module *its_mod;
-#endif
+static struct its_array its_pages;
 static void *its_page;
 static unsigned int its_offset;
 
@@ -151,68 +150,78 @@ static void *its_init_thunk(void *thunk, int reg)
 	return thunk + offset;
 }
 
-#ifdef CONFIG_MODULES
 void its_init_mod(struct module *mod)
 {
 	if (!cpu_feature_enabled(X86_FEATURE_INDIRECT_THUNK_ITS))
 		return;
 
-	mutex_lock(&text_mutex);
-	its_mod = mod;
-	its_page = NULL;
+	if (mod) {
+		mutex_lock(&text_mutex);
+		its_mod = mod;
+		its_page = NULL;
+	}
 }
 
 void its_fini_mod(struct module *mod)
 {
+	struct its_array *pages = &its_pages;
+
 	if (!cpu_feature_enabled(X86_FEATURE_INDIRECT_THUNK_ITS))
 		return;
 
 	WARN_ON_ONCE(its_mod != mod);
 
-	its_mod = NULL;
-	its_page = NULL;
-	mutex_unlock(&text_mutex);
+	if (mod) {
+		pages = &mod->arch.its_pages;
+		its_mod = NULL;
+		its_page = NULL;
+		mutex_unlock(&text_mutex);
+	}
 
-	for (int i = 0; i < mod->its_num_pages; i++) {
-		void *page = mod->its_page_array[i];
+	for (int i = 0; i < pages->num; i++) {
+		void *page = pages->pages[i];
 		execmem_restore_rox(page, PAGE_SIZE);
 	}
+
+	if (!mod)
+		kfree(pages->pages);
 }
 
 void its_free_mod(struct module *mod)
 {
+	struct its_array *pages = &its_pages;
+
 	if (!cpu_feature_enabled(X86_FEATURE_INDIRECT_THUNK_ITS))
 		return;
 
-	for (int i = 0; i < mod->its_num_pages; i++) {
-		void *page = mod->its_page_array[i];
+	if (mod)
+		pages = &mod->arch.its_pages;
+
+	for (int i = 0; i < pages->num; i++) {
+		void *page = pages->pages[i];
 		execmem_free(page);
 	}
-	kfree(mod->its_page_array);
+	kfree(pages->pages);
 }
-#endif /* CONFIG_MODULES */
 
 static void *its_alloc(void)
 {
-	void *page __free(execmem) = execmem_alloc(EXECMEM_MODULE_TEXT, PAGE_SIZE);
+	struct its_array *pages = &its_pages;
+	void *tmp;
 
+	void *page __free(execmem) = execmem_alloc(EXECMEM_MODULE_TEXT, PAGE_SIZE);
 	if (!page)
 		return NULL;
 
-#ifdef CONFIG_MODULES
-	if (its_mod) {
-		void *tmp = krealloc(its_mod->its_page_array,
-				     (its_mod->its_num_pages+1) * sizeof(void *),
-				     GFP_KERNEL);
-		if (!tmp)
-			return NULL;
+	tmp = krealloc(pages->pages, (pages->num + 1) * sizeof(void *), GFP_KERNEL);
+	if (!tmp)
+		return NULL;
 
-		its_mod->its_page_array = tmp;
-		its_mod->its_page_array[its_mod->its_num_pages++] = page;
+	pages->pages = tmp;
+	pages->pages[pages->num++] = page;
 
+	if (its_mod)
 		execmem_make_temp_rw(page, PAGE_SIZE);
-	}
-#endif /* CONFIG_MODULES */
 
 	return no_free_ptr(page);
 }
@@ -2338,6 +2347,8 @@ void __init alternative_instructions(void)
 	apply_retpolines(__retpoline_sites, __retpoline_sites_end);
 	apply_returns(__return_sites, __return_sites_end);
 
+	its_fini_mod(NULL);
+
 	/*
 	 * Adjust all CALL instructions to point to func()-10, including
 	 * those in .altinstr_replacement.

Re: [PATCH 3/3] x86/alternative: make kernel ITS thunks read-only

[Peter Zijlstra]

On Wed, May 28, 2025 at 05:58:21PM +0200, Peter Zijlstra wrote:
> On Wed, May 28, 2025 at 03:30:33PM +0200, Jürgen Groß wrote:
> 
> > Have a look at its_fini_mod().
> 
> Oh, that's what you mean. But this still isn't very nice, you now have
> restore_rox() without make_temp_rw(), which was the intended usage
> pattern.
> 
> Bah, I hate how execmem works different for !PSE, Mike, you see a sane
> way to fix this?
> 
> Anyway, if we have to do something like this, then I would prefer it
> shaped something like so:
> 

Missing file:

diff --git a/arch/x86/include/asm/module.h b/arch/x86/include/asm/module.h
index e988bac0a4a1..3c2de4ce3b10 100644
--- a/arch/x86/include/asm/module.h
+++ b/arch/x86/include/asm/module.h
@@ -5,12 +5,20 @@
 #include <asm-generic/module.h>
 #include <asm/orc_types.h>
 
+struct its_array {
+#ifdef CONFIG_MITIGATION_ITS
+	void **pages;
+	int num;
+#endif
+};
+
 struct mod_arch_specific {
 #ifdef CONFIG_UNWINDER_ORC
 	unsigned int num_orcs;
 	int *orc_unwind_ip;
 	struct orc_entry *orc_unwind;
 #endif
+	struct its_array its_pages;
 };
 
 #endif /* _ASM_X86_MODULE_H */

> ---
> diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
> index ecfe7b497cad..33d4d139cb50 100644
> --- a/arch/x86/kernel/alternative.c
> +++ b/arch/x86/kernel/alternative.c
> @@ -111,9 +111,8 @@ static bool cfi_paranoid __ro_after_init;
>  
>  #ifdef CONFIG_MITIGATION_ITS
>  
> -#ifdef CONFIG_MODULES
>  static struct module *its_mod;
> -#endif
> +static struct its_array its_pages;
>  static void *its_page;
>  static unsigned int its_offset;
>  
> @@ -151,68 +150,78 @@ static void *its_init_thunk(void *thunk, int reg)
>  	return thunk + offset;
>  }
>  
> -#ifdef CONFIG_MODULES
>  void its_init_mod(struct module *mod)
>  {
>  	if (!cpu_feature_enabled(X86_FEATURE_INDIRECT_THUNK_ITS))
>  		return;
>  
> -	mutex_lock(&text_mutex);
> -	its_mod = mod;
> -	its_page = NULL;
> +	if (mod) {
> +		mutex_lock(&text_mutex);
> +		its_mod = mod;
> +		its_page = NULL;
> +	}
>  }
>  
>  void its_fini_mod(struct module *mod)
>  {
> +	struct its_array *pages = &its_pages;
> +
>  	if (!cpu_feature_enabled(X86_FEATURE_INDIRECT_THUNK_ITS))
>  		return;
>  
>  	WARN_ON_ONCE(its_mod != mod);
>  
> -	its_mod = NULL;
> -	its_page = NULL;
> -	mutex_unlock(&text_mutex);
> +	if (mod) {
> +		pages = &mod->arch.its_pages;
> +		its_mod = NULL;
> +		its_page = NULL;
> +		mutex_unlock(&text_mutex);
> +	}
>  
> -	for (int i = 0; i < mod->its_num_pages; i++) {
> -		void *page = mod->its_page_array[i];
> +	for (int i = 0; i < pages->num; i++) {
> +		void *page = pages->pages[i];
>  		execmem_restore_rox(page, PAGE_SIZE);
>  	}
> +
> +	if (!mod)
> +		kfree(pages->pages);
>  }
>  
>  void its_free_mod(struct module *mod)
>  {
> +	struct its_array *pages = &its_pages;
> +
>  	if (!cpu_feature_enabled(X86_FEATURE_INDIRECT_THUNK_ITS))
>  		return;
>  
> -	for (int i = 0; i < mod->its_num_pages; i++) {
> -		void *page = mod->its_page_array[i];
> +	if (mod)
> +		pages = &mod->arch.its_pages;
> +
> +	for (int i = 0; i < pages->num; i++) {
> +		void *page = pages->pages[i];
>  		execmem_free(page);
>  	}
> -	kfree(mod->its_page_array);
> +	kfree(pages->pages);
>  }
> -#endif /* CONFIG_MODULES */
>  
>  static void *its_alloc(void)
>  {
> -	void *page __free(execmem) = execmem_alloc(EXECMEM_MODULE_TEXT, PAGE_SIZE);
> +	struct its_array *pages = &its_pages;
> +	void *tmp;
>  
> +	void *page __free(execmem) = execmem_alloc(EXECMEM_MODULE_TEXT, PAGE_SIZE);
>  	if (!page)
>  		return NULL;
>  
> -#ifdef CONFIG_MODULES
> -	if (its_mod) {
> -		void *tmp = krealloc(its_mod->its_page_array,
> -				     (its_mod->its_num_pages+1) * sizeof(void *),
> -				     GFP_KERNEL);
> -		if (!tmp)
> -			return NULL;
> +	tmp = krealloc(pages->pages, (pages->num + 1) * sizeof(void *), GFP_KERNEL);
> +	if (!tmp)
> +		return NULL;
>  
> -		its_mod->its_page_array = tmp;
> -		its_mod->its_page_array[its_mod->its_num_pages++] = page;
> +	pages->pages = tmp;
> +	pages->pages[pages->num++] = page;
>  
> +	if (its_mod)
>  		execmem_make_temp_rw(page, PAGE_SIZE);
> -	}
> -#endif /* CONFIG_MODULES */
>  
>  	return no_free_ptr(page);
>  }
> @@ -2338,6 +2347,8 @@ void __init alternative_instructions(void)
>  	apply_retpolines(__retpoline_sites, __retpoline_sites_end);
>  	apply_returns(__return_sites, __return_sites_end);
>  
> +	its_fini_mod(NULL);
> +
>  	/*
>  	 * Adjust all CALL instructions to point to func()-10, including
>  	 * those in .altinstr_replacement.

Re: [PATCH 3/3] x86/alternative: make kernel ITS thunks read-only

[Mike Rapoport]

On Wed, May 28, 2025 at 05:58:21PM +0200, Peter Zijlstra wrote:
> On Wed, May 28, 2025 at 03:30:33PM +0200, Jürgen Groß wrote:
> 
> > Have a look at its_fini_mod().
> 
> Oh, that's what you mean. But this still isn't very nice, you now have
> restore_rox() without make_temp_rw(), which was the intended usage
> pattern.
> 
> Bah, I hate how execmem works different for !PSE, Mike, you see a sane
> way to fix this?

Not really :(

But just resetting permissions in the end like you did makes perfect sense
to me. It's like STRICT_MODULE_RWX, somebody has to set the pages to ROX at
some point and running execmem_restore_rox() on something that was already
ROX won't cost much, set_memory will bail out early.

> Anyway, if we have to do something like this, then I would prefer it
> shaped something like so:
> 
> ---
> diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
> index ecfe7b497cad..33d4d139cb50 100644
> --- a/arch/x86/kernel/alternative.c
> +++ b/arch/x86/kernel/alternative.c
> @@ -111,9 +111,8 @@ static bool cfi_paranoid __ro_after_init;
>  
>  #ifdef CONFIG_MITIGATION_ITS
>  
> -#ifdef CONFIG_MODULES
>  static struct module *its_mod;
> -#endif
> +static struct its_array its_pages;
>  static void *its_page;
>  static unsigned int its_offset;
>  
> @@ -151,68 +150,78 @@ static void *its_init_thunk(void *thunk, int reg)
>  	return thunk + offset;
>  }
>  
> -#ifdef CONFIG_MODULES
>  void its_init_mod(struct module *mod)
>  {
>  	if (!cpu_feature_enabled(X86_FEATURE_INDIRECT_THUNK_ITS))
>  		return;
>  
> -	mutex_lock(&text_mutex);
> -	its_mod = mod;
> -	its_page = NULL;
> +	if (mod) {
> +		mutex_lock(&text_mutex);
> +		its_mod = mod;
> +		its_page = NULL;
> +	}
>  }
>  
>  void its_fini_mod(struct module *mod)
>  {
> +	struct its_array *pages = &its_pages;
> +
>  	if (!cpu_feature_enabled(X86_FEATURE_INDIRECT_THUNK_ITS))
>  		return;
>  
>  	WARN_ON_ONCE(its_mod != mod);
>  
> -	its_mod = NULL;
> -	its_page = NULL;
> -	mutex_unlock(&text_mutex);
> +	if (mod) {
> +		pages = &mod->arch.its_pages;
> +		its_mod = NULL;
> +		its_page = NULL;
> +		mutex_unlock(&text_mutex);
> +	}
>  
> -	for (int i = 0; i < mod->its_num_pages; i++) {
> -		void *page = mod->its_page_array[i];
> +	for (int i = 0; i < pages->num; i++) {
> +		void *page = pages->pages[i];
>  		execmem_restore_rox(page, PAGE_SIZE);
>  	}
> +
> +	if (!mod)
> +		kfree(pages->pages);
>  }
>  
>  void its_free_mod(struct module *mod)
>  {
> +	struct its_array *pages = &its_pages;
> +
>  	if (!cpu_feature_enabled(X86_FEATURE_INDIRECT_THUNK_ITS))
>  		return;
>  
> -	for (int i = 0; i < mod->its_num_pages; i++) {
> -		void *page = mod->its_page_array[i];
> +	if (mod)
> +		pages = &mod->arch.its_pages;
> +
> +	for (int i = 0; i < pages->num; i++) {
> +		void *page = pages->pages[i];
>  		execmem_free(page);
>  	}
> -	kfree(mod->its_page_array);
> +	kfree(pages->pages);
>  }
> -#endif /* CONFIG_MODULES */
>  
>  static void *its_alloc(void)
>  {
> -	void *page __free(execmem) = execmem_alloc(EXECMEM_MODULE_TEXT, PAGE_SIZE);
> +	struct its_array *pages = &its_pages;
> +	void *tmp;
>  
> +	void *page __free(execmem) = execmem_alloc(EXECMEM_MODULE_TEXT, PAGE_SIZE);
>  	if (!page)
>  		return NULL;
>  
> -#ifdef CONFIG_MODULES
> -	if (its_mod) {
> -		void *tmp = krealloc(its_mod->its_page_array,
> -				     (its_mod->its_num_pages+1) * sizeof(void *),
> -				     GFP_KERNEL);
> -		if (!tmp)
> -			return NULL;
> +	tmp = krealloc(pages->pages, (pages->num + 1) * sizeof(void *), GFP_KERNEL);
> +	if (!tmp)
> +		return NULL;
>  
> -		its_mod->its_page_array = tmp;
> -		its_mod->its_page_array[its_mod->its_num_pages++] = page;
> +	pages->pages = tmp;
> +	pages->pages[pages->num++] = page;
>  
> +	if (its_mod)
>  		execmem_make_temp_rw(page, PAGE_SIZE);
> -	}
> -#endif /* CONFIG_MODULES */
>  
>  	return no_free_ptr(page);
>  }
> @@ -2338,6 +2347,8 @@ void __init alternative_instructions(void)
>  	apply_retpolines(__retpoline_sites, __retpoline_sites_end);
>  	apply_returns(__return_sites, __return_sites_end);
>  
> +	its_fini_mod(NULL);
> +
>  	/*
>  	 * Adjust all CALL instructions to point to func()-10, including
>  	 * those in .altinstr_replacement.

-- 
Sincerely yours,
Mike.

Re: [PATCH 3/3] x86/alternative: make kernel ITS thunks read-only

[Mike Rapoport]

On Wed, May 28, 2025 at 05:58:21PM +0200, Peter Zijlstra wrote:
> On Wed, May 28, 2025 at 03:30:33PM +0200, Jürgen Groß wrote:
> 
> > Have a look at its_fini_mod().
> 
> Oh, that's what you mean. But this still isn't very nice, you now have
> restore_rox() without make_temp_rw(), which was the intended usage
> pattern.
> 
> Bah, I hate how execmem works different for !PSE, Mike, you see a sane
> way to fix this?

The least ugly thing I could think of is to replace the current pattern of

	execmem_alloc()
	exemem_make_temp_rw()
	/* update */
	execmem_restore_rox()

with

	execmem_alloc_rw()
	/* update */
	execmem_protect()

but I still haven't got to try it.

-- 
Sincerely yours,
Mike.

Re: [PATCH 3/3] x86/alternative: make kernel ITS thunks read-only

[Mike Rapoport]

On Wed, May 28, 2025 at 05:58:21PM +0200, Peter Zijlstra wrote:
> On Wed, May 28, 2025 at 03:30:33PM +0200, Jürgen Groß wrote:
> 
> > Have a look at its_fini_mod().
> 
> Oh, that's what you mean. But this still isn't very nice, you now have
> restore_rox() without make_temp_rw(), which was the intended usage
> pattern.
> 
> Bah, I hate how execmem works different for !PSE, Mike, you see a sane
> way to fix this?
> 
> Anyway, if we have to do something like this, then I would prefer it
> shaped something like so:

I expanded this a bit and here's what I've got:

https://lore.kernel.org/lkml/20250603111446.2609381-1-rppt@kernel.org/ 

> ---
> diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
> index ecfe7b497cad..33d4d139cb50 100644
> --- a/arch/x86/kernel/alternative.c
> +++ b/arch/x86/kernel/alternative.c
> @@ -111,9 +111,8 @@ static bool cfi_paranoid __ro_after_init;
>  
>  #ifdef CONFIG_MITIGATION_ITS
>  
> -#ifdef CONFIG_MODULES
>  static struct module *its_mod;
> -#endif
> +static struct its_array its_pages;
>  static void *its_page;
>  static unsigned int its_offset;
>  
> @@ -151,68 +150,78 @@ static void *its_init_thunk(void *thunk, int reg)
>  	return thunk + offset;
>  }
>  
> -#ifdef CONFIG_MODULES
>  void its_init_mod(struct module *mod)
>  {
>  	if (!cpu_feature_enabled(X86_FEATURE_INDIRECT_THUNK_ITS))
>  		return;
>  
> -	mutex_lock(&text_mutex);
> -	its_mod = mod;
> -	its_page = NULL;
> +	if (mod) {
> +		mutex_lock(&text_mutex);
> +		its_mod = mod;
> +		its_page = NULL;
> +	}
>  }
>  
>  void its_fini_mod(struct module *mod)
>  {
> +	struct its_array *pages = &its_pages;
> +
>  	if (!cpu_feature_enabled(X86_FEATURE_INDIRECT_THUNK_ITS))
>  		return;
>  
>  	WARN_ON_ONCE(its_mod != mod);
>  
> -	its_mod = NULL;
> -	its_page = NULL;
> -	mutex_unlock(&text_mutex);
> +	if (mod) {
> +		pages = &mod->arch.its_pages;
> +		its_mod = NULL;
> +		its_page = NULL;
> +		mutex_unlock(&text_mutex);
> +	}
>  
> -	for (int i = 0; i < mod->its_num_pages; i++) {
> -		void *page = mod->its_page_array[i];
> +	for (int i = 0; i < pages->num; i++) {
> +		void *page = pages->pages[i];
>  		execmem_restore_rox(page, PAGE_SIZE);
>  	}
> +
> +	if (!mod)
> +		kfree(pages->pages);
>  }
>  
>  void its_free_mod(struct module *mod)
>  {
> +	struct its_array *pages = &its_pages;
> +
>  	if (!cpu_feature_enabled(X86_FEATURE_INDIRECT_THUNK_ITS))
>  		return;
>  
> -	for (int i = 0; i < mod->its_num_pages; i++) {
> -		void *page = mod->its_page_array[i];
> +	if (mod)
> +		pages = &mod->arch.its_pages;
> +
> +	for (int i = 0; i < pages->num; i++) {
> +		void *page = pages->pages[i];
>  		execmem_free(page);
>  	}
> -	kfree(mod->its_page_array);
> +	kfree(pages->pages);
>  }
> -#endif /* CONFIG_MODULES */
>  
>  static void *its_alloc(void)
>  {
> -	void *page __free(execmem) = execmem_alloc(EXECMEM_MODULE_TEXT, PAGE_SIZE);
> +	struct its_array *pages = &its_pages;
> +	void *tmp;
>  
> +	void *page __free(execmem) = execmem_alloc(EXECMEM_MODULE_TEXT, PAGE_SIZE);
>  	if (!page)
>  		return NULL;
>  
> -#ifdef CONFIG_MODULES
> -	if (its_mod) {
> -		void *tmp = krealloc(its_mod->its_page_array,
> -				     (its_mod->its_num_pages+1) * sizeof(void *),
> -				     GFP_KERNEL);
> -		if (!tmp)
> -			return NULL;
> +	tmp = krealloc(pages->pages, (pages->num + 1) * sizeof(void *), GFP_KERNEL);
> +	if (!tmp)
> +		return NULL;
>  
> -		its_mod->its_page_array = tmp;
> -		its_mod->its_page_array[its_mod->its_num_pages++] = page;
> +	pages->pages = tmp;
> +	pages->pages[pages->num++] = page;
>  
> +	if (its_mod)
>  		execmem_make_temp_rw(page, PAGE_SIZE);
> -	}
> -#endif /* CONFIG_MODULES */
>  
>  	return no_free_ptr(page);
>  }
> @@ -2338,6 +2347,8 @@ void __init alternative_instructions(void)
>  	apply_retpolines(__retpoline_sites, __retpoline_sites_end);
>  	apply_returns(__return_sites, __return_sites_end);
>  
> +	its_fini_mod(NULL);
> +
>  	/*
>  	 * Adjust all CALL instructions to point to func()-10, including
>  	 * those in .altinstr_replacement.

-- 
Sincerely yours,
Mike.

Re: [PATCH 3/3] x86/alternative: make kernel ITS thunks read-only

[kernel test robot]

Hi Juergen,

kernel test robot noticed the following build errors:

[auto build test ERROR on tip/master]
[also build test ERROR on tip/x86/core linus/master v6.15 next-20250528]
[cannot apply to tip/x86/mm tip/auto-latest]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Juergen-Gross/x86-execmem-don-t-use-PAGE_KERNEL-protection-for-code-pages/20250528-204423
base:   tip/master
patch link:    https://lore.kernel.org/r/20250528123557.12847-4-jgross%40suse.com
patch subject: [PATCH 3/3] x86/alternative: make kernel ITS thunks read-only
config: x86_64-buildonly-randconfig-002-20250529 (https://download.01.org/0day-ci/archive/20250529/202505291124.eAZ7fgbG-lkp@intel.com/config)
compiler: clang version 20.1.2 (https://github.com/llvm/llvm-project 58df0ef89dd64126512e4ee27b4ac3fd8ddf6247)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250529/202505291124.eAZ7fgbG-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202505291124.eAZ7fgbG-lkp@intel.com/

All errors (new ones prefixed by >>):

>> arch/x86/kernel/alternative.c:2353:6: error: use of undeclared identifier 'its_page'
    2353 |         if (its_page)
         |             ^
>> arch/x86/kernel/alternative.c:2354:3: error: call to undeclared function 'its_set_kernel_ro'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
    2354 |                 its_set_kernel_ro(its_page);
         |                 ^
   arch/x86/kernel/alternative.c:2354:21: error: use of undeclared identifier 'its_page'
    2354 |                 its_set_kernel_ro(its_page);
         |                                   ^
   arch/x86/kernel/alternative.c:2355:2: error: use of undeclared identifier 'its_page'
    2355 |         its_page = NULL;
         |         ^
   4 errors generated.


vim +/its_page +2353 arch/x86/kernel/alternative.c

  2308	
  2309	void __init alternative_instructions(void)
  2310	{
  2311		u64 ibt;
  2312	
  2313		int3_selftest();
  2314	
  2315		/*
  2316		 * The patching is not fully atomic, so try to avoid local
  2317		 * interruptions that might execute the to be patched code.
  2318		 * Other CPUs are not running.
  2319		 */
  2320		stop_nmi();
  2321	
  2322		/*
  2323		 * Don't stop machine check exceptions while patching.
  2324		 * MCEs only happen when something got corrupted and in this
  2325		 * case we must do something about the corruption.
  2326		 * Ignoring it is worse than an unlikely patching race.
  2327		 * Also machine checks tend to be broadcast and if one CPU
  2328		 * goes into machine check the others follow quickly, so we don't
  2329		 * expect a machine check to cause undue problems during to code
  2330		 * patching.
  2331		 */
  2332	
  2333		/*
  2334		 * Make sure to set (artificial) features depending on used paravirt
  2335		 * functions which can later influence alternative patching.
  2336		 */
  2337		paravirt_set_cap();
  2338	
  2339		/* Keep CET-IBT disabled until caller/callee are patched */
  2340		ibt = ibt_save(/*disable*/ true);
  2341	
  2342		__apply_fineibt(__retpoline_sites, __retpoline_sites_end,
  2343				__cfi_sites, __cfi_sites_end, true);
  2344	
  2345		/*
  2346		 * Rewrite the retpolines, must be done before alternatives since
  2347		 * those can rewrite the retpoline thunks.
  2348		 */
  2349		apply_retpolines(__retpoline_sites, __retpoline_sites_end);
  2350		apply_returns(__return_sites, __return_sites_end);
  2351	
  2352		/* Make potential last thunk page read-only. */
> 2353		if (its_page)
> 2354			its_set_kernel_ro(its_page);
  2355		its_page = NULL;
  2356	
  2357		/*
  2358		 * Adjust all CALL instructions to point to func()-10, including
  2359		 * those in .altinstr_replacement.
  2360		 */
  2361		callthunks_patch_builtin_calls();
  2362	
  2363		apply_alternatives(__alt_instructions, __alt_instructions_end);
  2364	
  2365		/*
  2366		 * Seal all functions that do not have their address taken.
  2367		 */
  2368		apply_seal_endbr(__ibt_endbr_seal, __ibt_endbr_seal_end);
  2369	
  2370		ibt_restore(ibt);
  2371	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki