[PATCH] of: unittest: Fix overlay node reference leak in of_notify()

[PATCH] of: unittest: Fix overlay node reference leak in of_notify()

[Wentao Liang]

of_notify() takes a reference on nd->overlay but only drops it in the
default branch of the action switch. The PRE_APPLY, POST_APPLY,
PRE_REMOVE and POST_REMOVE branches return without dropping it, so the
reference is leaked on every notification. Move the of_node_put() to the
common exit path.

Fixes: 992b0dc5c38a ("of: overlay: unittest: add tests for overlay notifiers")
Cc: stable@vger.kernel.org
Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
---
 drivers/of/unittest.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
index c4d4c10ceaa2..5259c45e355d 100644
--- a/drivers/of/unittest.c
+++ b/drivers/of/unittest.c
@@ -3429,11 +3429,12 @@ static int of_notify(struct notifier_block *nb, unsigned long action,
 		break;
 
 	default:			/* should not happen */
-		of_node_put(nd->overlay);
 		ret = -EINVAL;
 		break;
 	}
 
+	of_node_put(nd->overlay);
+
 	if (ret)
 		return notifier_from_errno(ret);
 
-- 
2.34.1

Re: [PATCH] of: unittest: Fix overlay node reference leak in of_notify()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] of: double-free of overlay device node in of_notify()
--

commit 2fb07d7f45ad569912e3b48cd4b44d5fe901b5ee
Author: Wentao Liang <vulab@iscas.ac.cn>

of: unittest: Fix overlay node reference leak in of_notify()

of_notify() takes a reference on nd->overlay but only drops it in the
default branch of the action switch. The PRE_APPLY, POST_APPLY,
PRE_REMOVE and POST_REMOVE branches return without dropping it, so the
reference is leaked on every notification. Move the of_node_put() to the
common exit path.

> The PRE_APPLY, POST_APPLY,
> PRE_REMOVE and POST_REMOVE branches return without dropping it, so the
> reference is leaked on every notification.

Is this description accurate?

Looking at of_find_node_by_name(), it explicitly consumes and drops the
reference count of its from argument. Since these branches all call
of_find_node_by_name(nd->overlay, ...), they appear to naturally drop the
reference on nd->overlay.

Could the commit message be clarified to explain what exactly is leaking?

> diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
> index e255f54f4d760..3202781998808 100644
> --- a/drivers/of/unittest.c
> +++ b/drivers/of/unittest.c
> @@ -3460,11 +3460,12 @@ static int of_notify(struct notifier_block *nb, unsigned long action,
>  		break;
>  
>  	default:			/* should not happen */
> -		of_node_put(nd->overlay);
>  		ret = -EINVAL;
>  		break;
>  	}
>  
> +	of_node_put(nd->overlay);

[Severity: High]
Does moving this of_node_put() into the common exit path cause a double-free
of nd->overlay?

Since the non-default branches already consume the reference via
of_find_node_by_name(nd->overlay, ...):

	case OF_OVERLAY_PRE_APPLY:
		found = of_find_node_by_name(nd->overlay, "test-unittest16");

will this unconditional put at the end of the function result in a second,
unbalanced put on all valid notification paths, potentially leading to a
use-after-free during the OF unittests?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260917124457.2151809-1-vulab@iscas.ac.cn?part=1