Repair image-fit: switch ENOLINK to ENOENT
PATCH 1/1 Repair image-fit: switch ENOLINK to ENOENT
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.
--- common/image-fdt.c | 2 +- common/image.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
common/image-fdt.c +1 -1
@@ -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 ;
common/image.c +1 -1
@@ -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 ;
Signature · 1 line
2.9.3
PATCH 1/1 Repair image-fit: switch ENOLINK to ENOENT
Quoted 9 lines · Marek Vasut
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.
Signed-off-by Marek Vasut <[email protected]>Cc Jonathan Gray <[email protected]>Cc Paul Burton <[email protected]>Cc Tom Rini <[email protected]>
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160918/d516df88/attachment.sig>
PATCH 1/1 Repair image-fit: switch ENOLINK to ENOENT
In reply to Marek VasutHi Marek,
Quoted 4 lines · Marek Vasut
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.
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.
Thanks,
Paul
Quoted 37 lines
Signed-off-by Marek Vasut <[email protected]>Cc Jonathan Gray <[email protected]>Cc Paul Burton <[email protected]>Cc Tom Rini <[email protected]>--- common/image-fdt.c | 2 +- common/image.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)common/image-fdt.c +1
-1@@ -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 ;common/image.c +1
-1@@ -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 ;
PATCH 1/1 Repair image-fit: switch ENOLINK to ENOENT
Quoted 1 line · Paul Burton
Hi Marek,
Hi,
Quoted 12 lines · Marek Vasut
Quoted 4 lines
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...
Quoted 7 lines
The (potential?) problem with the approach you took in this patch is that the return value from
fit_get_node_from_configno longer differentiates between the ramdisk property of a FIT config being missing (-ENOLINKprior tobac17b78dace) & 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.
Quoted 6 lines
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, offit_get_node_from_configto see whether they rely upon-ENOENTfor the config not found case.
Can you roll a patch for that ?
Thanks!
Quoted 40 lines
Thanks,
PaulQuoted 37 lines
Signed-off-by Marek Vasut <[email protected]>Cc Jonathan Gray <[email protected]>Cc Paul Burton <[email protected]>Cc Tom Rini <[email protected]>--- common/image-fdt.c | 2 +- common/image.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)common/image-fdt.c +1
-1@@ -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 ;common/image.c +1
-1@@ -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 ;
Signature · 2 lines
Best regards, Marek Vasut