of: dynamic: resource: add WARN in release_resource for DT overlays

of: dynamic: resource: add WARN in release_resource for DT overlays

[Robert ABEL]

This patch prevents a kernel OOPS when removing a DT overlay containing
nodes with reg properties. release_resources is called for these nodes.
However, the resource structs were never initialized, hence the kernel OOPS.

This is obviously a stopgap measure until a proper solution is coded, see [1].


[1]: https://lkml.org/lkml/2014/4/17/359

[PATCH] of: resource: add WARN for invalid release_resource calls

[Robert ABEL]

Signed-off-by: Robert ABEL <rabel@cit-ec.uni-bielefeld.de>
---
 kernel/resource.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/kernel/resource.c b/kernel/resource.c
index 0bcebff..b4c9b27 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -236,6 +236,13 @@ static int __release_resource(struct resource *old)
 {
 	struct resource *tmp, **p;
 
+    /* devicetree overlays:
+     * of code doesn't initialize parent, child, sibling
+     * gracefully 'do the right thing' here
+     */
+	if (WARN(!old->parent, "%s: uninitialized resource %s\n", __FUNCTION__, old->name))
+		return 0;
+
 	p = &old->parent->child;
 	for (;;) {
 		tmp = *p;
-- 
2.5.0

Re: of: dynamic: resource: add WARN in release_resource for DT overlays

[Andrew Morton]

On Thu, 30 Jul 2015 15:17:16 +0200 Robert ABEL <rabel@cit-ec.uni-bielefeld.de> wrote:

> 
> This patch prevents a kernel OOPS when removing a DT overlay containing
> nodes with reg properties. release_resources is called for these nodes.
> However, the resource structs were never initialized, hence the kernel OOPS.
> 
> This is obviously a stopgap measure until a proper solution is coded, see [1].
> 
> 
> [1]: https://lkml.org/lkml/2014/4/17/359
>
> ...
>
> --- a/kernel/resource.c
> +++ b/kernel/resource.c
> @@ -236,6 +236,13 @@ static int __release_resource(struct resource *old)
>  {
>  	struct resource *tmp, **p;
>  
> +    /* devicetree overlays:
> +     * of code doesn't initialize parent, child, sibling
> +     * gracefully 'do the right thing' here
> +     */
> +	if (WARN(!old->parent, "%s: uninitialized resource %s\n", __FUNCTION__, old->name))
> +		return 0;
> +
>  	p = &old->parent->child;
>  	for (;;) {
>  		tmp = *p;

Can we please have a (much) better changelog for this?  Don't ask
people to go off and trawl through lkml.org threads (especially as
lkml.org seems to be down half the time).

What precisely is causing the crash, what are the plans to fix it, why
do we need a stopgap rather than fixing it properly, etc.

Why not put the workaround at the offending call site (whereever that
is?) rather than in the callee?