Re: [PATCH] of: address: Unify resource bounds overflow checking
From: Rob Herring <robh@kernel.org>
Date: 2024-09-13 18:56:52
Also in:
linux-devicetree, lkml
On Fri, Sep 13, 2024 at 8:15 AM Michael Ellerman [off-list ref] wrote:
Thomas Weißschuh [off-list ref] writes:quoted
The members "start" and "end" of struct resource are of type "resource_size_t" which can be 32bit wide. Values read from OF however are always 64bit wide. Refactor the diff overflow checks into a helper function. Also extend the checks to validate each calculation step. Signed-off-by: Thomas Weißschuh <redacted> --- drivers/of/address.c | 45 ++++++++++++++++++++++++++------------------- 1 file changed, 26 insertions(+), 19 deletions(-)diff --git a/drivers/of/address.c b/drivers/of/address.c index 7e59283a4472..df854bb427ce 100644 --- a/drivers/of/address.c +++ b/drivers/of/address.c@@ -198,6 +198,25 @@ static u64 of_bus_pci_map(__be32 *addr, const __be32 *range, int na, int ns, #endif /* CONFIG_PCI */ +static int __of_address_resource_bounds(struct resource *r, u64 start, u64 size) +{ + u64 end = start; + + if (overflows_type(start, r->start)) + return -EOVERFLOW; + if (size == 0) + return -EOVERFLOW; + if (check_add_overflow(end, size - 1, &end)) + return -EOVERFLOW; + if (overflows_type(end, r->end)) + return -EOVERFLOW;This breaks PCI on powerpc qemu. Part of the PCI probe reads a resource that's zero sized, which used to succeed but now fails due to the size check above. The diff below fixes it for me.
I fixed it up with your change.
quoted hunk ↗ jump to hunk
It leaves r.end == r.start, which is fine in my case, because the code only uses r.start. And it seems more sane than the old code which would return end = start - 1, for zero sized resources. cheersdiff --git a/drivers/of/address.c b/drivers/of/address.c index df854bb427ce..a001e789a6c4 100644 --- a/drivers/of/address.c +++ b/drivers/of/address.c@@ -204,9 +204,7 @@ static int __of_address_resource_bounds(struct resource *r, u64 start, u64 size) if (overflows_type(start, r->start)) return -EOVERFLOW; - if (size == 0) - return -EOVERFLOW; - if (check_add_overflow(end, size - 1, &end)) + if (size > 0 && check_add_overflow(end, size - 1, &end)) return -EOVERFLOW; if (overflows_type(end, r->end)) return -EOVERFLOW;