[PATCH] arm64: zynqmp: Fix set_fdtfile() not to break u-boots DTB

[Michal Simek]

On 24. 06. 20 14:44, Michal Simek wrote:
> Origin function was calling strsep which replaced delimiter ',' by a null
> byte ('\0'). Operation was done directly on FDT which ends up with the
> following behavior:
> 
> ZynqMP>  printenv fdtfile
> fdtfile=xilinx/zynqmp.dtb
> ZynqMP> fdt addr $fdtcontroladdr
> ZynqMP> fdt print / compatible
> compatible = "xlnx", "zynqmp"
> 
> As is visible fdtfile was correctly composed but a null byte caused that
> xlnx was separated from zynqmp.
> This hasn't been spotted because in all Xilinx DTs there are at least 3
> compatible string and only the first one was affected by this issue.
> But for systems which only had one compatible string "xlnx,zynqmp" it was
> causing an issue when U-Boot's DT was used by Linux kernel.
> 
> The patch removes strsep calling and strchr is called instead which just
> locate the first char after deliminator ',' (variable called "name").
> And using this pointer in fdtfile composing.
> 
> Fixes: 91d7e0c47f51 ("arm64: zynqmp: Create fdtfile from compatible string")
> Signed-off-by: Michal Simek <michal.simek at xilinx.com>
> ---
> 
>  board/xilinx/zynqmp/zynqmp.c | 19 +++++++++++++------
>  1 file changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
> index ebb71729081d..8a4df6fc1ab6 100644
> --- a/board/xilinx/zynqmp/zynqmp.c
> +++ b/board/xilinx/zynqmp/zynqmp.c
> @@ -541,23 +541,30 @@ static int set_fdtfile(void)
>  	char *compatible, *fdtfile;
>  	const char *suffix = ".dtb";
>  	const char *vendor = "xilinx/";
> +	int fdt_compat_len;
>  
>  	if (env_get("fdtfile"))
>  		return 0;
>  
> -	compatible = (char *)fdt_getprop(gd->fdt_blob, 0, "compatible", NULL);
> -	if (compatible) {
> +	compatible = (char *)fdt_getprop(gd->fdt_blob, 0, "compatible",
> +					 &fdt_compat_len);
> +	if (compatible && fdt_compat_len) {
> +		char *name;
> +
>  		debug("Compatible: %s\n", compatible);
>  
> -		/* Discard vendor prefix */
> -		strsep(&compatible, ",");
> +		name = strchr(compatible, ',');
> +		if (!name)
> +			return -EINVAL;
> +
> +		name++;
>  
> -		fdtfile = calloc(1, strlen(vendor) + strlen(compatible) +
> +		fdtfile = calloc(1, strlen(vendor) + strlen(name) +
>  				 strlen(suffix) + 1);
>  		if (!fdtfile)
>  			return -ENOMEM;
>  
> -		sprintf(fdtfile, "%s%s%s", vendor, compatible, suffix);
> +		sprintf(fdtfile, "%s%s%s", vendor, name, suffix);
>  
>  		env_set("fdtfile", fdtfile);
>  		free(fdtfile);
> 


Also:
Reported-by: Igor Lantsman <igor.lantsman at opsys-tech.com>
Signed-off-by: Igor Lantsman <igor.lantsman at opsys-tech.com>

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: OpenPGP digital signature
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200624/b9e7ba99/attachment.sig>