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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Fri, 11 Feb 2022 19:41:20 +0100
From:   Borislav Petkov <bp@...en8.de>
To:     Ross Philipson <ross.philipson@...cle.com>
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 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/

> 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.

> +
> +			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.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette