linux-kernel - Re: [PATCH 2/2] x86/boot: Add setup_indirect support in early_memremap_is_setup

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 for Android: free password hash cracker in your pocket

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

Date:   Tue, 15 Feb 2022 06:36:49 -0500
From:   Ross Philipson <ross.philipson@...cle.com>
To:     Borislav Petkov <bp@...en8.de>
Cc:     linux-kernel@...r.kernel.org, x86@...nel.org,
        daniel.kiper@...cle.com, dpsmith@...rtussolutions.com,
        tglx@...utronix.de, mingo@...hat.com, hpa@...or.com,
        luto@...capital.net, kanth.ghatraju@...cle.com,
        trenchboot-devel@...glegroups.com
Subject: Re: [PATCH 2/2] x86/boot: Add setup_indirect support in
 early_memremap_is_setup_data

On 2/11/22 13:41, Borislav Petkov wrote:
> On Thu, Jan 27, 2022 at 12:04:16PM -0500, Ross Philipson wrote:
>> The x86 boot documentation describes the setup_indirect structures and
>> how they are used. Only one of the two functions in ioremap.c that needed
>> to be modified to be aware of the introduction of setup_indirect
>> functionality was updated. This adds comparable support to the other
> 
> s/This adds/Add/

Ack.

> 
>> function where it was missing.
>>
>> Fixes: b3c72fc9a78e ("x86/boot: Introduce setup_indirect")
>>
> 
> No need for a newline here.
> 
>> Signed-off-by: Ross Philipson <ross.philipson@...cle.com>
>> Reviewed-by: Daniel Kiper <daniel.kiper@...cle.com>
>> ---
>>  arch/x86/mm/ioremap.c | 21 +++++++++++++++++++--
>>  1 file changed, 19 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
>> index b45e86e..9129b29 100644
>> --- a/arch/x86/mm/ioremap.c
>> +++ b/arch/x86/mm/ioremap.c
>> @@ -670,17 +670,34 @@ static bool __init early_memremap_is_setup_data(resource_size_t phys_addr,
>>  
>>  	paddr = boot_params.hdr.setup_data;
>>  	while (paddr) {
>> -		unsigned int len;
>> +		unsigned int len, size;
>>  
>>  		if (phys_addr == paddr)
>>  			return true;
>>  
>>  		data = early_memremap_decrypted(paddr, sizeof(*data));
>> +		size = sizeof(*data);
>>  
>>  		paddr_next = data->next;
>>  		len = data->len;
>>  
>> -		early_memunmap(data, sizeof(*data));
>> +		if ((phys_addr > paddr) && (phys_addr < (paddr + len))) {
>> +			early_memunmap(data, sizeof(*data));
>> +			return true;
>> +		}
>> +
>> +		if (data->type == SETUP_INDIRECT) {
>> +			size += len;
>> +			early_memunmap(data, sizeof(*data));
>> +			data = early_memremap_decrypted(paddr, size);
> 
> That can return NULL.

Although there is a bit more indirection here, it is the same reply with
questions in the first patch.

> 
>> +
>> +			if (((struct setup_indirect *)data->data)->type != SETUP_INDIRECT) {
>> +				paddr = ((struct setup_indirect *)data->data)->addr;
>> +				len = ((struct setup_indirect *)data->data)->len;
> 
> As before, use a helper variable here pls.
> 
> Thx.
> 

Thanks,

Ross Philipson