From: sashiko-bot@kernel.org
To: "Linus Walleij" <linusw@kernel.org>
Cc: vkoul@kernel.org, Frank.Li@kernel.org, dmaengine@vger.kernel.org
Subject: Re: [PATCH v2 09/13] dmaengine: ste_dma40: Fix memcpy channel parsing
Date: Thu, 20 Aug 2026 13:29:47 +0000 [thread overview]
Message-ID: <20260820132947.C10EC1F00A3A@smtp.kernel.org> (raw)
In-Reply-To: <20260820-dma40-fixes-v2-9-63238334c707@kernel.org>
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
Pre-existing issues:
- [High] Out-of-bounds array access in `d40_phy_res_init` due to unvalidated `disabled-channels` property values.
- [Medium] Leaked OF node reference from `of_parse_phandle()` in `d40_probe()`.
- [Medium] Global `dma40_memcpy_channels` array is modified without locking, preventing multiple instances of the device from configuring correctly.
- [Medium] Unvalidated `args_count` in `d40_xlate` leads to reading uninitialized stack memory.
- [Medium] The driver fails to probe if the optional 'memcpy-channels' property is missing from the device tree.
--
--- Patch [9]: [PATCH v2 09/13] dmaengine: ste_dma40: Fix memcpy channel parsing ---
commit 50b8205871db7013fde7fb8ac4433303d26eb570
Author: Linus Walleij <linusw@kernel.org>
dmaengine: ste_dma40: Fix memcpy channel parsing
d40_of_probe() validates the memcpy-channels property against
D40_MEMCPY_MAX_CHANS, but reads the property directly into
dma40_memcpy_channels. That array has fewer entries, so a long property
can overwrite adjacent data.
Validate the property length against ARRAY_SIZE(dma40_memcpy_channels)
before reading it into the array.
Closes: https://lore.kernel.org/dmaengine/20260819225215.5EA2F1F000E9@smtp.kernel.org/
> diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
> index 8cf73b37f8bfd..4e840846466b8 100644
> --- a/drivers/dma/ste_dma40.c
> +++ b/drivers/dma/ste_dma40.c
> @@ -3470,7 +3470,8 @@ static int __init d40_of_probe(struct device *dev,
> list = of_get_property(np, "memcpy-channels", &num_memcpy);
> num_memcpy /= sizeof(*list);
>
> - if (num_memcpy > D40_MEMCPY_MAX_CHANS || num_memcpy <= 0) {
> + if (num_memcpy > ARRAY_SIZE(dma40_memcpy_channels) ||
> + num_memcpy <= 0) {
[Severity: Medium]
This is a pre-existing issue, but does this cause the probe to fail if the
"memcpy-channels" property is completely omitted from the device tree? If
of_get_property() returns NULL, num_memcpy would be 0, which triggers this
error path. Should this property be strictly required, or can the driver
continue without it?
[Severity: Medium]
This is also a pre-existing issue, but does reading the "memcpy-channels"
property directly into the global dma40_memcpy_channels array cause problems
if there are multiple instances of this device? It seems probing subsequent
instances would silently overwrite the global array and corrupt the channel
mapping for previously probed controllers.
> d40_err(dev,
> "Invalid number of memcpy channels specified (%d)\n",
> num_memcpy);
[ ... ]
[Severity: Medium]
This isn't a bug introduced by this patch, but does d40_probe() leak an OF
node reference?
drivers/dma/ste_dma40.c:d40_probe() {
...
np_lcpa = of_parse_phandle(np, "sram", 0);
...
}
The of_parse_phandle() call increments the node's reference count, but I
don't see a matching of_node_put() to release it. Can this cause a memory
leak when the driver probes?
[Severity: High]
This is a pre-existing issue, but is it possible for malformed device tree
values to cause an out-of-bounds write here?
drivers/dma/ste_dma40.c:d40_phy_res_init() {
...
base->phy_res[chan].allocated_src = D40_ALLOC_PHY;
...
}
It appears d40_of_probe() reads the "disabled-channels" property but only
verifies the element count, without validating the actual channel integers.
If those unvalidated values are used as direct indices in d40_phy_res_init(),
could this corrupt the heap array?
[Severity: Medium]
This is a pre-existing issue, but does d40_xlate() read uninitialized memory
if the device tree is misconfigured?
drivers/dma/ste_dma40.c:d40_xlate() {
...
flags = dma_spec->args[2];
...
}
If a device tree incorrectly specifies a dma-cells value less than 3, the
args array would only be partially initialized by of_parse_phandle_with_args().
Could unconditionally accessing args[2] without verifying args_count lead to
reading uninitialized stack data?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260820-dma40-fixes-v2-0-63238334c707@kernel.org?part=9
next prev parent reply other threads:[~2026-08-20 13:29 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-20 13:14 [PATCH v2 00/13] dmaengine: ste_dma40: Fix probe and allocation bugs Linus Walleij
2026-08-20 13:14 ` [PATCH v2 01/13] dmaengine: ste_dma40: Fix failed start cleanup Linus Walleij
2026-08-20 13:30 ` sashiko-bot
2026-08-20 13:14 ` [PATCH v2 02/13] dmaengine: ste_dma40: Check runtime PM in IRQ Linus Walleij
2026-08-20 13:31 ` sashiko-bot
2026-08-20 13:14 ` [PATCH v2 03/13] dmaengine: ste_dma40: Init hardware before registration Linus Walleij
2026-08-20 13:14 ` [PATCH v2 04/13] dmaengine: ste_dma40: Fix DMA registration unwind Linus Walleij
2026-08-20 13:30 ` sashiko-bot
2026-08-20 13:14 ` [PATCH v2 05/13] dmaengine: ste_dma40: Fix LCLA allocation order Linus Walleij
2026-08-20 13:29 ` sashiko-bot
2026-08-20 13:15 ` [PATCH v2 06/13] dmaengine: ste_dma40: Fix probe LCLA free Linus Walleij
2026-08-20 13:15 ` [PATCH v2 07/13] dmaengine: ste_dma40: Fix probe runtime PM disable Linus Walleij
2026-08-20 13:29 ` sashiko-bot
2026-08-20 13:15 ` [PATCH v2 08/13] dmaengine: ste_dma40: Fix probe IRQ leak Linus Walleij
2026-08-20 13:29 ` sashiko-bot
2026-08-20 13:15 ` [PATCH v2 09/13] dmaengine: ste_dma40: Fix memcpy channel parsing Linus Walleij
2026-08-20 13:29 ` sashiko-bot [this message]
2026-08-20 13:15 ` [PATCH v2 10/13] dmaengine: ste_dma40: Fix logical channel bounds check Linus Walleij
2026-08-20 13:32 ` sashiko-bot
2026-08-20 13:15 ` [PATCH v2 11/13] dmaengine: ste_dma40: Fix event group bounds Linus Walleij
2026-08-20 13:34 ` sashiko-bot
2026-08-20 13:15 ` [PATCH v2 12/13] dmaengine: ste_dma40: Validate memcpy configuration Linus Walleij
2026-08-20 13:37 ` sashiko-bot
2026-08-20 13:15 ` [PATCH v2 13/13] dmaengine: Use unique debugfs names Linus Walleij
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=20260820132947.C10EC1F00A3A@smtp.kernel.org \
--to=sashiko-bot@kernel.org \
--cc=Frank.Li@kernel.org \
--cc=dmaengine@vger.kernel.org \
--cc=linusw@kernel.org \
--cc=sashiko-reviews@lists.linux.dev \
--cc=vkoul@kernel.org \
/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