sound: core: snd_card_free()'s one-shot 'releasing' admission is an unlocked TOCTOU (latent: no concurrent caller in tree today)

sound: core: snd_card_free()'s one-shot 'releasing' admission is an unlocked TOCTOU (latent: no concurrent caller in tree today)

[徐晗翔]

snd_card_free() implements its re-entrancy protection as a single
unlocked boolean: it reads card->releasing, writes a stack completion
pointer into card->release_completion, and only sets releasing = true
much later inside snd_card_do_free() -- which runs from the card_dev
kref release, possibly long after the check.  If two threads ever
entered it concurrently for the same card, the second would either
drop a second card_dev reference (kref underflow: the card is freed
while files are open, and the later file close does a third
put_device on freed memory), or overwrite the first caller's
completion (snd_card_do_free() completes only the last writer, so the
first caller sleeps forever, typically holding the driver-core device
lock), or race past the first caller's kfree(card) outright.  I
enumerated every caller in the tree: today they are all serialized
(one entry point per card at a time -- PCI remove under the pci_dev
mutex; managed cards call it multiple times but sequentially on the
same thread; the USB-audio and misc drivers serialize via
register_mutex / shutdown counters / interface locks), so this is
reported as a latent, defensive-hardening issue, not a reachable bug
-- the code comment at sound/core/init.c:653-657 already admits the
multi-entry design ("the release via devres may call snd_card_free()
twice due to its nature"), and the protection between those entries
is only the assumption that they never overlap.

The detailed description:
* The code (Linux 7.3, sound/core/init.c:649-667)
      void snd_card_free(struct snd_card *card)
      {
              DECLARE_COMPLETION_ONSTACK(released);
              if (card->releasing)                    /* (1) unlocked read */
                      return;
              card->release_completion = &released;  /* (2) unlocked write
                                                        of a stack addr */
              snd_card_free_when_closed(card);        /* (3) disconnect +
                                                        put_device, no
                                                        card-level lock */
              wait_for_completion(&released);         /* (4) */
      }
  releasing = true happens only in snd_card_do_free()
  (init.c:588), invoked from the card_dev device release
  (init.c:151-154) -- i.e. after put_device() inside (3) has dropped
  the last-but-one reference, arbitrarily later than (1).

* Consequences if a concurrent second entry ever exists
  1. kref double drop: with one open ALSA file (each file holds a
     card_dev reference, init.c:1111), two put_device() calls from
     two snd_card_free()s can bring the kref to zero while the file
     is still open; snd_card_do_free() frees all devices and the card
     (init.c:610), and the file's close path then does a third
     put_device() (init.c:1153) on freed memory (kref underflow +
     the close path's subsequent work on freed card memory).
  2. completion overwrite: the second caller overwrites
     release_completion with its own stack address; do_free
     completes only the last writer (init.c:607-608); the first
     caller waits forever -- with its caller context typically
     holding a driver-core device lock, this is a locked-up hang.
  3. post-kfree access: any interleaving where the first entry
     finishes kfree(card) before the second executes (2)/(3) is
     straight UAF on the card.
  Note snd_card_free_when_closed() itself (init.c:625-633) calls
  put_device() unconditionally -- snd_card_disconnect() inside it is
  one-shot via card->shutdown, but the reference drop is not guarded
  either, so any *combination* of the two entry functions has the
  same exposure.

* Caller enumeration (why this is latent today)
  - HDA PCI controllers (snd_card_new, non-managed): single
    snd_card_free() from PCI remove, under pci_dev->mutex
    (device_release_driver_internal).
  - managed cards (snd_devm_card_new users): driver remove + the two
    devres actions all run sequentially in the same thread during
    device_del/driver unbind (drivers/base/dd.c:593-618); the first
    call completes do_free and sets releasing, the later ones return
    at (1).  This is exactly the case the init.c comment describes.
  - snd_card_free_on_error(): probe-context, under the device lock.
  - USB audio main driver (card.c:1116/1165): probe-error vs
    disconnect serialized by register_mutex +
    atomic_inc_return(&chip->shutdown) == 1 + num_interfaces
    counting.
  - line6 / ua101 / 6fire / hiface / bcd2000 / usx2y: single-interface
    device locks or devices_mutex serialization.
  - virtio (virtio_card.c:355): single call under the virtio device
    lock.
  No pair of these can run concurrently against the same card in the
  current tree.

* Verification
  On a QEMU/KVM x86_64 guest with an emulated Intel HDA controller
  (intel-hda + hda-duplex), KASAN kernel built from the vanilla 7.3
  tarball (+ CONFIG_SND_HDA_RECONFIG/GENERIC), exercising the full
  card create / reconfig / PCI unbind / bind cycle and a 40-round
  unbind/bind churn: zero splats -- consistent with the enumeration
  (there is nothing to trigger).  This is a negative result and is
  reported as such.



Regards,
Hanxiang Xu
xuhx@hust.edu.cn