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

[徐晗翔 <xuhx@hust.edu.cn>]

`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.


Regards,
Hanxiang Xu
xuhx@hust.edu.cn