The overflow checking has to deal with different datatypes and edgecases. Add a new kunit testcase to make sure it works correctly. Signed-off-by: Thomas Weißschuh <thomas.weissschuh@xxxxxxxxxxxxx> --- Changes in v3: - Avoid constant truncation warnings by using u64 for test case expectations - Run through 0day before submission - Link to v2: https://lore.kernel.org/r/20250127-of-address-overflow-v2-1-61b5046044e9@xxxxxxxxxxxxx Changes in v2: - Rebase on robh/for-master - Drop already applied patch - Add missing MODULE_IMPORT_NS() - Fix sparse warnings: "cast truncates bits from constant value" - Link to v1: https://lore.kernel.org/r/20250120-of-address-overflow-v1-0-dd68dbf47bce@xxxxxxxxxxxxx --- Technically it's possible to run this unittest with !CONFIG_OF_ADDRESS, so there is an explicit check inside the test. It would also be possible to add a dedicated source file, but that seemed like a lot of churn to me. --- drivers/of/address.c | 5 +- drivers/of/of_private.h | 4 ++ drivers/of/of_test.c | 119 +++++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 126 insertions(+), 2 deletions(-) diff --git a/drivers/of/address.c b/drivers/of/address.c index 26f7fc3d759976539dbd05fb07fe25b27ae4faa7..4976cdf33dd4d81b0de6b979bb79bce734634af5 100644 --- a/drivers/of/address.c +++ b/drivers/of/address.c @@ -16,6 +16,8 @@ #include <linux/string.h> #include <linux/dma-direct.h> /* for bus_dma_region */ +#include <kunit/visibility.h> + /* Uncomment me to enable of_dump_addr() debugging output */ // #define DEBUG @@ -183,7 +185,7 @@ 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) +VISIBLE_IF_KUNIT int __of_address_resource_bounds(struct resource *r, u64 start, u64 size) { if (overflows_type(start, r->start)) return -EOVERFLOW; @@ -197,6 +199,7 @@ static int __of_address_resource_bounds(struct resource *r, u64 start, u64 size) return 0; } +EXPORT_SYMBOL_IF_KUNIT(__of_address_resource_bounds); /* * of_pci_range_to_resource - Create a resource from an of_pci_range diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h index f3e1193c8ded4899f39677a76da073e2266a1b9a..1bdc7ceef3c5fc854bd7708a50281bbfa439838d 100644 --- a/drivers/of/of_private.h +++ b/drivers/of/of_private.h @@ -208,4 +208,8 @@ static void __maybe_unused of_dump_addr(const char *s, const __be32 *addr, int n static void __maybe_unused of_dump_addr(const char *s, const __be32 *addr, int na) { } #endif +#if IS_ENABLED(CONFIG_KUNIT) +int __of_address_resource_bounds(struct resource *r, u64 start, u64 size); +#endif + #endif /* _LINUX_OF_PRIVATE_H */ diff --git a/drivers/of/of_test.c b/drivers/of/of_test.c index b0557ded838fdf70f0b679c31ead38f501371304..8bba5a72c9c7ca7d1ac9b1c5a4dbe2850c328c43 100644 --- a/drivers/of/of_test.c +++ b/drivers/of/of_test.c @@ -2,6 +2,7 @@ /* * KUnit tests for OF APIs */ +#include <linux/ioport.h> #include <linux/module.h> #include <linux/of.h> @@ -54,8 +55,124 @@ static struct kunit_suite of_dtb_suite = { .init = of_dtb_test_init, }; +struct of_address_resource_bounds_case { + u64 start; + u64 size; + int ret; + + u64 res_start; + u64 res_end; +}; + +static void of_address_resource_bounds_case_desc(const struct of_address_resource_bounds_case *p, + char *name) +{ + snprintf(name, KUNIT_PARAM_DESC_SIZE, "start=0x%016llx,size=0x%016llx", p->start, p->size); +} + +static const struct of_address_resource_bounds_case of_address_resource_bounds_cases[] = { + { + .start = 0, + .size = 0, + .ret = 0, + .res_start = 0, + .res_end = -1, + }, + { + .start = 0, + .size = 0x1000, + .ret = 0, + .res_start = 0, + .res_end = 0xfff, + }, + { + .start = 0x1000, + .size = 0, + .ret = 0, + .res_start = 0x1000, + .res_end = 0xfff, + }, + { + .start = 0x1000, + .size = 0x1000, + .ret = 0, + .res_start = 0x1000, + .res_end = 0x1fff, + }, + { + .start = 1, + .size = RESOURCE_SIZE_MAX, + .ret = 0, + .res_start = 1, + .res_end = RESOURCE_SIZE_MAX, + }, + { + .start = RESOURCE_SIZE_MAX, + .size = 1, + .ret = 0, + .res_start = RESOURCE_SIZE_MAX, + .res_end = RESOURCE_SIZE_MAX, + }, + { + .start = 2, + .size = RESOURCE_SIZE_MAX, + .ret = -EOVERFLOW, + }, + { + .start = RESOURCE_SIZE_MAX, + .size = 2, + .ret = -EOVERFLOW, + }, + { + .start = ULL(0x100000000), + .size = 1, + .ret = sizeof(resource_size_t) > sizeof(u32) ? 0 : -EOVERFLOW, + .res_start = ULL(0x100000000), + .res_end = ULL(0x100000000), + }, + { + .start = 0x1000, + .size = 0xffffffff, + .ret = sizeof(resource_size_t) > sizeof(u32) ? 0 : -EOVERFLOW, + .res_start = 0x1000, + .res_end = ULL(0x100000ffe), + }, +}; + +KUNIT_ARRAY_PARAM(of_address_resource_bounds, + of_address_resource_bounds_cases, of_address_resource_bounds_case_desc); + +static void of_address_resource_bounds(struct kunit *test) +{ + const struct of_address_resource_bounds_case *param = test->param_value; + struct resource r; /* Intentionally uninitialized */ + int ret; + + if (!IS_ENABLED(CONFIG_OF_ADDRESS)) + kunit_skip(test, "CONFIG_OF_ADDRESS not enabled\n"); + + ret = __of_address_resource_bounds(&r, param->start, param->size); + KUNIT_EXPECT_EQ(test, param->ret, ret); + if (ret == 0) { + KUNIT_EXPECT_EQ(test, (resource_size_t)param->res_start, r.start); + KUNIT_EXPECT_EQ(test, (resource_size_t)param->res_end, r.end); + KUNIT_EXPECT_EQ(test, param->size, resource_size(&r)); + } +} + +static struct kunit_case of_address_test_cases[] = { + KUNIT_CASE_PARAM(of_address_resource_bounds, of_address_resource_bounds_gen_params), + {} +}; + +static struct kunit_suite of_address_suite = { + .name = "of_address", + .test_cases = of_address_test_cases, +}; + kunit_test_suites( - &of_dtb_suite, + &of_dtb_suite, &of_address_suite, ); MODULE_DESCRIPTION("KUnit tests for OF APIs"); +MODULE_IMPORT_NS("EXPORTED_FOR_KUNIT_TESTING"); MODULE_LICENSE("GPL"); --- base-commit: 15e2f65f2ecfeb8e39315522e2b5cfdc5651fc10 change-id: 20250120-of-address-overflow-a59476362885 Best regards, -- Thomas Weißschuh <thomas.weissschuh@xxxxxxxxxxxxx>
