linux-kernel - Re: [PATCH v1 3/7] dmaengine: tegra-apb: Prevent race conditions on channel's freeing

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:   Thu, 2 Jan 2020 18:09:45 +0300
From:   Dmitry Osipenko <digetx@...il.com>
To:     Michał Mirosław <mirq-linux@...e.qmqm.pl>
Cc:     Laxman Dewangan <ldewangan@...dia.com>,
        Vinod Koul <vkoul@...nel.org>,
        Dan Williams <dan.j.williams@...el.com>,
        Thierry Reding <thierry.reding@...il.com>,
        Jonathan Hunter <jonathanh@...dia.com>,
        dmaengine@...r.kernel.org, linux-tegra@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 3/7] dmaengine: tegra-apb: Prevent race conditions on
 channel's freeing

30.12.2019 23:50, Michał Mirosław пишет:
> On Mon, Dec 30, 2019 at 09:45:55PM +0100, Michał Mirosław wrote:
>> On Sat, Dec 28, 2019 at 11:46:36PM +0300, Dmitry Osipenko wrote:
>>> It's unsafe to check the channel's "busy" state without taking a lock,
>>> it is also unsafe to assume that tasklet isn't in-fly.
>>
>> 'in-flight'. Also, the patch seems to have two independent bug-fixes
>> in it. Second one doesn't look right, at least not without an explanation.
>>
>> First:
>>
>>> -	if (tdc->busy)
>>> -		tegra_dma_terminate_all(dc);
>>> +	tegra_dma_terminate_all(dc);
>>
>> Second:
>>
>>> +	tasklet_kill(&tdc->tasklet);
> 
> BTW, maybe you can convert the code to threaded interrupt handler and
> just get rid of the tasklet instead of fixing it?

This shouldn't bring much benefit because the the code's logic won't be
changed since we will still have to use the threaded ISR part as the
bottom-half and then IRQ API doesn't provide a nice way to synchronize
interrupt's execution, while tasklet_kill() is a nice way to sync it.