[U-Boot] [PATCH] Repair image-fit: switch ENOLINK to ENOENT

[Marek Vasut]

On 09/19/2016 10:29 AM, Paul Burton wrote:
> Hi Marek,

Hi,

> On 18/09/16 14:27, Marek Vasut wrote:
>> This patch broke booting of any fitImage-wrapped kernel images due
>> to replacement of ENOLINK with ENOENT without checking where the
>> ENOLINK return value is being tested for. Adjust the tests as well
>> to repair the breakage.
> 
> It's not obvious from the commit message what "this patch" refers to - I
> think it would be useful to include the hash & subject of the broken
> commit, eg:
> 
>   Commit bac17b78dace ("image-fit: switch ENOLINK to ENOENT") broke...
> 
> The (potential?) problem with the approach you took in this patch is
> that the return value from fit_get_node_from_config no longer
> differentiates between the ramdisk property of a FIT config being
> missing (-ENOLINK prior to bac17b78dace) & the configuration itself
> being missing (-ENOENT). Could that possibly lead to accepting invalid
> FIT images? If not then I imagine it's by some convoluted chance & that
> we'd probably be best to not rely on it.

Crap, that's a very valid point.

> If using ENOLINK isn't an option then my suggestion would be that we
> change the config not found case to return -EINVAL & the property not
> found can keep returning -ENOENT. Semantically that would seem to make
> sense but it would mean checking all the callers, direct or indirect, of
> fit_get_node_from_config to see whether they rely upon -ENOENT for the
> config not found case.

Can you roll a patch for that ?

Thanks!

> Thanks,
>     Paul
> 
>>
>> Signed-off-by: Marek Vasut <marex at denx.de>
>> Cc: Jonathan Gray <jsg at jsg.id.au>
>> Cc: Paul Burton <paul.burton at imgtec.com>
>> Cc: Tom Rini <trini at konsulko.com>
>> ---
>>  common/image-fdt.c | 2 +-
>>  common/image.c     | 2 +-
>>  2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/common/image-fdt.c b/common/image-fdt.c
>> index d6ee225..3d23608 100644
>> --- a/common/image-fdt.c
>> +++ b/common/image-fdt.c
>> @@ -285,7 +285,7 @@ int boot_get_fdt(int flag, int argc, char * const argv[], uint8_t arch,
>>  			fdt_noffset = fit_get_node_from_config(images,
>>  							       FIT_FDT_PROP,
>>  							       fdt_addr);
>> -			if (fdt_noffset == -ENOLINK)
>> +			if (fdt_noffset == -ENOENT)
>>  				return 0;
>>  			else if (fdt_noffset < 0)
>>  				return 1;
>> diff --git a/common/image.c b/common/image.c
>> index 7ad04ca..c8d9bc8 100644
>> --- a/common/image.c
>> +++ b/common/image.c
>> @@ -1078,7 +1078,7 @@ int boot_get_ramdisk(int argc, char * const argv[], bootm_headers_t *images,
>>  			rd_addr = map_to_sysmem(images->fit_hdr_os);
>>  			rd_noffset = fit_get_node_from_config(images,
>>  					FIT_RAMDISK_PROP, rd_addr);
>> -			if (rd_noffset == -ENOLINK)
>> +			if (rd_noffset == -ENOENT)
>>  				return 0;
>>  			else if (rd_noffset < 0)
>>  				return 1;
>>


-- 
Best regards,
Marek Vasut