linux-kernel - Re: [PATCH 5/9] dmaengine: dw-edma: HDMA: Fix possible race condition in remote setup

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]

Date:   Mon, 19 Jun 2023 20:41:05 +0200
From:   Köry Maincent <kory.maincent@...tlin.com>
To:     Serge Semin <fancer.lancer@...il.com>
Cc:     Cai Huoqing <cai.huoqing@...ux.dev>,
        Manivannan Sadhasivam <mani@...nel.org>,
        Vinod Koul <vkoul@...nel.org>,
        Gustavo Pimentel <Gustavo.Pimentel@...opsys.com>,
        dmaengine@...r.kernel.org, linux-kernel@...r.kernel.org,
        Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
        Herve Codina <herve.codina@...tlin.com>
Subject: Re: [PATCH 5/9] dmaengine: dw-edma: HDMA: Fix possible race
 condition in remote setup

On Mon, 19 Jun 2023 20:15:50 +0300
Serge Semin <fancer.lancer@...il.com> wrote:

> On Fri, Jun 09, 2023 at 10:16:50AM +0200, Köry Maincent wrote:
> > From: Kory Maincent <kory.maincent@...tlin.com>
> > 
> > When writing the linked list elements and pointer the control need to be
> > written at the end. If the control is written and the SAR and DAR not
> > stored we could face a race condition.
> > 
> > Fixes: e74c39573d35 ("dmaengine: dw-edma: Add support for native HDMA")
> > Signed-off-by: Kory Maincent <kory.maincent@...tlin.com>  
> 
> Once again. Is this a hypothetical bug or have you actually
> experienced the denoted problem? If you do please describe the
> circumstances, give more details. Otherwise it sounds weird. Here is
> why.
> 
> DW eDMA HW manual states that the control LL DWORD is indeed supposed
> to be written after the rest of the descriptor DWORDs are written. But
> AFAICS it's only relevant for the LL tree entries recycling. Current
> DW eDMA driver design doesn't truly implement that pattern. Instead
> the DMA transfer is halted at the end of the chunk. Then the engine is
> recharged with the next chunk and execution is started over. So the
> runtime recycling isn't implemented (alas) for which the CB flag of
> the control DWORD needs to be updated only after the rest of the LLI
> fields.

This one is only hypothetical. It appears to me that writing the control
after the configuration of sar and dar is more relevant to prevent race issues
and should be the usual coding choice. Also you are right saying that it will
be relevant only for the LL tree entries recycling.
Simple question from non DMA expert: isn't cyclic DMA mode recycle the LL tree
entries? 

> 
> If you described a hypothetical problem then it would be ok to accept
> the change for the sake of consistency but I would have dropped the
> Fixes tag and updated the patch description with more details of the
> race condition you are talking about.

Alright, I will do that.

Köry