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
| ||
|
Message-ID: <85a76ca4-6cad-e533-ff7b-64a660d1a2a0@st.com> Date: Tue, 23 Apr 2019 17:18:58 +0200 From: Arnaud Pouliquen <arnaud.pouliquen@...com> To: Vinod Koul <vkoul@...nel.org> CC: Dan Williams <dan.j.williams@...el.com>, Pierre-Yves MORDRET <pierre-yves.mordret@...com>, <linux-stm32@...md-mailman.stormreply.com>, <linux-kernel@...r.kernel.org>, <dmaengine@...r.kernel.org> Subject: Re: [PATCH] dmaengine: stm32-dma: fix residue calculation in stm32-dma Hello Vinod, Just a gentle reminder, if you could take a moment to review this patch. FYI, the patch has already been internally reviewed by Pierre Yves mordret... His sign-off is in the commit message. Thanks, Arnaud On 3/27/19 1:21 PM, Arnaud Pouliquen wrote: > During residue calculation. the DMA can switch to the next sg. When > this race condition occurs, the residue returned value is not valid. > Indeed the position in the sg returned by the hardware is the position > of the next sg, not the current sg. > Solution is to check the sg after the calculation to verify it. > If a transition is detected we consider that the DMA has switched to > the beginning of next sg. > > Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@...com> > Signed-off-by: Pierre-Yves MORDRET <pierre-yves.mordret@...com> > --- > drivers/dma/stm32-dma.c | 70 ++++++++++++++++++++++++++++++++++++++++--------- > 1 file changed, 57 insertions(+), 13 deletions(-) > > diff --git a/drivers/dma/stm32-dma.c b/drivers/dma/stm32-dma.c > index 4903a40..30309d2 100644 > --- a/drivers/dma/stm32-dma.c > +++ b/drivers/dma/stm32-dma.c > @@ -1038,33 +1038,77 @@ static u32 stm32_dma_get_remaining_bytes(struct stm32_dma_chan *chan) > return ndtr << width; > } > > +static bool stm32_dma_is_current_sg(struct stm32_dma_chan *chan) > +{ > + struct stm32_dma_device *dmadev = stm32_dma_get_dev(chan); > + struct stm32_dma_sg_req *sg_req; > + u32 dma_scr, dma_smar, id; > + > + id = chan->id; > + dma_scr = stm32_dma_read(dmadev, STM32_DMA_SCR(id)); > + > + if (!(dma_scr & STM32_DMA_SCR_DBM)) > + return true; > + > + sg_req = &chan->desc->sg_req[chan->next_sg]; > + > + if (dma_scr & STM32_DMA_SCR_CT) { > + dma_smar = stm32_dma_read(dmadev, STM32_DMA_SM0AR(id)); > + return (dma_smar == sg_req->chan_reg.dma_sm0ar); > + } > + > + dma_smar = stm32_dma_read(dmadev, STM32_DMA_SM1AR(id)); > + > + return (dma_smar == sg_req->chan_reg.dma_sm1ar); > +} > + > static size_t stm32_dma_desc_residue(struct stm32_dma_chan *chan, > struct stm32_dma_desc *desc, > u32 next_sg) > { > u32 modulo, burst_size; > - u32 residue = 0; > + u32 residue; > + u32 n_sg = next_sg; > + struct stm32_dma_sg_req *sg_req = &chan->desc->sg_req[chan->next_sg]; > int i; > > + residue = stm32_dma_get_remaining_bytes(chan); > + > /* > - * In cyclic mode, for the last period, residue = remaining bytes from > - * NDTR > + * Calculate the residue means compute the descriptors > + * information: > + * - the sg currently transferred > + * - the remaining position in this sg (NDTR). > + * > + * The issue is that a race condition can occur if DMA is > + * running. DMA can have started to transfer the next sg before > + * the position in sg is read. In this case the remaing position > + * can correspond to the new sg position. > + * The strategy implemented in the stm32 driver is to check the > + * sg transition. If detected we can not trust the SxNDTR register > + * value, this register can not be up to date during the transition. > + * In this case we can assume that the dma is at the beginning of next > + * sg so we calculate the residue in consequence. > */ > - if (chan->desc->cyclic && next_sg == 0) { > - residue = stm32_dma_get_remaining_bytes(chan); > - goto end; > + > + if (!stm32_dma_is_current_sg(chan)) { > + n_sg++; > + if (n_sg == chan->desc->num_sgs) > + n_sg = 0; > + residue = sg_req->len; > } > > /* > - * For all other periods in cyclic mode, and in sg mode, > - * residue = remaining bytes from NDTR + remaining periods/sg to be > - * transferred > + * In cyclic mode, for the last period, residue = remaining bytes > + * from NDTR, > + * else for all other periods in cyclic mode, and in sg mode, > + * residue = remaining bytes from NDTR + remaining > + * periods/sg to be transferred > */ > - for (i = next_sg; i < desc->num_sgs; i++) > - residue += desc->sg_req[i].len; > - residue += stm32_dma_get_remaining_bytes(chan); > + if (!chan->desc->cyclic || n_sg != 0) > + for (i = n_sg; i < desc->num_sgs; i++) > + residue += desc->sg_req[i].len; > > -end: > if (!chan->mem_burst) > return residue; > >
Powered by blists - more mailing lists