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

[Takashi Iwai]

On Mon, 28 Sep 2026 18:16:26 +0200,
ROJOX wrote:
> 
> 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?

AFAIK, there is no lifecycle protection in the code in question.
So a serialization like get_device() would be likely a good to have,
indeed.  Feel free to cook and pitch your fix.


thanks,

Takashi

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