Re: ALSA: ctxfi: free atimer only after the timer IRQ is quiesced

[Takashi Iwai <tiwai@suse.de>]

On Fri, 13 Nov 2026 12:58:51 +0100,
徐晗翔 wrote:
> 
> `ct_timer_free()` frees `struct ct_timer` while the ctxfi timer IRQ handler can
> still be executing, or can start after `snd_card_disconnect()` has already done
> its single `synchronize_irq()`.  A handler that has passed the non-atomic
> `hw->irq_callback` check then calls `ct_timer_interrupt()` with the freed
> `atimer`; `free_irq()` only runs later, inside `ct_atc_destroy()` ->
> `destroy_hw_obj()`.  This is a use-after-free on the timer object and can crash
> or wedge the device during PCI removal/unbind.
> 
> The detailed description:
> 
> * Test environment / affected version
> 
>   Linux 7.3.0 source tree under review.  This report is source-level only: no
>   ctxfi hardware was available and QEMU has no ctxfi device model, so no
>   driver-specific reproducer was run.  The source paths/line numbers below are
>   from 7.3.
> 
> * Mechanism
> 
>   `ct_timer_new()` (`sound/pci/ctxfi/cttimer.c:388-411`) installs the native
>   timer callback by default when `!use_system_timer && hw->set_timer_irq`:
> 
>     hw->irq_callback_data = atimer;
>     hw->irq_callback      = ct_timer_interrupt;
> 
>   The PCI IRQ handlers `ct_20k1_interrupt()` (`cthw20k1.c:1878-1891`) and
>   `ct_20k2_interrupt()` (`cthw20k2.c:2045-2059`) do:
> 
>     if (hw->irq_callback)
>         hw->irq_callback(hw->irq_callback_data, status);
> 
>   `ct_timer_interrupt()` (`cttimer.c:379-386`) then reads
>   `timer->ops->interrupt` and calls it with `timer`.  Neither the check nor
>   the two loads is atomic.
> 
>   `ct_timer_free()` (`cttimer.c:414-421`) does:
> 
>     struct hw *hw = atimer->atc->hw;
>     hw->irq_callback = NULL;
>     if (atimer->ops->free_global)
>         atimer->ops->free_global(atimer);
>     kfree(atimer);
> 
>   `ct_xfitimer_free_global()` only calls `ct_xfitimer_irq_stop()`; that helper
>   stops the hardware only if `atimer->running` is set (`cttimer.c:137-145`).
>   It does not wait for an in-flight IRQ handler.
> 
>   `ct_atc_destroy()` (`ctatc.c:1250-1286`) then calls `ct_timer_free()` at
>   1257-1260, calls `destroy_hw_obj(atc->hw)` at 1274-1275, and only after that
>   `kfree(atc)` at 1283.  `destroy_hw_obj()` -> `hw_card_shutdown()` ->
>   `free_irq()` at `cthw20k1.c:1969` / `cthw20k2.c:2140`.  So `atc` itself is
>   not the freed object in this window; `atimer` is freed before `free_irq()`.
> 
> * Why the existing synchronization does not close the window
> 
>   ctxfi does set `card->sync_irq` at `cthw20k1.c:1937` / `cthw20k2.c:2104`,
>   and `snd_card_disconnect()` (`sound/core/init.c:495-562`) calls
>   `synchronize_irq(card->sync_irq)` once at 540-541.  But that call happens
>   before `ct_timer_free()`, only waits handlers that are running at that
>   instant, and does not disable the IRQ line.  A new handler can enter after
>   the one-time sync and before `free_irq()`.
> 
>   The `hw->irq_callback = NULL` store does not close the race either: a
>   handler that loaded the callback pointer before the store can still call it.
> 
> * Trigger model
> 
>   Open a ctxfi device (native timer path, the default), then remove/unbind the
>   PCI device while an interrupt is pending or arrives during teardown.  The
>   window is between `ct_timer_free()` and `free_irq()`.  Consequence: UAF read
>   of `atimer->ops` and timer fields in IRQ context; crash, stall, or potential
>   control-flow corruption.
> 
> * Suggested direction
> 
>   After clearing `hw->irq_callback` and calling `free_global()`, but before
>   `kfree(atimer)`, do:
> 
>     if (hw->irq >= 0)
>         synchronize_irq(hw->irq);
> 
>   Alternatively, move `free_irq()` earlier so it precedes the timer free.
>   Keep the existing `hw->irq == -1` initialization contract.
> 
> 
> Please keep me in Cc; I am not subscribed to the lists.

Could you rather submit a fix patch (with a proper patch description)
for each your reported issue?  You can put Link: tag to pointing to
the earlier reports.


thanks,

Takashi