| Message ID | 20190905084849.20596-4-matthias.bgg@kernel.org |
|---|---|
| State | Accepted |
| Commit | 8076fc298ee1004cfbd9f9f4882931aff1ffda06 |
| Delegated to: | Simon Glass |
| Headers | show |
| Series | Fix default values for address and size cells | expand |
On Thu, 5 Sep 2019 at 02:49, <matthias.bgg@kernel.org> wrote: > > From: Matthias Brugger <mbrugger@suse.com> > > The commit "libfdt: fdt_address_cells() and fdt_size_cells()" introduced > a bug as it consolidated code between the helpers for getting > be 0, and is frequently found so in practice for /cpus. IEEE1275 only > requires implementations to handle 1..4 for #address-cells, although one > could make a case for #address-cells == #size-cells == 0 being used to > represent a bridge with a single port. > > While we're there, it's not totally obvious that the existing implicit > cast of a u32 to int will give the correct results according to strict C, > although it does work in practice. Straighten that up to cast only after > we've made our range checks. > > This is based on upstream commit: > b8d6eca ("libfdt: Allow #size-cells of 0") > but misses the test cases,as we don't implement them in U-Boot. > > Signed-off-by: Matthias Brugger <mbrugger@suse.com> > --- > > scripts/dtc/libfdt/fdt_addresses.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) This is v2 but I don't see a change log? - Simon
On Mon, 16 Sep 2019 at 22:48, Simon Glass <sjg@chromium.org> wrote: > > On Thu, 5 Sep 2019 at 02:49, <matthias.bgg@kernel.org> wrote: > > > > From: Matthias Brugger <mbrugger@suse.com> > > > > The commit "libfdt: fdt_address_cells() and fdt_size_cells()" introduced > > a bug as it consolidated code between the helpers for getting > > be 0, and is frequently found so in practice for /cpus. IEEE1275 only > > requires implementations to handle 1..4 for #address-cells, although one > > could make a case for #address-cells == #size-cells == 0 being used to > > represent a bridge with a single port. > > > > While we're there, it's not totally obvious that the existing implicit > > cast of a u32 to int will give the correct results according to strict C, > > although it does work in practice. Straighten that up to cast only after > > we've made our range checks. > > > > This is based on upstream commit: > > b8d6eca ("libfdt: Allow #size-cells of 0") > > but misses the test cases,as we don't implement them in U-Boot. > > > > Signed-off-by: Matthias Brugger <mbrugger@suse.com> > > --- > > > > scripts/dtc/libfdt/fdt_addresses.c | 8 +++++--- > > 1 file changed, 5 insertions(+), 3 deletions(-) > > This is v2 but I don't see a change log? Reviewed-by: Simon Glass <sjg@chromium.org> (please use patman if you can next time)
On Mon, 16 Sep 2019 at 22:48, Simon Glass <sjg@chromium.org> wrote: > > On Thu, 5 Sep 2019 at 02:49, <matthias.bgg@kernel.org> wrote: > > > > From: Matthias Brugger <mbrugger@suse.com> > > > > The commit "libfdt: fdt_address_cells() and fdt_size_cells()" introduced > > a bug as it consolidated code between the helpers for getting > > be 0, and is frequently found so in practice for /cpus. IEEE1275 only > > requires implementations to handle 1..4 for #address-cells, although one > > could make a case for #address-cells == #size-cells == 0 being used to > > represent a bridge with a single port. > > > > While we're there, it's not totally obvious that the existing implicit > > cast of a u32 to int will give the correct results according to strict C, > > although it does work in practice. Straighten that up to cast only after > > we've made our range checks. > > > > This is based on upstream commit: > > b8d6eca ("libfdt: Allow #size-cells of 0") > > but misses the test cases,as we don't implement them in U-Boot. > > > > Signed-off-by: Matthias Brugger <mbrugger@suse.com> > > --- > > > > scripts/dtc/libfdt/fdt_addresses.c | 8 +++++--- > > 1 file changed, 5 insertions(+), 3 deletions(-) > > This is v2 but I don't see a change log? Reviewed-by: Simon Glass <sjg@chromium.org> (please use patman if you can next time) Applied to u-boot-dm/next, thanks!
diff --git a/scripts/dtc/libfdt/fdt_addresses.c b/scripts/dtc/libfdt/fdt_addresses.c index f13a87dfa0..788c143113 100644 --- a/scripts/dtc/libfdt/fdt_addresses.c +++ b/scripts/dtc/libfdt/fdt_addresses.c @@ -59,7 +59,7 @@ static int fdt_cells(const void *fdt, int nodeoffset, const char *name) { const fdt32_t *c; - int val; + uint32_t val; int len; c = fdt_getprop(fdt, nodeoffset, name, &len); @@ -70,10 +70,10 @@ static int fdt_cells(const void *fdt, int nodeoffset, const char *name) return -FDT_ERR_BADNCELLS; val = fdt32_to_cpu(*c); - if ((val <= 0) || (val > FDT_MAX_NCELLS)) + if (val > FDT_MAX_NCELLS) return -FDT_ERR_BADNCELLS; - return val; + return (int)val; } int fdt_address_cells(const void *fdt, int nodeoffset) @@ -81,6 +81,8 @@ int fdt_address_cells(const void *fdt, int nodeoffset) int val; val = fdt_cells(fdt, nodeoffset, "#address-cells"); + if (val == 0) + return -FDT_ERR_BADNCELLS; if (val == -FDT_ERR_NOTFOUND) return 2; return val;