linux-kernel - Re: [PATCH v4 00/10] clk: Add kunit tests for fixed rate and parent data

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Tue, 14 May 2024 14:29:40 -0700
From: Stephen Boyd <sboyd@...nel.org>
To: David Gow <davidgow@...gle.com>, Rob Herring <robh@...nel.org>
Cc: Michael Turquette <mturquette@...libre.com>, linux-kernel@...r.kernel.org, linux-clk@...r.kernel.org, patches@...ts.linux.dev, kunit-dev@...glegroups.com, linux-kselftest@...r.kernel.org, devicetree@...r.kernel.org, Brendan Higgins <brendan.higgins@...ux.dev>, Rae Moar <rmoar@...gle.com>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Rafael J . Wysocki <rafael@...nel.org>, Saravana Kannan <saravanak@...gle.com>, Daniel Latypov <dlatypov@...gle.com>, Christian Marangi <ansuelsmth@...il.com>, Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>, Conor Dooley <conor+dt@...nel.org>, Maxime Ripard <maxime@...no.tech>
Subject: Re: [PATCH v4 00/10] clk: Add kunit tests for fixed rate and parent data

Quoting Stephen Boyd (2024-05-02 18:27:42)
> Quoting David Gow (2024-05-01 01:08:11)
> > 
> > The other thing I've noted so far is that the
> > of_apply_kunit_platform_device and of_overlay_apply_kunit_cleanup
> > tests fail (and BUG() with a NULL pointer) on powerpc:
> > > [15:18:51]     # of_overlay_apply_kunit_platform_device: EXPECTATION FAILED at drivers/of/overlay_test.c:47
> > > [15:18:51]     Expected pdev is not null, but is
> > > [15:18:51] BUG: Kernel NULL pointer dereference at 0x0000004c
> 
> This seems to be because pdev is NULL and we call put_device(&pdev->dev)
> on it. We could be nicer and have an 'if (pdev)' check there. I wonder
> if that fixes the other two below?
> 
> ---8<---
> diff --git a/drivers/of/overlay_test.c b/drivers/of/overlay_test.c
> index 223e5a5c23c5..85cfbe6bb132 100644
> --- a/drivers/of/overlay_test.c
> +++ b/drivers/of/overlay_test.c
> @@ -91,7 +92,8 @@ static void of_overlay_apply_kunit_cleanup(struct kunit *test)
>         dev = bus_find_device(&platform_bus_type, NULL, kunit_compatible,
>                               of_overlay_bus_match_compatible);
>         KUNIT_EXPECT_PTR_EQ(test, NULL, dev);
> -       put_device(dev);
> +       if (dev)
> +               put_device(dev);
>  }

This last hunk isn't needed.

>  
>  static struct kunit_case of_overlay_apply_kunit_test_cases[] = {
> 
> > > [15:18:51]     # of_overlay_apply_kunit_platform_device: try faulted: last line seen lib/kunit/resource.c:99
> > > [15:18:51]     # of_overlay_apply_kunit_platform_device: internal error occurred preventing test case from running: -4
> > > [15:18:51] [FAILED] of_overlay_apply_kunit_platform_device
> > 
> > > [15:18:51] BUG: Kernel NULL pointer dereference at 0x0000004c
> > > [15:18:51] note: kunit_try_catch[698] exited with irqs disabled
> > > [15:18:51]     # of_overlay_apply_kunit_cleanup: try faulted: last line seen drivers/of/overlay_test.c:77
> > > [15:18:51]     # of_overlay_apply_kunit_cleanup: internal error occurred preventing test case from running: -4
> > > [15:18:51] [FAILED] of_overlay_apply_kunit_cleanup
> > 
> > I've not had a chance to dig into it any further, yet, but it appears
> > to work on all of the other architectures I tried.
> 
> Cool. I don't know why powerpc doesn't make devices. Maybe it has a
> similar design to sparc to create resources. I'll check it out.
> 

powerpc doesn't mark the root node with OF_POPULATED_BUS. If I set that
in of_platform_default_populate_init() then the overlays can be applied.

---8<----
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 389d4ea6bfc1..fa7b439e9402 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -565,6 +565,10 @@ static int __init of_platform_default_populate_init(void)
 				of_platform_device_create(node, buf, NULL);
 		}

+		node = of_find_node_by_path("/");
+		if (node)
+			of_node_set_flag(node, OF_POPULATED_BUS);
+		of_node_put(node);
 	} else {
 		/*
 		 * Handle certain compatibles explicitly, since we don't want to create

I'm guessing this is wrong though, because I see bunch of powerpc specific code
calling of_platform_bus_probe() which will set the flag on the actual platform
bus nodes. Maybe we should just allow overlays to create devices at the root
node regardless? Of course, the flag doc says "platform bus created for
children" and if we never populated the root then that isn't entirely accurate.

Rob, can you point me in the right direction? Do we need to use simple-bus in
the test overlays and teach overlay code to populate that bus?