linux-kernel - Re: [PATCH v1] dmaengine: ti: Add NULL check in udma

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <87fri6lame.fsf@AUSNATLYNCH.amd.com>
Date: Thu, 17 Apr 2025 16:51:53 -0500
From: Nathan Lynch <nathan.lynch@....com>
To: Henry Martin <bsdhenrymartin@...il.com>, <peter.ujfalusi@...il.com>,
	<vkoul@...nel.org>
CC: <dmaengine@...r.kernel.org>, <linux-kernel@...r.kernel.org>, Henry Martin
	<bsdhenrymartin@...il.com>
Subject: Re: [PATCH v1] dmaengine: ti: Add NULL check in udma_probe()

Henry Martin <bsdhenrymartin@...il.com> writes:
> devm_kasprintf() returns NULL when memory allocation fails. Currently,
> udma_probe() does not check for this case, which results in a NULL
> pointer dereference.

Yes, it does look like this would happen when uc->name is eventually
passed to dma_pool_create(), at least.

>
> Add NULL check after devm_kasprintf() to prevent this issue.
>
> Fixes: 25dcb5dd7b7c ("dmaengine: ti: New driver for K3 UDMA")
> Signed-off-by: Henry Martin <bsdhenrymartin@...il.com>
> ---
>  drivers/dma/ti/k3-udma.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/dma/ti/k3-udma.c b/drivers/dma/ti/k3-udma.c
> index 7ed1956b4642..f1c2f8170730 100644
> --- a/drivers/dma/ti/k3-udma.c
> +++ b/drivers/dma/ti/k3-udma.c
> @@ -5582,7 +5582,8 @@ static int udma_probe(struct platform_device *pdev)
>  		uc->config.dir = DMA_MEM_TO_MEM;
>  		uc->name = devm_kasprintf(dev, GFP_KERNEL, "%s chan%d",
>  					  dev_name(dev), i);
> -
> +		if (!uc->name)
> +			return -ENOMEM;
>  		vchan_init(&uc->vc, &ud->ddev);
>  		/* Use custom vchan completion handling */
>  		tasklet_setup(&uc->vc.task, udma_vchan_complete);

Returning -ENOMEM directly seems fine, even though this is in a loop
200+ lines into udma_probe(). I don't see any unmanaged device resources
that need to be released before returning, and if I missed one, all the
error paths this code precedes would have the same problem.

Reviewed-by: Nathan Lynch <nathan.lynch@....com>