Re: [PATCH v2 3/3] soundwire: intel_auxdevice: Don't disable IRQs before removing children

[Richard Patel <ripatel@wii.dev>]

On Fri, Oct 09, 2026 at 10:03:25AM +0100, Charles Keepax wrote:
> On Thu, Oct 08, 2026 at 11:11:32PM +0000, Richard Patel wrote:
> > On Thu, Oct 08, 2026 at 01:41:06PM +0100, Charles Keepax wrote:
> > > On Mon, Oct 05, 2026 at 02:11:47PM +0100, Charles Keepax wrote:
> > > > On Mon, Oct 05, 2026 at 11:32:05AM +0100, Charles Keepax wrote:
> > > > > On Sun, Oct 04, 2026 at 12:29:58PM +0000, Richard Patel wrote:
> > > > > > On Fri, Sep 25, 2026 at 04:42:16PM +0100, Charles Keepax wrote:
> > > Ok found some time to look at this properly I think this is all
> > > fine. sdw_intel_exit() first calls sdw_intel_cleanup() which will
> > > eventually call sdw_cdns_enable_interrupt(..., false), which
> > > should disable the SoundWire IRQs. Then sdw_intel_exit() frees
> > > the ctx, whilst at that point whilst the IRQ is still registered
> > > one should no longer be able to see soundwire IRQs, so you shouldn't
> > > get a dereferencing of ctx.
> >
> > On my Galaxy Book6, I was able to get a ctx UAF with your v2 patch set
> > by adding a sleep.
> Hmm... yeah, I guess the masking ensures a new IRQ can't come in
> but nothing ensures a currently running IRQ is synchronised in.
> Well assuming the masking does actually prevent an IRQ coming in.
> 
> That is a little awkward, normally freeing the IRQ would
> synchronise it but as the "IRQ" here is done as a pile of
> callbacks that doesn't happen. We could do a manual sync on the
> IRQ but that feels like a bit of a layering violation, since
> the actually IRQ is several layers away in another part of the
> code. We could add some flags/completions such that we can wait
> for the current IRQ to finish but feels a bit like adding code
> that shouldn't exist. I think the correct solution is probably
> to switch the handling over to the IRQ framework.
> 
> I am going to go for the theory this is not directly a problem
> with this series since the problem exists unchanged before and
> after the series. So lets not block this stuff on it, but I will

Yep, sounds good :-) Thanks again for the fixes. 

> try to find time to start porting more of the handling over to
> the IRQ framework, or happy to help review if you would rather
> take a run at it.

I was going to defer kfree(ctx) via RCU, what do you think?

Cheers,
-- Richard