linux-kernel - Re: [PATCH 1/2] dmaengine: axi-dmac: Use devm_request

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

Message-ID: <CAJYdmePT+EYK5MgF4tjS47reF2FPOOd+G3jZaQ4GitgQMMQsEg@mail.gmail.com>
Date:   Thu, 26 Apr 2018 09:43:01 -0700
From:   Moritz Fischer <moritz.fischer.private@...il.com>
To:     Lars-Peter Clausen <lars@...afoo.de>
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        dmaengine@...r.kernel.org, vinod.koul@...el.com,
        dan.j.williams@...el.com
Subject: Re: [PATCH 1/2] dmaengine: axi-dmac: Use devm_request_irq()

Hi Lars,

On Thu, Apr 26, 2018 at 9:26 AM, Lars-Peter Clausen <lars@...afoo.de> wrote:
> On 04/26/2018 06:12 PM, Moritz Fischer wrote:
>> Use devm_request_irq() instead of request_irq() to request the IRQ.
>>
>> Signed-off-by: Moritz Fischer <mdf@...nel.org>
>> ---
>>
>> Hi Lars,
>>
>> was there a specific reason not to use devm_request_irq() ?
>
> Yes, it is wrong :)
>
> This reorders the sequence in the remove() function. We really need to free
> the IRQ at the point in the sequence where it is at the moment. We need to
> call tasklet_kill() after the free_irq() function has been called, otherwise
> the tasklet can be re-scheduled and we might end up with a use after free.
>
> But also if we disable the clock to the core before the IRQ is freed we
> might end up trying to access the register map while the clock is off and
> then never get a response and block the AXI bus.
>
> This is all very very unlikely to ever happen, but still I'd like to have
> correct code.

Thanks for the explanation. I'll drop this and resubmit [2/2] without
this change.

- Moritz