linux-kernel - Re: [PATCH V2 XRT Alveo Infrastructure 3/9] of: handle fdt buffer alignment inside unflatten function

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]

Message-ID: <YaWE/2ikgpXi2hzY@robh.at.kernel.org>
Date:   Mon, 29 Nov 2021 19:57:19 -0600
From:   Rob Herring <robh@...nel.org>
To:     Lizhi Hou <lizhi.hou@...inx.com>
Cc:     linux-kernel@...r.kernel.org, linux-fpga@...r.kernel.org,
        maxz@...inx.com, sonal.santan@...inx.com, yliu@...inx.com,
        michal.simek@...inx.com, stefanos@...inx.com,
        devicetree@...r.kernel.org, trix@...hat.com, mdf@...nel.org,
        dwmw2@...radead.org, Max Zhen <max.zhen@...inx.com>
Subject: Re: [PATCH V2 XRT Alveo Infrastructure 3/9] of: handle fdt buffer
 alignment inside unflatten function

On Fri, Nov 19, 2021 at 02:24:06PM -0800, Lizhi Hou wrote:
> Add alignment check to of_fdt_unflatten_tree(). If it is not aligned,
> allocate a aligned buffer and copy the fdt blob. So the caller does not
> have to deal with the buffer alignment before calling this function.
> XRT uses this function to unflatten fdt which is from Alveo firmware.
> 
> Signed-off-by: Sonal Santan <sonal.santan@...inx.com>
> Signed-off-by: Max Zhen <max.zhen@...inx.com>
> Signed-off-by: Lizhi Hou <lizhi.hou@...inx.com>
> ---
>  drivers/of/fdt.c | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index 4546572af24b..d64445e43ceb 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -455,13 +455,28 @@ void *of_fdt_unflatten_tree(const unsigned long *blob,
>  			    struct device_node *dad,
>  			    struct device_node **mynodes)
>  {
> +	void *new_fdt = NULL, *fdt_align;
>  	void *mem;
>  
> +	if (fdt_check_header(blob)) {
> +		pr_err("Invalid fdt blob\n");
> +		return NULL;
> +	}
> +	fdt_align = (void *)PTR_ALIGN(blob, FDT_ALIGN_SIZE);
> +	if (fdt_align != blob) {
> +		new_fdt = kmalloc(fdt_totalsize(blob) + FDT_ALIGN_SIZE, GFP_KERNEL);
> +		if (!new_fdt)
> +			return NULL;
> +		fdt_align = PTR_ALIGN(new_fdt, FDT_ALIGN_SIZE);

Where's the copy?

> +	}
> +
>  	mutex_lock(&of_fdt_unflatten_mutex);
> -	mem = __unflatten_device_tree(blob, dad, mynodes, &kernel_tree_alloc,
> +	mem = __unflatten_device_tree(fdt_align, dad, mynodes, &kernel_tree_alloc,
>  				      true);
>  	mutex_unlock(&of_fdt_unflatten_mutex);
>  
> +	kfree(new_fdt);

You know the unflattened DT just references strings and property values 
from the flattened DT. So you just caused a use after free.

Fix your firmware to align the DT.

Rob