[U-Boot] [PATCH] fdt: fix fdtdec_get_addr_size not to require any size cells
From: Stephen Warren fdtdec_get_addr_size() may be used in two cases: a) With sizep supplied, in which case both an address and a size are parsed from DT. In this case, the DT property must be large enough to contain both values. b) With sizep NULL, in which case only an address is parsed from DT. In this case, the DT property only need be large enough to contain this address value. Commit 02464e386bb5 "fdt: add new fdt address parsing functions" broke this relaxed checking, and required the DT property to contain both an address and a size value in all cases. Fix fdtdec_get_addr_size() to vary ns based on whether the size value is being parsed from the DT or not. This is safe since the function only parses the first entry in the property, so the overall value of (na + ns) need not be accurate, since it is never used to step through the property data to find other entries. Besides, this fixed behaviour essentially matches the original behaviour before the patch this patch fixes. (The original code validated that the property was exactly the length of either na or (na + ns), whereas the current code only validates that the property is at least that long. For non-failure cases, the two behaviours are identical). Cc: Przemyslaw Marczak Cc: Simon Glass Cc: Thierry Reding Cc: Bin Meng Cc: Michal Suchanek Fixes: 02464e386bb5 ("fdt: add new fdt address parsing functions") Reported-by: Przemyslaw Marczak Signed-off-by: Stephen Warren --- lib/fdtdec.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/fdtdec.c b/lib/fdtdec.c index 9f0b65de3831..1fdb4f0d9ce9 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -180,10 +180,11 @@ fdt_addr_t fdtdec_get_addr_size_auto_noparent(const void *blob, int node, fdt_addr_t fdtdec_get_addr_size(const void *blob, int node, const char *prop_name, fdt_size_t *sizep) { + int ns = sizep ? (sizeof(fdt_size_t) / sizeof(fdt32_t)) : 0; + return fdtdec_get_addr_size_fixed(blob, node, prop_name, 0, sizeof(fdt_addr_t) / sizeof(fdt32_t), - sizeof(fdt_size_t) / sizeof(fdt32_t), - sizep); + ns, sizep); } fdt_addr_t fdtdec_get_addr(const void *blob, int node, -- 1.9.1 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] fdt: fix fdtdec_get_addr_size not to require any size cells
On 29 September 2015 at 05:33, Simon Glass wrote: > On 28 September 2015 at 04:41, Przemyslaw Marczak > wrote: >> Hello Stephen, >> >> >> On 09/25/2015 06:11 PM, Stephen Warren wrote: >>> >>> From: Stephen Warren >>> >>> fdtdec_get_addr_size() may be used in two cases: >>> a) With sizep supplied, in which case both an address and a size are >>> parsed from DT. In this case, the DT property must be large enough to >>> contain both values. >>> b) With sizep NULL, in which case only an address is parsed from DT. >>> In this case, the DT property only need be large enough to contain this >>> address value. Commit 02464e386bb5 "fdt: add new fdt address parsing >>> functions" broke this relaxed checking, and required the DT property to >>> contain both an address and a size value in all cases. >>> >>> Fix fdtdec_get_addr_size() to vary ns based on whether the size value >>> is being parsed from the DT or not. This is safe since the function only >>> parses the first entry in the property, so the overall value of (na + ns) >>> need not be accurate, since it is never used to step through the property >>> data to find other entries. Besides, this fixed behaviour essentially >>> matches the original behaviour before the patch this patch fixes. (The >>> original code validated that the property was exactly the length of >>> either na or (na + ns), whereas the current code only validates that the >>> property is at least that long. For non-failure cases, the two behaviours >>> are identical). >>> >>> Cc: Przemyslaw Marczak >>> Cc: Simon Glass >>> Cc: Thierry Reding >>> Cc: Bin Meng >>> Cc: Michal Suchanek >>> Fixes: 02464e386bb5 ("fdt: add new fdt address parsing functions") >>> Reported-by: Przemyslaw Marczak >>> Signed-off-by: Stephen Warren >>> --- >>> lib/fdtdec.c | 5 +++-- >>> 1 file changed, 3 insertions(+), 2 deletions(-) > > Acked-by: Simon Glass Applied to u-boot-fdt, thanks! ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] fdt: fix fdtdec_get_addr_size not to require any size cells
Hello Stephen, On 09/25/2015 06:11 PM, Stephen Warren wrote: From: Stephen Warren fdtdec_get_addr_size() may be used in two cases: a) With sizep supplied, in which case both an address and a size are parsed from DT. In this case, the DT property must be large enough to contain both values. b) With sizep NULL, in which case only an address is parsed from DT. In this case, the DT property only need be large enough to contain this address value. Commit 02464e386bb5 "fdt: add new fdt address parsing functions" broke this relaxed checking, and required the DT property to contain both an address and a size value in all cases. Fix fdtdec_get_addr_size() to vary ns based on whether the size value is being parsed from the DT or not. This is safe since the function only parses the first entry in the property, so the overall value of (na + ns) need not be accurate, since it is never used to step through the property data to find other entries. Besides, this fixed behaviour essentially matches the original behaviour before the patch this patch fixes. (The original code validated that the property was exactly the length of either na or (na + ns), whereas the current code only validates that the property is at least that long. For non-failure cases, the two behaviours are identical). Cc: Przemyslaw Marczak Cc: Simon Glass Cc: Thierry Reding Cc: Bin Meng Cc: Michal Suchanek Fixes: 02464e386bb5 ("fdt: add new fdt address parsing functions") Reported-by: Przemyslaw Marczak Signed-off-by: Stephen Warren --- lib/fdtdec.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/fdtdec.c b/lib/fdtdec.c index 9f0b65de3831..1fdb4f0d9ce9 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -180,10 +180,11 @@ fdt_addr_t fdtdec_get_addr_size_auto_noparent(const void *blob, int node, fdt_addr_t fdtdec_get_addr_size(const void *blob, int node, const char *prop_name, fdt_size_t *sizep) { + int ns = sizep ? (sizeof(fdt_size_t) / sizeof(fdt32_t)) : 0; + return fdtdec_get_addr_size_fixed(blob, node, prop_name, 0, sizeof(fdt_addr_t) / sizeof(fdt32_t), - sizeof(fdt_size_t) / sizeof(fdt32_t), - sizep); + ns, sizep); } fdt_addr_t fdtdec_get_addr(const void *blob, int node, I tested this on top of latest mainline and it fixes the issue for use of fdtdec_get_addr(), however my fixes should be also applied. Tested-by: Przemyslaw Marczak Best regards, -- Przemyslaw Marczak Samsung R&D Institute Poland Samsung Electronics p.marc...@samsung.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] fdt: fix fdtdec_get_addr_size not to require any size cells
On 28 September 2015 at 04:41, Przemyslaw Marczak wrote: > Hello Stephen, > > > On 09/25/2015 06:11 PM, Stephen Warren wrote: >> >> From: Stephen Warren >> >> fdtdec_get_addr_size() may be used in two cases: >> a) With sizep supplied, in which case both an address and a size are >> parsed from DT. In this case, the DT property must be large enough to >> contain both values. >> b) With sizep NULL, in which case only an address is parsed from DT. >> In this case, the DT property only need be large enough to contain this >> address value. Commit 02464e386bb5 "fdt: add new fdt address parsing >> functions" broke this relaxed checking, and required the DT property to >> contain both an address and a size value in all cases. >> >> Fix fdtdec_get_addr_size() to vary ns based on whether the size value >> is being parsed from the DT or not. This is safe since the function only >> parses the first entry in the property, so the overall value of (na + ns) >> need not be accurate, since it is never used to step through the property >> data to find other entries. Besides, this fixed behaviour essentially >> matches the original behaviour before the patch this patch fixes. (The >> original code validated that the property was exactly the length of >> either na or (na + ns), whereas the current code only validates that the >> property is at least that long. For non-failure cases, the two behaviours >> are identical). >> >> Cc: Przemyslaw Marczak >> Cc: Simon Glass >> Cc: Thierry Reding >> Cc: Bin Meng >> Cc: Michal Suchanek >> Fixes: 02464e386bb5 ("fdt: add new fdt address parsing functions") >> Reported-by: Przemyslaw Marczak >> Signed-off-by: Stephen Warren >> --- >> lib/fdtdec.c | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) Acked-by: Simon Glass ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot