Skip to contentSkip to conversation

dmaengine: validate dev and name in dma_request_chan()

PATCH Applied [email protected] 4 participants 4 messages Jul 16 – Jul 17 dmaengine

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 dmaengine: validate dev and name in dma_request_chan()

From: bui duc phuc <[email protected]>

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.

---
 drivers/dma/dmaengine.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
drivers/dma/dmaengine.c +6 -1
index 9049171df857..7b479cd93b9b 100644
@@ -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))
Signature · 1 line
2.43.0
#1 lore raw
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

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 <[email protected]>

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.

Quoted 3 lines
drivers/dma/dmaengine.c +0 -0

[ ... ]

Quoted 4 lines
drivers/dma/dmaengine.c +0 -0
@@ -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)
 {

[ ... ]

Quoted 10 lines

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?

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
Reviewed-by on v1 1/1
#2 lore raw
In reply to [email protected]
Quoted 10 lines · On Thu, Jul 16, 2026 at 12:27:58PM +0700, [email protected] wrote:

From: bui duc phuc <[email protected]>

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.

Quoted 8 lines

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.

---

Quoted 27 lines
 drivers/dma/dmaengine.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
drivers/dma/dmaengine.c +6 -2
index 9049171df857..7b479cd93b9b 100644
@@ -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

maintainer Applied
#3 lore raw
In reply to [email protected]
Quoted 9 lines · On Thu, 16 Jul 2026 12:27:58 +0700, [email protected] wrote:

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.

[...]

Applied, thanks!

[1/1] dmaengine: validate dev and name in dma_request_chan()

      commit: dc8a5238e1c7db74a7b4f02bcf0b05559b6627c5

Best regards,

Signature · 1 line
~Vinod