Devicetree
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Miquel Raynal (Schneider Electric)" <miquel.raynal@bootlin.com>
Cc: robh@kernel.org, devicetree@vger.kernel.org, conor+dt@kernel.org
Subject: Re: [PATCH v4 7/7] clk: tests: Add Kunit testing for nexus nodes
Date: Fri, 17 Jul 2026 17:41:31 +0000	[thread overview]
Message-ID: <20260717174131.CC6961F00A3A@smtp.kernel.org> (raw)
In-Reply-To: <20260717-schneider-v7-2-rc1-eip201-upstream-v4-7-751547e160e5@bootlin.com>

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] `of_find_node_by_name_kunit()` creates a double-free / use-after-free trap via its `from` argument.
- [Medium] `of_find_node_by_name_kunit()` returns an `ERR_PTR` on failure, breaking the expected `NULL` failure semantic of `of_find_node_by_name()`.
--

--- Patch [7]: [PATCH v4 7/7] clk: tests: Add Kunit testing for nexus nodes ---
commit abd4c1ecb509bcaa0e834d4c06b41750affacb27
Author: Miquel Raynal (Schneider Electric) <miquel.raynal@bootlin.com>

clk: tests: Add Kunit testing for nexus nodes

Add a nexus node with a child requesting a mapped clock in the fake DT
overlay to verify that the parsing is also correctly working. Create an
of_find_node_by_name() like kunit helper to garbage collect the node
automatically in case of failed assertion.

> diff --git a/drivers/clk/clk_kunit_helpers.c b/drivers/clk/clk_kunit_helpers.c
> index 68a28e70bb61c..daaf1cf1546c4 100644
> --- a/drivers/clk/clk_kunit_helpers.c
> +++ b/drivers/clk/clk_kunit_helpers.c
[ ... ]
> +struct device_node *of_find_node_by_name_kunit(struct kunit *test, struct device_node *from,
> +					       const char *name)
> +{
> +	struct device_node *np;
> +	int ret;
> +
> +	np = of_find_node_by_name(from, name);

[Severity: Medium]
Will this lead to a double-free if the from argument is another
KUnit-managed node?

of_find_node_by_name() explicitly drops the reference on its from argument.
If a test author passes a node previously returned by
of_find_node_by_name_kunit(), the underlying of_find_node_by_name() will
consume the reference immediately.

KUnit is unaware of this and will execute its deferred action at the end of the
test, calling of_node_put() a second time on the already freed node.

> +	if (!np)
> +		return NULL;
> +
> +	ret = kunit_add_action_or_reset(test, of_node_put_wrapper, np);
> +	if (ret)
> +		return ERR_PTR(ret);

[Severity: Medium]
Could this ERR_PTR() return value cause crashes for callers using standard OF
node pointer checks?

The standard of_find_node_by_name() strictly returns NULL on failure, and
callers typically check for failure using if (!np). Returning an ERR_PTR()
here means truthy checks will pass, potentially leading to an invalid pointer
dereference when the caller attempts to use the returned error pointer as a
device_node struct.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260717-schneider-v7-2-rc1-eip201-upstream-v4-0-751547e160e5@bootlin.com?part=7

      reply	other threads:[~2026-07-17 17:41 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-17 15:59 [PATCH v4 0/7] clk: Add support for clock nexus Miquel Raynal (Schneider Electric)
2026-07-17 15:59 ` [PATCH v4 1/7] dt-bindings: clock: Introduce nexus nodes Miquel Raynal (Schneider Electric)
2026-07-17 16:51   ` sashiko-bot
2026-07-17 15:59 ` [PATCH v4 2/7] clk: tests: Add clk_parse_clkspec() Kunit testing Miquel Raynal (Schneider Electric)
2026-07-17 15:59 ` [PATCH v4 3/7] clk: tests: Add Kunit testing for of_clk_get_parent_name() Miquel Raynal (Schneider Electric)
2026-07-17 17:11   ` sashiko-bot
2026-07-17 15:59 ` [PATCH v4 4/7] clk: Improve a couple of comments Miquel Raynal (Schneider Electric)
2026-07-17 15:59 ` [PATCH v4 5/7] clk: Use the generic OF phandle parsing in only one place Miquel Raynal (Schneider Electric)
2026-07-17 17:28   ` sashiko-bot
2026-07-17 15:59 ` [PATCH v4 6/7] clk: Add support for clock nexus dt bindings Miquel Raynal (Schneider Electric)
2026-07-17 15:59 ` [PATCH v4 7/7] clk: tests: Add Kunit testing for nexus nodes Miquel Raynal (Schneider Electric)
2026-07-17 17:41   ` sashiko-bot [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260717174131.CC6961F00A3A@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=miquel.raynal@bootlin.com \
    --cc=robh@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox