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@xxxxxxxxxx>
> Signed-off-by: Max Zhen <max.zhen@xxxxxxxxxx>
> Signed-off-by: Lizhi Hou <lizhi.hou@xxxxxxxxxx>
> ---
> 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
