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

["Jürgen Groß" <jgross@suse.com>]

[-- 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 --]