linux-kernel - Re: [PATCH] irq: fix the interrupt trigger type override issue

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAJNi4rPooS82fB+6FditywXTga00JbGoFM6MR8P7U3s7mdbJCg@mail.gmail.com>
Date: Mon, 2 Sep 2024 20:50:27 +0800
From: richard clark <richard.xnu.clark@...il.com>
To: Thomas Gleixner <tglx@...utronix.de>
Cc: linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org, 
	torvalds@...ux-foundation.org, richard clark <richard.xnu.clark@...il.com>
Subject: Re: [PATCH] irq: fix the interrupt trigger type override issue

Hi Thomas,

On Mon, Sep 2, 2024 at 5:51 PM Thomas Gleixner <tglx@...utronix.de> wrote:
>
> Richard!
>
> On Mon, Sep 02 2024 at 16:42, richard clark wrote:
> > On Mon, Sep 2, 2024 at 3:34 PM Thomas Gleixner <tglx@...utronix.de> wrote:
> >> 1) insmod()
> >>      irq_create_fwspec_mapping(fwspec)
> >>        irq_domain_translate(fwspec, ... &type); <- Sets type to the FW value
> >>
> >>        virq = irq_find_mapping(domain, hwirq);
> >>        if (virq) {
> >>          // Path not taken
> >>        }
> >>
> >>        // Map interrupt
> >>        ...
> >>
> >>        irqd_set_trigger_type(..., type);
> >>
> >> 2) rmmod()
> >>      tears down mapping
> >>
> > This just tears down the action allocated and installed by
> > request_irq(...), but does not teardown the irq's node inserted in the
> > revmap_tree.
>
> So what creates the mapping? If the driver creates it then why doesn't
> it unmap it when it exits?
>
Kernel allocates an irq and creates the mapping during its
initialization when parsing the device's interrupt firmware, not the
driver does that.
>
> >> 3) insmod()
> >>
> >>       Should be exactly the same as #1 because the previous mapping was
> >>       torn down by rmmod()
> >>
> > Not the same exactly, the {irq, irq_data} will still be in the
> > revmap_tree, so it will enter another path in this case:
>
> That's exactly the question. Why does the mapping persist?
>
AFAICT the kernel creates the mapping to bind the allocated irq to its
firmware node, thus this {key, value} pair can be used persistently in
the future while not re-iterate the cycle again. irq and the mapping
can be regarded as the part of the kernel, not the scope of the
driver.
>
> >> So how exactly does that happen what you describe?
> >>
> > The logic is if the trigger type specified by request_irq(...) is not
> > consistent with the firmware one, the request_irq will override the
> > FW. We need to keep this logic the same as when we insmod the same
> > kmod next time -- override the FW's too instead of returning a
> > mismatch type error.
>
> I can see how that can happen, but what's missing is the information why
> this mapping persists and why it's tried to be set up again.
>
As I mentioned, it doesn't try to set up again. It will just lookup
the mapping from the tree for the persistent reason when driver try to
request the irq...

Thanks,
>
> Thanks,
>
>         tglx