| 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
| ||
|
Date: Sat, 29 Jun 2024 10:00:34 +0800
From: Jie Hai <haijie1@...wei.com>
To: Frank Li <Frank.li@....com>
CC: <vkoul@...nel.org>, Paul Walmsley <paul.walmsley@...ive.com>, Samuel
Holland <samuel.holland@...ive.com>, Li Zetao <lizetao1@...wei.com>, Guanhua
Gao <guanhua.gao@....com>, "open list:DMA GENERIC OFFLOAD ENGINE SUBSYSTEM"
<dmaengine@...r.kernel.org>, open list <linux-kernel@...r.kernel.org>, "open
list:FREESCALE eDMA DRIVER" <imx@...ts.linux.dev>, "open list:SIFIVE DRIVERS"
<linux-riscv@...ts.infradead.org>
Subject: Re: [PATCH v2] dmaegine: virt-dma : Fix multi-user with vchan
On 2024/6/20 22:38, Frank Li wrote:
> On Thu, Jun 20, 2024 at 10:53:53AM +0800, Jie Hai wrote:
>> List desc_allocated was introduced for the case of a transfer
>> submitted multiple times. But elegating descriptors on the list
>> causes other problems.
>>
>> For example, in the multi-thread scenario, which tasks are
>> continuously created and submitted by each thread. If one of
>> the threads calls dmaengine_terminate_all, for dirvers using
>> vchan_get_all_descriptors, all descriptors will be freed. If
>> there's another thread submitting a transfer A by
>> vchan_tx_submit, the following results may be generated:
>> 1. desc A is freeing -> visit wrong address of node prep/next.
>> 2. desc A is freed -> visit invalid address of A.
>>
>> In the above case, calltrace is generated and the system is
>> suspended. This can be tested by dmatest.
>
> What's test steps to reproduce this problem?
>
> Frank
Thanks for your review.
The operations are as follows:
modprobe hisi_dma
modprobe dmatest
echo 0 > /sys/module/dmatest/parameters/iterations
echo "dma0chan0" > /sys/module/dmatest/parameters/channel
echo 20 > /sys/module/dmatest/parameters/threads_per_chan
echo 1 > /sys/module/dmatest/parameters/run
wait for a while and stop the test by:
echo 0 > /sys/module/dmatest/parameters/run
>>
>> This patch removes desc_allocated from vchan_get_all_descriptors,
>> and add new function 'vchan_get_all_allocated_descs' to get all
>> descriptors ever allocated.
>>
>> And apply vchan_get_all_allocated_descs to free chan resource and
>> vchan_get_all_descriptors to terminate all transfers, respectively.
>> This avoids freeing up descriptors in use by other threads.
>>
>> Signed-off-by: Jie Hai <haijie1@...wei.com>
>> ---
>> drivers/dma/fsl-dpaa2-qdma/dpaa2-qdma.c | 2 +-
>> drivers/dma/fsl-edma-common.c | 2 +-
>> drivers/dma/fsl-qdma.c | 2 +-
>> drivers/dma/sf-pdma/sf-pdma.c | 2 +-
>> drivers/dma/virt-dma.h | 20 ++++++++++++++++++--
>> 5 files changed, 22 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/dma/fsl-dpaa2-qdma/dpaa2-qdma.c b/drivers/dma/fsl-dpaa2-qdma/dpaa2-qdma.c
>> index 36384d019263..efdecf15e1b3 100644
>> --- a/drivers/dma/fsl-dpaa2-qdma/dpaa2-qdma.c
>> +++ b/drivers/dma/fsl-dpaa2-qdma/dpaa2-qdma.c
>> @@ -71,7 +71,7 @@ static void dpaa2_qdma_free_chan_resources(struct dma_chan *chan)
>> LIST_HEAD(head);
>>
>> spin_lock_irqsave(&dpaa2_chan->vchan.lock, flags);
>> - vchan_get_all_descriptors(&dpaa2_chan->vchan, &head);
>> + vchan_get_all_allocated_descs(&dpaa2_chan->vchan, &head);
>> spin_unlock_irqrestore(&dpaa2_chan->vchan.lock, flags);
>>
>> vchan_dma_desc_free_list(&dpaa2_chan->vchan, &head);
>> diff --git a/drivers/dma/fsl-edma-common.c b/drivers/dma/fsl-edma-common.c
>> index 3af430787315..1e0ad87eb7fa 100644
>> --- a/drivers/dma/fsl-edma-common.c
>> +++ b/drivers/dma/fsl-edma-common.c
>> @@ -828,7 +828,7 @@ void fsl_edma_free_chan_resources(struct dma_chan *chan)
>> if (edma->drvdata->dmamuxs)
>> fsl_edma_chan_mux(fsl_chan, 0, false);
>> fsl_chan->edesc = NULL;
>> - vchan_get_all_descriptors(&fsl_chan->vchan, &head);
>> + vchan_get_all_allocated_descs(&fsl_chan->vchan, &head);
>> fsl_edma_unprep_slave_dma(fsl_chan);
>> spin_unlock_irqrestore(&fsl_chan->vchan.lock, flags);
>>
>> diff --git a/drivers/dma/fsl-qdma.c b/drivers/dma/fsl-qdma.c
>> index 5005e138fc23..7af428db404e 100644
>> --- a/drivers/dma/fsl-qdma.c
>> +++ b/drivers/dma/fsl-qdma.c
>> @@ -316,7 +316,7 @@ static void fsl_qdma_free_chan_resources(struct dma_chan *chan)
>> LIST_HEAD(head);
>>
>> spin_lock_irqsave(&fsl_chan->vchan.lock, flags);
>> - vchan_get_all_descriptors(&fsl_chan->vchan, &head);
>> + vchan_get_all_allocated_descs(&fsl_chan->vchan, &head);
>> spin_unlock_irqrestore(&fsl_chan->vchan.lock, flags);
>>
>> vchan_dma_desc_free_list(&fsl_chan->vchan, &head);
>> diff --git a/drivers/dma/sf-pdma/sf-pdma.c b/drivers/dma/sf-pdma/sf-pdma.c
>> index 428473611115..4dc8a8c8ad80 100644
>> --- a/drivers/dma/sf-pdma/sf-pdma.c
>> +++ b/drivers/dma/sf-pdma/sf-pdma.c
>> @@ -147,7 +147,7 @@ static void sf_pdma_free_chan_resources(struct dma_chan *dchan)
>> sf_pdma_disable_request(chan);
>> kfree(chan->desc);
>> chan->desc = NULL;
>> - vchan_get_all_descriptors(&chan->vchan, &head);
>> + vchan_get_all_allocated_descs(&chan->vchan, &head);
>> sf_pdma_disclaim_chan(chan);
>> spin_unlock_irqrestore(&chan->vchan.lock, flags);
>> vchan_dma_desc_free_list(&chan->vchan, &head);
>> diff --git a/drivers/dma/virt-dma.h b/drivers/dma/virt-dma.h
>> index 59d9eabc8b67..4492641b79f6 100644
>> --- a/drivers/dma/virt-dma.h
>> +++ b/drivers/dma/virt-dma.h
>> @@ -187,13 +187,29 @@ static inline void vchan_get_all_descriptors(struct virt_dma_chan *vc,
>> {
>> lockdep_assert_held(&vc->lock);
>>
>> - list_splice_tail_init(&vc->desc_allocated, head);
>> list_splice_tail_init(&vc->desc_submitted, head);
>> list_splice_tail_init(&vc->desc_issued, head);
>> list_splice_tail_init(&vc->desc_completed, head);
>> list_splice_tail_init(&vc->desc_terminated, head);
>> }
>>
>> +/**
>> + * vchan_get_all_allocated_descs - obtain all descriptors
>> + * @vc: virtual channel to get descriptors from
>> + * @head: list of descriptors found
>> + *
>> + * vc.lock must be held by caller
>> + *
>> + * Removes all descriptors from internal lists, and provides a list of all
>> + * descriptors found
>> + */
>> +static inline void vchan_get_all_allocated_descs(struct virt_dma_chan *vc,
>> + struct list_head *head)
>> +{
>> + list_splice_tail_init(&vc->desc_allocated, head);
>> + vchan_get_all_descriptors(vc, head);
>> +}
>> +
>> static inline void vchan_free_chan_resources(struct virt_dma_chan *vc)
>> {
>> struct virt_dma_desc *vd;
>> @@ -201,7 +217,7 @@ static inline void vchan_free_chan_resources(struct virt_dma_chan *vc)
>> LIST_HEAD(head);
>>
>> spin_lock_irqsave(&vc->lock, flags);
>> - vchan_get_all_descriptors(vc, &head);
>> + vchan_get_all_allocated_descs(vc, &head);
>> list_for_each_entry(vd, &head, node)
>> dmaengine_desc_clear_reuse(&vd->tx);
>> spin_unlock_irqrestore(&vc->lock, flags);
>> --
>> 2.33.0
>>
> .
Powered by blists - more mailing lists