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

[Peter Zijlstra]

On Tue, Jun 03, 2025 at 02:14:44PM +0300, Mike Rapoport wrote:
> From: "Peter Zijlstra (Intel)" <peterz@xxxxxxxxxxxxx>
> 
> 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@xxxxxxxxxxxxx>
> Co-developed-by: Mike Rapoport (Microsoft) <rppt@xxxxxxxxxx>
> Signed-off-by: Mike Rapoport (Microsoft) <rppt@xxxxxxxxxx>
> ---

How about something like this on top?

--- 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;
+
 	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);
 }
 
 void its_free_mod(struct module *mod)
@@ -222,40 +222,29 @@ void its_free_mod(struct module *mod)
 	}
 	kfree(mod->arch.its_pages.pages);
 }
+#endif /* CONFIG_MODULES */
 
-static void *its_alloc_mod(void)
+static void *its_alloc(void)
 {
-	void *page = __its_alloc(&its_mod->arch.its_pages);
-
-	if (page)
-		execmem_make_temp_rw(page, PAGE_SIZE);
+	struct its_array *pages = &its_pages;
+	void *page;
 
-	return page;
-}
-#endif /* CONFIG_MODULES */
+#ifdef CONFIG_MODULE
+	if (its_mod)
+		pages = &its_mod->arch.its_pages;
+#endif
 
-static void *its_alloc_core(void)
-{
-	void *page = __its_alloc(&its_pages);
+	page = __its_alloc(pages);
+	if (!page)
+		return NULL;
 
-	if (page) {
-		execmem_make_temp_rw(page, PAGE_SIZE);
+	execmem_make_temp_rw(page, PAGE_SIZE);
+	if (pages == &its_pages)
 		set_memory_x((unsigned long)page, 1);
-	}
 
 	return page;
 }
 
-static void *its_alloc(void)
-{
-#ifdef CONFIG_MODULES
-	if (its_mod)
-		return its_alloc_mod();
-#endif /* CONFIG_MODULES */
-
-	return its_alloc_core();
-}
-
 static void *its_allocate_thunk(int reg)
 {
 	int size = 3 + (reg / 8);