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

[Richard Patel <ripatel@wii.dev>]

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:  
> > > > I don't understand the code very well, but isn't there a second UAF
> > > > with the ctx object getting freed?  kfree(ctx) in sdw_intel_exit()
> > > > runs well before the IRQ is unregistered.                        
> > >                                                                    
> > > This situation is unfortunately fairly complex, the IRQ is shared  
> > > between many different functions and only the SoundWire function   
> > > relies on ctx. I will do some more poking, but I believe the       
> > > free order is such that the soundwire stuff is shutdown before     
> > > ctx is freed. I am not 100% certain if that will prevent the       
> > > SoundWire IRQ path from getting called, although I would like      
> > > to believe it does :-)                                             
> >                                                                      
> > Hmm... ok so poking this a little more looks like I do see just      
> > see these freed in hard the wrong order so we should probably        
> > fix that up too. Thanks for spotting that I will have a bit of       
> > a think.                                                             
>                                                                        
> 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.

  BUG: unable to handle page fault for address: fffffffffffffff8
  Oops: Oops: 0000 [#1] SMP NOPTI
  CPU: 5 UID: 0 PID: 34491 Comm: irq/165-AudioDS Kdump: loaded Tainted: G           OE       7.3.0-rc6-ibt+ #2 PREEMPT(lazy) 
  RIP: 0010:sdw_intel_thread+0x3c/0x70 [soundwire_intel]
  Call Trace:
   <TASK>
   hda_dsp_interrupt_thread+0x97/0x320 [snd_sof_intel_hda_generic]
   irq_thread_fn+0x23/0x60
   irq_thread+0xc7/0x190
   kthread+0xe5/0x120
   </TASK>

--Richard

---
diff --git a/drivers/soundwire/intel_init.c b/drivers/soundwire/intel_init.c
index a7437cd42028..66a08dad7281 100644
--- a/drivers/soundwire/intel_init.c
+++ b/drivers/soundwire/intel_init.c
@@ -155,6 +155,8 @@ irqreturn_t sdw_intel_thread(int irq, void *dev_id)
 	struct sdw_intel_ctx *ctx = dev_id;
 	struct sdw_intel_link_res *link;
 
+	msleep(5000);
+
 	mutex_lock(&ctx->link_lock);
 	list_for_each_entry(link, &ctx->link_list, list)
 		sdw_cdns_irq(irq, link->cdns);
@@ -393,6 +395,7 @@ void sdw_intel_exit(struct sdw_intel_ctx *ctx)
 	sdw_intel_cleanup(ctx);
 	kfree(ctx->peripherals);
 	kfree(ctx->ldev);
+	memset(ctx, 0, sizeof(*ctx));
 	kfree(ctx);
 }
 EXPORT_SYMBOL_NS(sdw_intel_exit, "SOUNDWIRE_INTEL_INIT");