linux-hardening - Re: [PATCH 01/11] dmaengine: dw-edma: Add spinlock to protect DONE_INT_MASK and ABORT_INT

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <u5morpcx47pgyg7emt6yhhyivevwtbgvp7xme4uf6ssrcvew2n@yznzt7mj4ns3>
Date: Tue, 30 Dec 2025 22:00:49 +0530
From: Manivannan Sadhasivam <mani@...nel.org>
To: Frank Li <Frank.Li@....com>
Cc: Vinod Koul <vkoul@...nel.org>, 
	Gustavo Pimentel <Gustavo.Pimentel@...opsys.com>, Kees Cook <kees@...nel.org>, 
	"Gustavo A. R. Silva" <gustavoars@...nel.org>, Krzysztof Wilczyński <kwilczynski@...nel.org>, 
	Kishon Vijay Abraham I <kishon@...nel.org>, Bjorn Helgaas <bhelgaas@...gle.com>, 
	Christoph Hellwig <hch@....de>, Niklas Cassel <cassel@...nel.org>, dmaengine@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-hardening@...r.kernel.org, linux-pci@...r.kernel.org, 
	linux-nvme@...ts.infradead.org, imx@...ts.linux.dev
Subject: Re: [PATCH 01/11] dmaengine: dw-edma: Add spinlock to protect
 DONE_INT_MASK and ABORT_INT_MASK

On Fri, Dec 12, 2025 at 05:24:40PM -0500, Frank Li wrote:
> The DONE_INT_MASK and ABORT_INT_MASK registers are shared by all DMA
> channels, and modifying them requires a read-modify-write sequence.
> Because this operation is not atomic, concurrent calls to
> dw_edma_v0_core_start() can introduce race conditions if two channels
> update these registers simultaneously.
> 
> Add a spinlock to serialize access to these registers and prevent race
> conditions.
> 

dw_edma_v0_core_start() is called by dw_edma_start_transfer() in 3 places, and
protected by 'chan->vc.lock' in 2 places. Only in dw_edma_device_resume(), it is
not protected. Don't you need to protect it instead?

- Mani

> Signed-off-by: Frank Li <Frank.Li@....com>
> ---
>  drivers/dma/dw-edma/dw-edma-v0-core.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c
> index b75fdaffad9a4ea6cd8d15e8f43bea550848b46c..2850a9df80f54d00789144415ed2dfe31dea3965 100644
> --- a/drivers/dma/dw-edma/dw-edma-v0-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-v0-core.c
> @@ -364,6 +364,7 @@ static void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
>  {
>  	struct dw_edma_chan *chan = chunk->chan;
>  	struct dw_edma *dw = chan->dw;
> +	unsigned long flags;
>  	u32 tmp;
>  
>  	dw_edma_v0_core_write_chunk(chunk);
> @@ -408,6 +409,8 @@ static void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
>  			}
>  		}
>  		/* Interrupt unmask - done, abort */
> +		raw_spin_lock_irqsave(&dw->lock, flags);
> +
>  		tmp = GET_RW_32(dw, chan->dir, int_mask);
>  		tmp &= ~FIELD_PREP(EDMA_V0_DONE_INT_MASK, BIT(chan->id));
>  		tmp &= ~FIELD_PREP(EDMA_V0_ABORT_INT_MASK, BIT(chan->id));
> @@ -416,6 +419,9 @@ static void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
>  		tmp = GET_RW_32(dw, chan->dir, linked_list_err_en);
>  		tmp |= FIELD_PREP(EDMA_V0_LINKED_LIST_ERR_MASK, BIT(chan->id));
>  		SET_RW_32(dw, chan->dir, linked_list_err_en, tmp);
> +
> +		raw_spin_unlock_irqrestore(&dw->lock, flags);
> +
>  		/* Channel control */
>  		SET_CH_32(dw, chan->dir, chan->id, ch_control1,
>  			  (DW_EDMA_V0_CCS | DW_EDMA_V0_LLE));
> 
> -- 
> 2.34.1
> 

-- 
மணிவண்ணன் சதாசிவம்