| Message ID | 20240706045454.215701-1-sboyd@kernel.org |
|---|---|
| Headers | show |
| Series | clk: Add kunit tests for fixed rate and parent data | expand |
On Sat, 6 Jul 2024 at 12:55, Stephen Boyd <sboyd@kernel.org> wrote: > > Introduce KUnit resource wrappers around platform_driver_register(), > platform_device_alloc(), and platform_device_add() so that test authors > can register platform drivers/devices from their tests and have the > drivers/devices automatically be unregistered when the test is done. > > This makes test setup code simpler when a platform driver or platform > device is needed. Add a few test cases at the same time to make sure the > APIs work as intended. > > Cc: Brendan Higgins <brendan.higgins@linux.dev> > Reviewed-by: David Gow <davidgow@google.com> > Cc: Rae Moar <rmoar@google.com> > Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Cc: "Rafael J. Wysocki" <rafael@kernel.org> > Signed-off-by: Stephen Boyd <sboyd@kernel.org> > --- Hmm... this is failing under KASAN for me. I'll take a closer look next week, but in case there's anything super-obvious, here's the report: > [15:54:33] BUG: KASAN: slab-use-after-free in kobject_put+0x224/0x280 > [15:54:33] Read of size 1 at addr ffff888002ca484c by task kunit_try_catch/226 > [15:54:33] > [15:54:33] CPU: 0 PID: 226 Comm: kunit_try_catch Tainted: G D N 6.10.0-rc1-g21fbb4cfd3cc #25 > [15:54:33] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014 > [15:54:33] Call Trace: > [15:54:33] <TASK> > [15:54:33] dump_stack_lvl+0x3f/0x50 > [15:54:33] print_report+0xcd/0x620 > [15:54:33] ? kobject_put+0x224/0x280 > [15:54:33] kasan_report+0xde/0x110 > [15:54:33] ? kobject_put+0x224/0x280 > [15:54:33] kobject_put+0x224/0x280 > [15:54:33] kunit_remove_resource+0x12c/0x1d0 > [15:54:33] kunit_cleanup+0x8d/0x130 > [15:54:33] ? __pfx_kunit_try_run_case_cleanup+0x10/0x10 > [15:54:33] ? __pfx_kunit_generic_run_threadfn_adapter+0x10/0x10 > [15:54:33] kunit_generic_run_threadfn_adapter+0x7b/0xe0 > [15:54:33] kthread+0x289/0x360 > [15:54:33] ? __pfx_kthread+0x10/0x10 > [15:54:33] ret_from_fork+0x2f/0x70 > [15:54:33] ? __pfx_kthread+0x10/0x10 > [15:54:33] ret_from_fork_asm+0x19/0x30 > [15:54:33] </TASK> > [15:54:33] > [15:54:33] Allocated by task 225: > [15:54:33] kasan_save_stack+0x33/0x60 > [15:54:33] kasan_save_track+0x14/0x30 > [15:54:33] __kasan_kmalloc+0x8f/0xa0 > [15:54:33] platform_device_alloc+0x26/0x200 > [15:54:33] kunit_platform_device_alloc_init+0x52/0xc0 > [15:54:33] __kunit_add_resource+0x98/0x1e0 > [15:54:33] kunit_platform_device_alloc+0xe7/0x170 > [15:54:33] kunit_platform_device_add_test+0x91/0x560 > [15:54:33] kunit_try_run_case+0x1ad/0x490 > [15:54:33] kunit_generic_run_threadfn_adapter+0x7b/0xe0 > [15:54:33] kthread+0x289/0x360 > [15:54:33] ret_from_fork+0x2f/0x70 > [15:54:33] ret_from_fork_asm+0x19/0x30 > [15:54:33] > [15:54:33] Freed by task 226: > [15:54:33] kasan_save_stack+0x33/0x60 > [15:54:33] kasan_save_track+0x14/0x30 > [15:54:33] kasan_save_free_info+0x3b/0x60 > [15:54:33] __kasan_slab_free+0x101/0x160 > [15:54:33] kfree+0x94/0x190 > [15:54:33] device_release+0x9a/0x210 > [15:54:33] kobject_put+0x150/0x280 > [15:54:33] kunit_remove_resource+0x12c/0x1d0 > [15:54:33] kunit_cleanup+0x8d/0x130 > [15:54:33] kunit_generic_run_threadfn_adapter+0x7b/0xe0 > [15:54:33] kthread+0x289/0x360 > [15:54:33] ret_from_fork+0x2f/0x70 > [15:54:33] ret_from_fork_asm+0x19/0x30 > [15:54:33] > [15:54:33] The buggy address belongs to the object at ffff888002ca4800 > [15:54:33] which belongs to the cache kmalloc-1k of size 1024 > [15:54:33] The buggy address is located 76 bytes inside of > [15:54:33] freed 1024-byte region [ffff888002ca4800, ffff888002ca4c00) > [15:54:33] > [15:54:33] The buggy address belongs to the physical page: > [15:54:33] page: refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x2ca4 > [15:54:33] head: order:2 mapcount:0 entire_mapcount:0 nr_pages_mapped:0 pincount:0 > [15:54:33] flags: 0x4000000000000040(head|zone=1) > [15:54:33] page_type: 0xffffefff(slab) > [15:54:33] raw: 4000000000000040 ffff888001041b00 dead000000000122 0000000000000000 > [15:54:33] raw: 0000000000000000 0000000080080008 00000001ffffefff 0000000000000000 > [15:54:33] head: 4000000000000040 ffff888001041b00 dead000000000122 0000000000000000 > [15:54:33] head: 0000000000000000 0000000080080008 00000001ffffefff 0000000000000000 > [15:54:33] head: 4000000000000002 ffffea00000b2901 ffffffffffffffff 0000000000000000 > [15:54:33] head: 0000000000000004 0000000000000000 00000000ffffffff 0000000000000000 > [15:54:33] page dumped because: kasan: bad access detected > [15:54:33] > [15:54:33] Memory state around the buggy address: > [15:54:33] ffff888002ca4700: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc > [15:54:33] ffff888002ca4780: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc > [15:54:33] >ffff888002ca4800: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb > [15:54:33] ^ > [15:54:33] ffff888002ca4880: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb > [15:54:33] ffff888002ca4900: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb > [15:54:33] ================================================================== > [15:54:33] ------------[ cut here ]------------ > [15:54:33] refcount_t: underflow; use-after-free. > [15:54:33] WARNING: CPU: 0 PID: 226 at lib/refcount.c:28 refcount_warn_saturate+0xf2/0x150 > [15:54:33] CPU: 0 PID: 226 Comm: kunit_try_catch Tainted: G B D N 6.10.0-rc1-g21fbb4cfd3cc #25 > [15:54:33] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014 > [15:54:33] RIP: 0010:refcount_warn_saturate+0xf2/0x150 > [15:54:33] Code: 3b aa cb 00 01 e8 3e c1 b0 ff 0f 0b eb 91 80 3d 2a aa cb 00 00 75 88 48 c7 c7 40 f3 88 ad c6 05 1a aa cb 00 01 e8 1e c1 b0 ff <0f> 0b e9 6e ff ff ff 80 3d 0a aa cb 00 00 0f 85 61 ff ff ff 48 c7 > [15:54:33] RSP: 0018:ffff88800357fe58 EFLAGS: 00000286 > [15:54:33] RAX: 0000000000000000 RBX: ffff888002ca4848 RCX: 1ffffffff5b920d4 > [15:54:33] RDX: 0000000000000000 RSI: 0000000000000004 RDI: ffffffffadc906a0 > [15:54:33] RBP: 0000000000000003 R08: 0000000000000000 R09: fffffbfff5b920d4 > [15:54:33] R10: 0000000000000003 R11: 0000000000000001 R12: ffff88800111fb28 > [15:54:33] R13: ffff88800111fb28 R14: ffff88800111fb28 R15: 0000000000000286 > [15:54:33] FS: 0000000000000000(0000) GS:ffffffffadc6f000(0000) knlGS:0000000000000000 > [15:54:33] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > [15:54:33] CR2: dffffc0000000000 CR3: 000000000a25a000 CR4: 00000000000006f0 > [15:54:33] Call Trace: > [15:54:33] <TASK> > [15:54:33] ? __warn+0xb0/0x170 > [15:54:33] ? refcount_warn_saturate+0xf2/0x150 > [15:54:33] ? report_bug+0x298/0x350 > [15:54:33] ? handle_bug+0x6d/0x90 > [15:54:33] ? exc_invalid_op+0x17/0x40 > [15:54:33] ? asm_exc_invalid_op+0x1a/0x20 > [15:54:33] ? refcount_warn_saturate+0xf2/0x150 > [15:54:33] ? refcount_warn_saturate+0xf2/0x150 > [15:54:33] kunit_remove_resource+0x12c/0x1d0 > [15:54:33] kunit_cleanup+0x8d/0x130 > [15:54:33] ? __pfx_kunit_try_run_case_cleanup+0x10/0x10 > [15:54:33] ? __pfx_kunit_generic_run_threadfn_adapter+0x10/0x10 > [15:54:33] kunit_generic_run_threadfn_adapter+0x7b/0xe0 > [15:54:33] kthread+0x289/0x360 > [15:54:33] ? __pfx_kthread+0x10/0x10 > [15:54:33] ret_from_fork+0x2f/0x70 > [15:54:33] ? __pfx_kthread+0x10/0x10 > [15:54:33] ret_from_fork_asm+0x19/0x30 > [15:54:33] </TASK> > [15:54:33] ---[ end trace 0000000000000000 ]--- > [15:54:33] [FAILED] kunit_platform_device_add_test It repros here with: ./tools/testing/kunit/kunit.py run --arch x86_64 --kconfig_add CONFIG_KASAN=y --kconfig_add CONFIG_KASAN_VMALLOC=y kunit_platform_driver Seems to otherwise pass on all of my default configs/archs. Cheers, -- David
Quoting David Gow (2024-07-06 01:04:25) > On Sat, 6 Jul 2024 at 12:55, Stephen Boyd <sboyd@kernel.org> wrote: > > > > Introduce KUnit resource wrappers around platform_driver_register(), > > platform_device_alloc(), and platform_device_add() so that test authors > > can register platform drivers/devices from their tests and have the > > drivers/devices automatically be unregistered when the test is done. > > > > This makes test setup code simpler when a platform driver or platform > > device is needed. Add a few test cases at the same time to make sure the > > APIs work as intended. > > > > Cc: Brendan Higgins <brendan.higgins@linux.dev> > > Reviewed-by: David Gow <davidgow@google.com> > > Cc: Rae Moar <rmoar@google.com> > > Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > Cc: "Rafael J. Wysocki" <rafael@kernel.org> > > Signed-off-by: Stephen Boyd <sboyd@kernel.org> > > --- > > Hmm... this is failing under KASAN for me. I'll take a closer look > next week, but in case there's anything super-obvious, here's the > report: Thanks. I reproduced with your commandline and I'll take a look.
Quoting Stephen Boyd (2024-07-07 11:02:06) > Quoting David Gow (2024-07-06 01:04:25) > > On Sat, 6 Jul 2024 at 12:55, Stephen Boyd <sboyd@kernel.org> wrote: > > > > > > Introduce KUnit resource wrappers around platform_driver_register(), > > > platform_device_alloc(), and platform_device_add() so that test authors > > > can register platform drivers/devices from their tests and have the > > > drivers/devices automatically be unregistered when the test is done. > > > > > > This makes test setup code simpler when a platform driver or platform > > > device is needed. Add a few test cases at the same time to make sure the > > > APIs work as intended. > > > > > > Cc: Brendan Higgins <brendan.higgins@linux.dev> > > > Reviewed-by: David Gow <davidgow@google.com> > > > Cc: Rae Moar <rmoar@google.com> > > > Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > > Cc: "Rafael J. Wysocki" <rafael@kernel.org> > > > Signed-off-by: Stephen Boyd <sboyd@kernel.org> > > > --- > > > > Hmm... this is failing under KASAN for me. I'll take a closer look > > next week, but in case there's anything super-obvious, here's the > > report: > > Thanks. I reproduced with your commandline and I'll take a look. > Seems that I forgot to fold this in when I was testing. ----8<--- diff --git a/lib/kunit/platform.c b/lib/kunit/platform.c index ba1b0006dc45..0b518de26065 100644 --- a/lib/kunit/platform.c +++ b/lib/kunit/platform.c @@ -75,7 +75,7 @@ kunit_platform_device_alloc_match(struct kunit *test, { struct platform_device *pdev = match_data; - return res->data == pdev && res->free != kunit_platform_device_alloc_exit; + return res->data == pdev && res->free == kunit_platform_device_alloc_exit; } KUNIT_DEFINE_ACTION_WRAPPER(platform_device_unregister_wrapper,
This patch series adds unit tests for the clk fixed rate basic type and the clk registration functions that use struct clk_parent_data. To get there, we add support for loading device tree overlays onto the live DTB along with probing platform drivers to bind to device nodes in the overlays. With this series, we're able to exercise some of the code in the common clk framework that uses devicetree lookups to find parents and the fixed rate clk code that scans device tree directly and creates clks. Please review. I Cced everyone to all the patches so they get the full context. I'm hoping I can take the whole pile through the clk tree as they all build upon each other. Or the DT part can be merged through the DT tree to reduce the dependencies. Changes from v5: https://lore.kernel.org/r/20240603223811.3815762-1-sboyd@kernel.org * Pick up reviewed-by tags * Drop test vendor prefix bindings as dtschema allows anything now * Use of_node_put_kunit() more to plug some reference leaks * Select DTC config to avoid compile fails because of missing dtc * Don't skip for OF_OVERLAY in overlay tests because they depend on it Changes from v4: https://lore.kernel.org/r/20240422232404.213174-1-sboyd@kernel.org * Picked up reviewed-by tags * Check for non-NULL device pointers before calling put_device() * Fix CFI issues with kunit actions * Introduce platform_device_prepare_wait_for_probe() helper to wait for a platform device to probe * Move platform code to lib/kunit and rename functions to have kunit prefix * Fix issue with platform wrappers messing up reference counting because they used kunit actions * New patch to populate overlay devices on root node for powerpc * Make fixed-rate binding generic single clk consumer binding Changes from v3: https://lore.kernel.org/r/20230327222159.3509818-1-sboyd@kernel.org * No longer depend on Frank's series[1] because it was merged upstream[2] * Use kunit_add_action_or_reset() to shorten code * Skip tests properly when CONFIG_OF_OVERLAY isn't set Changes from v2: https://lore.kernel.org/r/20230315183729.2376178-1-sboyd@kernel.org * Overlays don't depend on __symbols__ node * Depend on Frank's always create root node if CONFIG_OF series[1] * Added kernel-doc to KUnit API doc * Fixed some kernel-doc on functions * More test cases for fixed rate clk Changes from v1: https://lore.kernel.org/r/20230302013822.1808711-1-sboyd@kernel.org * Don't depend on UML, use unittest data approach to attach nodes * Introduce overlay loading API for KUnit * Move platform_device KUnit code to drivers/base/test * Use #define macros for constants shared between unit tests and overlays * Settle on "test" as a vendor prefix * Make KUnit wrappers have "_kunit" postfix [1] https://lore.kernel.org/r/20230317053415.2254616-1-frowand.list@gmail.com [2] https://lore.kernel.org/r/20240308195737.GA1174908-robh@kernel.org Stephen Boyd (8): of/platform: Allow overlays to create platform devices from the root node of: Add test managed wrappers for of_overlay_apply()/of_node_put() dt-bindings: vendor-prefixes: Add "test" vendor for KUnit and friends of: Add a KUnit test for overlays and test managed APIs platform: Add test managed platform_device/driver APIs clk: Add test managed clk provider/consumer APIs clk: Add KUnit tests for clk fixed rate basic type clk: Add KUnit tests for clks registered with struct clk_parent_data Documentation/dev-tools/kunit/api/clk.rst | 10 + Documentation/dev-tools/kunit/api/index.rst | 21 + Documentation/dev-tools/kunit/api/of.rst | 13 + .../dev-tools/kunit/api/platformdevice.rst | 10 + .../devicetree/bindings/vendor-prefixes.yaml | 2 + drivers/clk/.kunitconfig | 2 + drivers/clk/Kconfig | 11 + drivers/clk/Makefile | 9 +- drivers/clk/clk-fixed-rate_test.c | 379 +++++++++++++++ drivers/clk/clk-fixed-rate_test.h | 8 + drivers/clk/clk_kunit_helpers.c | 204 ++++++++ drivers/clk/clk_parent_data_test.h | 10 + drivers/clk/clk_test.c | 453 +++++++++++++++++- drivers/clk/kunit_clk_fixed_rate_test.dtso | 19 + drivers/clk/kunit_clk_parent_data_test.dtso | 28 ++ drivers/of/.kunitconfig | 1 + drivers/of/Kconfig | 10 + drivers/of/Makefile | 2 + drivers/of/kunit_overlay_test.dtso | 9 + drivers/of/of_kunit_helpers.c | 74 +++ drivers/of/overlay_test.c | 114 +++++ drivers/of/platform.c | 9 +- include/kunit/clk.h | 28 ++ include/kunit/of.h | 115 +++++ include/kunit/platform_device.h | 20 + lib/kunit/Makefile | 4 +- lib/kunit/platform-test.c | 223 +++++++++ lib/kunit/platform.c | 302 ++++++++++++ 28 files changed, 2084 insertions(+), 6 deletions(-) create mode 100644 Documentation/dev-tools/kunit/api/clk.rst create mode 100644 Documentation/dev-tools/kunit/api/of.rst create mode 100644 Documentation/dev-tools/kunit/api/platformdevice.rst create mode 100644 drivers/clk/clk-fixed-rate_test.c create mode 100644 drivers/clk/clk-fixed-rate_test.h create mode 100644 drivers/clk/clk_kunit_helpers.c create mode 100644 drivers/clk/clk_parent_data_test.h create mode 100644 drivers/clk/kunit_clk_fixed_rate_test.dtso create mode 100644 drivers/clk/kunit_clk_parent_data_test.dtso create mode 100644 drivers/of/kunit_overlay_test.dtso create mode 100644 drivers/of/of_kunit_helpers.c create mode 100644 drivers/of/overlay_test.c create mode 100644 include/kunit/clk.h create mode 100644 include/kunit/of.h create mode 100644 include/kunit/platform_device.h create mode 100644 lib/kunit/platform-test.c create mode 100644 lib/kunit/platform.c base-commit: 1613e604df0cd359cf2a7fbd9be7a0bcfacfabd0