* [PATCH] dmaengine: validate dev and name in dma_request_chan()
@ 2026-07-16 5:27 phucduc.bui
2026-07-16 5:37 ` sashiko-bot
2026-07-16 14:06 ` Frank Li
0 siblings, 2 replies; 3+ messages in thread
From: phucduc.bui @ 2026-07-16 5:27 UTC (permalink / raw)
To: Vinod Koul, Frank Li, dmaengine, linux-kernel; +Cc: bui duc phuc
From: bui duc phuc <phucduc.bui@gmail.com>
dma_request_chan() assumes both @dev and @name are valid, but neither is
checked before use.
dev is dereferenced immediately via dev_fwnode(), which accesses
dev->of_node or dev->fwnode without checking for NULL. Likewise, if name is
NULL and the OF/ACPI lookup does not succeed, the legacy filter-map path
eventually passes it to strcmp(), resulting in a NULL pointer dereference.
These are caller bugs rather than normal lookup failures, so add a
WARN_ON() at function entry to catch invalid arguments early during
development instead of crashing later.
No functional change for valid callers.
Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
drivers/dma/dmaengine.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
index 9049171df857..7b479cd93b9b 100644
--- a/drivers/dma/dmaengine.c
+++ b/drivers/dma/dmaengine.c
@@ -814,10 +814,15 @@ static const struct dma_slave_map *dma_filter_match(struct dma_device *device,
*/
struct dma_chan *dma_request_chan(struct device *dev, const char *name)
{
- struct fwnode_handle *fwnode = dev_fwnode(dev);
+ struct fwnode_handle *fwnode;
struct dma_device *d, *_d;
struct dma_chan *chan = NULL;
+ if (WARN_ON(!dev || !name))
+ return ERR_PTR(-EINVAL);
+
+ fwnode = dev_fwnode(dev);
+
if (is_of_node(fwnode))
chan = of_dma_request_slave_channel(to_of_node(fwnode), name);
else if (is_acpi_device_node(fwnode))
--
2.43.0
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] dmaengine: validate dev and name in dma_request_chan()
2026-07-16 5:27 [PATCH] dmaengine: validate dev and name in dma_request_chan() phucduc.bui
@ 2026-07-16 5:37 ` sashiko-bot
2026-07-16 14:06 ` Frank Li
1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-07-16 5:37 UTC (permalink / raw)
To: phucduc.bui; +Cc: vkoul, dmaengine, Frank.Li
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but if `kasprintf` fails to allocate `chan->name`, the function returns the channel but leaves `chan->slave` uninitialized.
--
--- Patch [1]: [PATCH] dmaengine: validate dev and name in dma_request_chan() ---
commit ac1213130dc39a9098faa262ed0755d63436e5be
Author: bui duc phuc <phucduc.bui@gmail.com>
dmaengine: validate dev and name in dma_request_chan()
This commit adds a WARN_ON check for NULL dev and name pointers at the
start of dma_request_chan(). It helps catch invalid caller arguments early
instead of causing NULL pointer dereferences later during lookup.
> diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
> --- a/drivers/dma/dmaengine.c
> +++ b/drivers/dma/dmaengine.c
[ ... ]
> @@ -814,10 +814,15 @@ static const struct dma_slave_map *dma_filter_match(struct dma_device *device,
> */
> struct dma_chan *dma_request_chan(struct device *dev, const char *name)
> {
[ ... ]
> found:
> #ifdef CONFIG_DEBUG_FS
> chan->dbg_client_name = kasprintf(GFP_KERNEL, "%s:%s", dev_name(dev), name);
> /* No functional issue if it fails, users are supposed to test before use */
> #endif
>
> chan->name = kasprintf(GFP_KERNEL, "dma:%s", name);
> if (!chan->name)
> return chan;
> chan->slave = dev;
[Severity: High]
This isn't a bug introduced by this patch, but is it intended to return the
channel with an uninitialized slave pointer here?
If kasprintf fails under memory pressure, the function returns the channel
early:
dma_request_chan() {
...
chan->name = kasprintf(GFP_KERNEL, "dma:%s", name);
if (!chan->name)
return chan;
...
}
This leaves chan->slave uninitialized. If DMA provider drivers rely on
chan->slave pointing to the client device, could this cause a NULL pointer
dereference or memory corruption when accessed later?
If returning the channel is intended upon name allocation failure, should
chan->slave be initialized first?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260716052758.23465-1-phucduc.bui@gmail.com?part=1
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] dmaengine: validate dev and name in dma_request_chan()
2026-07-16 5:27 [PATCH] dmaengine: validate dev and name in dma_request_chan() phucduc.bui
2026-07-16 5:37 ` sashiko-bot
@ 2026-07-16 14:06 ` Frank Li
1 sibling, 0 replies; 3+ messages in thread
From: Frank Li @ 2026-07-16 14:06 UTC (permalink / raw)
To: phucduc.bui; +Cc: Vinod Koul, Frank Li, dmaengine, linux-kernel
On Thu, Jul 16, 2026 at 12:27:58PM +0700, phucduc.bui@gmail.com wrote:
> From: bui duc phuc <phucduc.bui@gmail.com>
>
> dma_request_chan() assumes both @dev and @name are valid, but neither is
> checked before use.
>
> dev is dereferenced immediately via dev_fwnode(), which accesses
> dev->of_node or dev->fwnode without checking for NULL. Likewise, if name is
> NULL and the OF/ACPI lookup does not succeed, the legacy filter-map path
> eventually passes it to strcmp(), resulting in a NULL pointer dereference.
>
> These are caller bugs rather than normal lookup failures, so add a
> WARN_ON() at function entry to catch invalid arguments early during
> development instead of crashing later.
>
> No functional change for valid callers.
>
> Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
> ---
Reviewed-by: Frank Li <Frank.Li@nxp.com>
> drivers/dma/dmaengine.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
> index 9049171df857..7b479cd93b9b 100644
> --- a/drivers/dma/dmaengine.c
> +++ b/drivers/dma/dmaengine.c
> @@ -814,10 +814,15 @@ static const struct dma_slave_map *dma_filter_match(struct dma_device *device,
> */
> struct dma_chan *dma_request_chan(struct device *dev, const char *name)
> {
> - struct fwnode_handle *fwnode = dev_fwnode(dev);
> + struct fwnode_handle *fwnode;
> struct dma_device *d, *_d;
> struct dma_chan *chan = NULL;
>
> + if (WARN_ON(!dev || !name))
> + return ERR_PTR(-EINVAL);
> +
> + fwnode = dev_fwnode(dev);
> +
> if (is_of_node(fwnode))
> chan = of_dma_request_slave_channel(to_of_node(fwnode), name);
> else if (is_acpi_device_node(fwnode))
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-07-16 14:06 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-16 5:27 [PATCH] dmaengine: validate dev and name in dma_request_chan() phucduc.bui
2026-07-16 5:37 ` sashiko-bot
2026-07-16 14:06 ` Frank Li
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.