linux-kernel - Re: [PATCH] dmaengine: ti: k3-udma: Add missing locking

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: <dc9b9b53-dbca-49f5-a7e6-3a6a3112f1bb@gmail.com>
Date: Wed, 22 Jan 2025 17:22:55 +0200
From: Péter Ujfalusi <peter.ujfalusi@...il.com>
To: Ronald Wahl <rwahl@....de>, linux-kernel@...r.kernel.org
Cc: Ronald Wahl <ronald.wahl@...rand.com>,
 Vignesh Raghavendra <vigneshr@...com>, Vinod Koul <vkoul@...nel.org>,
 dmaengine@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH] dmaengine: ti: k3-udma: Add missing locking



On 20/01/2025 16:41, Ronald Wahl wrote:
> From: Ronald Wahl <ronald.wahl@...rand.com>
> 
> Recent kernels complain about a missing lock in k3-udma.c when the lock
> validator is enabled:
> 
> [    4.128073] WARNING: CPU: 0 PID: 746 at drivers/dma/ti/../virt-dma.h:169 udma_start.isra.0+0x34/0x238
> [    4.137352] CPU: 0 UID: 0 PID: 746 Comm: kworker/0:3 Not tainted 6.12.9-arm64 #28
> [    4.144867] Hardware name: pp-v12 (DT)
> [    4.148648] Workqueue: events udma_check_tx_completion
> [    4.153841] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> [    4.160834] pc : udma_start.isra.0+0x34/0x238
> [    4.165227] lr : udma_start.isra.0+0x30/0x238
> [    4.169618] sp : ffffffc083cabcf0
> [    4.172963] x29: ffffffc083cabcf0 x28: 0000000000000000 x27: ffffff800001b005
> [    4.180167] x26: ffffffc0812f0000 x25: 0000000000000000 x24: 0000000000000000
> [    4.187370] x23: 0000000000000001 x22: 00000000e21eabe9 x21: ffffff8000fa0670
> [    4.194571] x20: ffffff8001b6bf00 x19: ffffff8000fa0430 x18: ffffffc083b95030
> [    4.201773] x17: 0000000000000000 x16: 00000000f0000000 x15: 0000000000000048
> [    4.208976] x14: 0000000000000048 x13: 0000000000000000 x12: 0000000000000001
> [    4.216179] x11: ffffffc08151a240 x10: 0000000000003ea1 x9 : ffffffc08046ab68
> [    4.223381] x8 : ffffffc083cabac0 x7 : ffffffc081df3718 x6 : 0000000000029fc8
> [    4.230583] x5 : ffffffc0817ee6d8 x4 : 0000000000000bc0 x3 : 0000000000000000
> [    4.237784] x2 : 0000000000000000 x1 : 00000000001fffff x0 : 0000000000000000
> [    4.244986] Call trace:
> [    4.247463]  udma_start.isra.0+0x34/0x238
> [    4.251509]  udma_check_tx_completion+0xd0/0xdc
> [    4.256076]  process_one_work+0x244/0x3fc
> [    4.260129]  process_scheduled_works+0x6c/0x74
> [    4.264610]  worker_thread+0x150/0x1dc
> [    4.268398]  kthread+0xd8/0xe8
> [    4.271492]  ret_from_fork+0x10/0x20
> [    4.275107] irq event stamp: 220
> [    4.278363] hardirqs last  enabled at (219): [<ffffffc080a27c7c>] _raw_spin_unlock_irq+0x38/0x50
> [    4.287183] hardirqs last disabled at (220): [<ffffffc080a1c154>] el1_dbg+0x24/0x50
> [    4.294879] softirqs last  enabled at (182): [<ffffffc080037e68>] handle_softirqs+0x1c0/0x3cc
> [    4.303437] softirqs last disabled at (177): [<ffffffc080010170>] __do_softirq+0x1c/0x28
> [    4.311559] ---[ end trace 0000000000000000 ]---
> 
> This commit adds the missing locking.

Acked-by: Peter Ujfalusi <peter.ujfalusi@...il.com>

> 
> Fixes: 25dcb5dd7b7c ("dmaengine: ti: New driver for K3 UDMA")
> Cc: Peter Ujfalusi <peter.ujfalusi@...il.com>
> Cc: Vignesh Raghavendra <vigneshr@...com>
> Cc: Vinod Koul <vkoul@...nel.org>
> Cc: dmaengine@...r.kernel.org
> Cc: stable@...r.kernel.org
> Signed-off-by: Ronald Wahl <ronald.wahl@...rand.com>
> ---
>  drivers/dma/ti/k3-udma.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/dma/ti/k3-udma.c b/drivers/dma/ti/k3-udma.c
> index b3f27b3f9209..b9e497e8134b 100644
> --- a/drivers/dma/ti/k3-udma.c
> +++ b/drivers/dma/ti/k3-udma.c
> @@ -1091,8 +1091,11 @@ static void udma_check_tx_completion(struct work_struct *work)
>  	u32 residue_diff;
>  	ktime_t time_diff;
>  	unsigned long delay;
> +	unsigned long flags;
> 
>  	while (1) {
> +		spin_lock_irqsave(&uc->vc.lock, flags);
> +
>  		if (uc->desc) {
>  			/* Get previous residue and time stamp */
>  			residue_diff = uc->tx_drain.residue;
> @@ -1127,6 +1130,8 @@ static void udma_check_tx_completion(struct work_struct *work)
>  				break;
>  			}
> 
> +			spin_unlock_irqrestore(&uc->vc.lock, flags);
> +
>  			usleep_range(ktime_to_us(delay),
>  				     ktime_to_us(delay) + 10);
>  			continue;
> @@ -1143,6 +1148,8 @@ static void udma_check_tx_completion(struct work_struct *work)
> 
>  		break;
>  	}
> +
> +	spin_unlock_irqrestore(&uc->vc.lock, flags);
>  }
> 
>  static irqreturn_t udma_ring_irq_handler(int irq, void *data)
> --
> 2.48.0
> 

-- 
Péter