Message ID | 1443197501-17489-1-git-send-email-swarren@wwwdotorg.org |
---|---|
State | Accepted |
Delegated to: | Simon Glass |
Headers | show |
Hello Stephen, On 09/25/2015 06:11 PM, Stephen Warren wrote: > From: Stephen Warren <swarren@nvidia.com> > > 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 <p.marczak@samsung.com> > Cc: Simon Glass <sjg@chromium.org> > Cc: Thierry Reding <treding@nvidia.com> > Cc: Bin Meng <bmeng.cn@gmail.com> > Cc: Michal Suchanek <hramrach@gmail.com> > Fixes: 02464e386bb5 ("fdt: add new fdt address parsing functions") > Reported-by: Przemyslaw Marczak <p.marczak@samsung.com> > Signed-off-by: Stephen Warren <swarren@nvidia.com> > --- > 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 <p.marczak@samsung.com> Best regards,
On 28 September 2015 at 04:41, Przemyslaw Marczak <p.marczak@samsung.com> wrote: > Hello Stephen, > > > On 09/25/2015 06:11 PM, Stephen Warren wrote: >> >> From: Stephen Warren <swarren@nvidia.com> >> >> 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 <p.marczak@samsung.com> >> Cc: Simon Glass <sjg@chromium.org> >> Cc: Thierry Reding <treding@nvidia.com> >> Cc: Bin Meng <bmeng.cn@gmail.com> >> Cc: Michal Suchanek <hramrach@gmail.com> >> Fixes: 02464e386bb5 ("fdt: add new fdt address parsing functions") >> Reported-by: Przemyslaw Marczak <p.marczak@samsung.com> >> Signed-off-by: Stephen Warren <swarren@nvidia.com> >> --- >> lib/fdtdec.c | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) Acked-by: Simon Glass <sjg@chromium.org>
On 29 September 2015 at 05:33, Simon Glass <sjg@chromium.org> wrote: > On 28 September 2015 at 04:41, Przemyslaw Marczak <p.marczak@samsung.com> wrote: >> Hello Stephen, >> >> >> On 09/25/2015 06:11 PM, Stephen Warren wrote: >>> >>> From: Stephen Warren <swarren@nvidia.com> >>> >>> 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 <p.marczak@samsung.com> >>> Cc: Simon Glass <sjg@chromium.org> >>> Cc: Thierry Reding <treding@nvidia.com> >>> Cc: Bin Meng <bmeng.cn@gmail.com> >>> Cc: Michal Suchanek <hramrach@gmail.com> >>> Fixes: 02464e386bb5 ("fdt: add new fdt address parsing functions") >>> Reported-by: Przemyslaw Marczak <p.marczak@samsung.com> >>> Signed-off-by: Stephen Warren <swarren@nvidia.com> >>> --- >>> lib/fdtdec.c | 5 +++-- >>> 1 file changed, 3 insertions(+), 2 deletions(-) > > Acked-by: Simon Glass <sjg@chromium.org> Applied to u-boot-fdt, thanks!
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,