Re: [RFC] ALSA: hda: clarify unsol_event locking across driver unbind

[ROJOX]

Hi Takashi,

Thanks for taking a look.

Yes, I agree that the whole-controller teardown path appears safe in this
respect: snd_hdac_bus_exit() does cancel_work_sync(&bus->unsol_work), so an
already running unsolicited worker is drained before the bus itself is torn
down.

The case I am concerned about is the individual codec unbind/reset path,
where the HDA bus and its unsol_work remain alive.

For example, snd_hda_codec_reset() calls device_release_driver() for the
codec device:

https://github.com/torvalds/linux/blob/165768bb70265b5c38cf0b73fafd75be235f8b14/sound/hda/common/codec.c#L1799-L1810

and the legacy HDA remove wrapper reaches the codec driver's ->remove()
callback and subsequent unbind cleanup:

https://github.com/torvalds/linux/blob/165768bb70265b5c38cf0b73fafd75be235f8b14/sound/hda/common/bind.c#L152-L173

So I think there are two separate lifetime questions here.

Taking get_device() around the unsolicited dispatch would protect the
hdac_device/struct device object itself, which addresses the possibility of
the codec object disappearing while the worker is using it.

What I am not sure it protects is the bound driver's private state. The
device can remain alive while device_release_driver() runs the codec
driver's ->remove() callback and that callback tears down state subsequently
used by ->unsol_event().

Conceptually, I am worried about this ordering:

    unsol worker                     codec unbind
    ------------                     ------------
    get_device(codec)
    resolve current driver
                                     ->remove()
                                     free driver-private state
    ->unsol_event()
      use driver-private state
    put_device(codec)

So my concern is not primarily the lifetime of struct hdac_device itself,
but whether there is an existing guarantee that serializes the driver
binding/private state against unsol_event() during individual codec unbind.

If there is such an invariant elsewhere in the HDA or driver-core lifecycle,
I may simply be missing it.

Otherwise, would you expect the fix to protect only the device object with
get_device(), or would the current binding also need to be serialized or
revalidated against ->remove() before dispatch?

Thanks,
Rojox
iMac19,2 Linux audio project

On Mon, 28 Sep 2026 17:59:03 +0200, Takashi Iwai <tiwai@xxxxxxx> wrote:
> On Fri, 25 Sep 2026 04:57:13 +0200,
> ROJOX wrote:
> >
> > Hi,
> >
> > Could you advise on the lifetime and locking contract for
> > hdac_driver::unsol_event() relative to codec driver unbind?
> >
> > In current torvalds/linux, snd_hdac_bus_process_unsol_events() looks up a
> > codec in bus->caddr_tbl under bus->reg_lock, checks codec->registered, and
> > then drops reg_lock. It subsequently reads codec->dev.driver and dispatches
> > drv->unsol_event(codec, res), without taking a codec device reference or
> > device_lock in that interval:
> >
> > https://github.com/torvalds/linux/blob/165768bb70265b5c38cf0b73fafd75be235f8b14/sound/hda/core/bus.c#L173-L189
> >
> > Separately, driver-core unbind acquires the device lock and calls the remove
> > path while that lock is held. The HDA reset path can initiate this through
> > device_release_driver():
> >
> > https://github.com/torvalds/linux/blob/165768bb70265b5c38cf0b73fafd75be235f8b14/drivers/base/dd.c#L1315-L1374
> > https://github.com/torvalds/linux/blob/165768bb70265b5c38cf0b73fafd75be235f8b14/sound/hda/common/codec.c#L1799-L1810
> >
> > This appears to permit the following source-level ordering:
> >
> > unsol worker unbind task
> > ------------ -----------
> > lookup codec under reg_lock
> > drop reg_lock
> > read codec->dev.driver
> > enter unsol_event()
> > acquire device_lock
> > invoke remove
> > tear down private state
> > callback continues using private state
>
> Usually the HD-audio controller driver calls snd_hdac_bus_exit() at
> its remove callback (or via the destructor invoked from there), and it
> does cancel_work_sync() for the unsol event worker.
>
> I thought the removal of codec->dev.driver happened after the
> driver_detach(), so at that point, isn't the device object itself
> still alive?
>
> Or maybe I haven't followed the flow completely yet. If the issue is
> real, just taking the codec's device reference around the call would
> be the easiest solution, I guess.
>
> thanks,
>
> Takashi
>
> > I do not see the worker taking the device lock or another explicit
> > binding/private-state lifetime reference before dispatch. If the callback
> > uses state destroyed by the remove path, the unlocked dispatch therefore
> > appears able to overlap that teardown.
> >
> > The codec/device object lifetime, driver binding lifetime, and
> > driver-private state lifetime are distinct here: get_device() alone would
> > pin the first, but would not by itself prevent unbind or preserve private
> > state.
> >
> > The legacy HDA wrapper additionally checks shutdown and system-PM state
> > before calling the codec callback, but those checks do not appear to drain
> > a callback that has already passed them:
> >
> > https://github.com/torvalds/linux/blob/165768bb70265b5c38cf0b73fafd75be235f8b14/sound/hda/common/bind.c#L42-L52
> >
> > I also found the older unsolicited queue-index synchronization fix,
> > c637fa151259c0f74665fde7cba5b7eac1417ae5. That appears to address queue
> > consistency rather than this binding/private-state lifetime interval.
> >
> > I tested one scratch proof candidate, not a proposed patch. It synchronizes
> > address-table lookup/removal, obtains a codec device reference while the
> > entry is protected, drops reg_lock, takes device_lock, revalidates the
> > current binding and codec registration state, and dispatches the callback
> > while that lock is held.
> >
> > That closes the ordinary callback-versus-remove schedule in a deterministic
> > model, and the modified HDA objects build cleanly in a scratch Ubuntu
> > 7.0.0-34.34 source tree. However, hdac_driver::unsol_event() currently does
> > not document callback-under-device_lock or reentry restrictions, so I am
> > not assuming that this is the correct fix.
> >
> > Could you please clarify:
> >
> > 1. Is HDA core expected to serialize unsol_event() against unbind by holding
> > the codec device_lock through dispatch, or is there another existing
> > lifetime guarantee intended here?
> >
> > 2. If that lock context is acceptable, should the callback contract prohibit
> > recursively taking the same device lock and synchronous same-codec
> > unbind/reset/reprobe? I found no direct same-codec teardown call in the
> > audited in-tree callback bodies, but indirect and out-of-tree behavior is
> > not established.
> >
> > 3. Would callback-under-device_lock conflict with expected HDA runtime-PM or
> > system-PM behavior? My source review did not establish a generic contract
> > for this.
> >
> > 4. For an event queued before unbind/rebind, is it expected to be deliverable
> > to the newly bound driver, or should it be discarded across the
> > binding/reset boundary?
> >
> > This came up while auditing HDA/CS8409 codec lifetime behavior; the question
> > is about the generic HDA unsolicited-callback contract.
> >
> > No runtime crash has been reproduced. This is based on source/lifetime
> > analysis and deterministic concurrency modeling, not a runtime stress test.
> > I may be missing an existing lifetime guarantee or invariant, and would
> > appreciate correction.
> >
> > The source snapshot checked is torvalds/linux master at
> > 165768bb70265b5c38cf0b73fafd75be235f8b14. No patch is proposed or attached.
> >
> > Thanks,
> > Rojox
> > iMac19,2 Linux audio project