* [PATCH 0/2] ALSA: hda: Fix Cirrus ACPI device reference leaks
@ 2026-04-28 7:44 Shuhao Fu
2026-04-28 8:01 ` [PATCH 1/2] ALSA: hda: cs35l56: Put ACPI device after setting companion Shuhao Fu
` (3 more replies)
0 siblings, 4 replies; 7+ messages in thread
From: Shuhao Fu @ 2026-04-28 7:44 UTC (permalink / raw)
To: David Rhodes, Richard Fitzgerald
Cc: Jaroslav Kysela, Takashi Iwai, linux-sound, patches, linux-kernel
This series fixes two missing acpi_dev_put() cases in Cirrus HDA side-codec
ACPI lookup paths.
Patch 1 drops the lookup reference after synthesizing the companion in
cs35l56_hda_read_acpi().
Patch 2 drops the lookup reference on the early !physdev error path in
cs35l41_hda_read_acpi().
Shuhao Fu (2):
ALSA: hda: cs35l56: Put ACPI device after setting companion
ALSA: hda: cs35l41: Fix ACPI device leak on missing physical node
sound/hda/codecs/side-codecs/cs35l41_hda.c | 4 +++-
sound/hda/codecs/side-codecs/cs35l56_hda.c | 1 +
2 files changed, 4 insertions(+), 1 deletion(-)
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] ALSA: hda: cs35l56: Put ACPI device after setting companion
2026-04-28 7:44 [PATCH 0/2] ALSA: hda: Fix Cirrus ACPI device reference leaks Shuhao Fu
@ 2026-04-28 8:01 ` Shuhao Fu
2026-04-28 9:05 ` Richard Fitzgerald
2026-04-28 8:12 ` [PATCH 2/2] ALSA: hda: cs35l41: Put ACPI device on missing physical node Shuhao Fu
` (2 subsequent siblings)
3 siblings, 1 reply; 7+ messages in thread
From: Shuhao Fu @ 2026-04-28 8:01 UTC (permalink / raw)
To: David Rhodes, Richard Fitzgerald
Cc: Jaroslav Kysela, Takashi Iwai, linux-sound, patches, linux-kernel
acpi_dev_get_first_match_dev() returns a refcounted ACPI device and
callers are expected to balance it with acpi_dev_put().
When no companion is already attached, cs35l56_hda_read_acpi() looks
up an ACPI device and sets it with ACPI_COMPANION_SET(), but leaves
the lookup reference held.
ACPI_COMPANION_SET() does not take ownership of that reference, so
drop it with acpi_dev_put() after attaching the companion.
Fixes: 73cfbfa9caea ("ALSA: hda/cs35l56: Add driver for Cirrus Logic CS35L56 amplifier")
Signed-off-by: Shuhao Fu <sfual@cse.ust.hk>
---
sound/hda/codecs/side-codecs/cs35l56_hda.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/sound/hda/codecs/side-codecs/cs35l56_hda.c b/sound/hda/codecs/side-codecs/cs35l56_hda.c
index 1ace4beef50857b..16a5a40899b2873 100644
--- a/sound/hda/codecs/side-codecs/cs35l56_hda.c
+++ b/sound/hda/codecs/side-codecs/cs35l56_hda.c
@@ -1032,6 +1032,7 @@ static int cs35l56_hda_read_acpi(struct cs35l56_hda *cs35l56, int hid, int id)
return -ENODEV;
}
ACPI_COMPANION_SET(cs35l56->base.dev, adev);
+ acpi_dev_put(adev);
}
/* Initialize things that could be overwritten by a fixup */
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] ALSA: hda: cs35l41: Put ACPI device on missing physical node
2026-04-28 7:44 [PATCH 0/2] ALSA: hda: Fix Cirrus ACPI device reference leaks Shuhao Fu
2026-04-28 8:01 ` [PATCH 1/2] ALSA: hda: cs35l56: Put ACPI device after setting companion Shuhao Fu
@ 2026-04-28 8:12 ` Shuhao Fu
2026-04-28 10:40 ` [PATCH 1/2] ALSA: hda: cs35l56: Put ACPI device after setting companion Shuhao Fu
2026-05-07 14:32 ` [PATCH 0/2] ALSA: hda: Fix Cirrus ACPI device reference leaks Takashi Iwai
3 siblings, 0 replies; 7+ messages in thread
From: Shuhao Fu @ 2026-04-28 8:12 UTC (permalink / raw)
To: David Rhodes, Richard Fitzgerald
Cc: Jaroslav Kysela, Takashi Iwai, linux-sound, patches, linux-kernel
acpi_dev_get_first_match_dev() returns a refcounted ACPI device and
callers must balance it with acpi_dev_put().
cs35l41_hda_read_acpi() stores the returned ACPI device in
cs35l41->dacpi. That reference is normally released by the later
probe cleanup or the remove path, but the NULL-check on
physdev exits before either of those paths can run.
Drop the lookup reference before returning -ENODEV.
Fixes: c34b04cc6178 ("ALSA: hda: cs35l41: Fix NULL pointer dereference in cs35l41_hda_read_acpi()")
Signed-off-by: Shuhao Fu <sfual@cse.ust.hk>
---
sound/hda/codecs/side-codecs/cs35l41_hda.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/sound/hda/codecs/side-codecs/cs35l41_hda.c b/sound/hda/codecs/side-codecs/cs35l41_hda.c
index b64890006bb7019..acfccc848f82d82 100644
--- a/sound/hda/codecs/side-codecs/cs35l41_hda.c
+++ b/sound/hda/codecs/side-codecs/cs35l41_hda.c
@@ -1896,8 +1896,10 @@ static int cs35l41_hda_read_acpi(struct cs35l41_hda *cs35l41, const char *hid, i
cs35l41->dacpi = adev;
physdev = get_device(acpi_get_first_physical_node(adev));
- if (!physdev)
+ if (!physdev) {
+ acpi_dev_put(adev);
return -ENODEV;
+ }
sub = acpi_get_subsystem_id(ACPI_HANDLE(physdev));
if (IS_ERR(sub))
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] ALSA: hda: cs35l56: Put ACPI device after setting companion
2026-04-28 8:01 ` [PATCH 1/2] ALSA: hda: cs35l56: Put ACPI device after setting companion Shuhao Fu
@ 2026-04-28 9:05 ` Richard Fitzgerald
0 siblings, 0 replies; 7+ messages in thread
From: Richard Fitzgerald @ 2026-04-28 9:05 UTC (permalink / raw)
To: Shuhao Fu, David Rhodes
Cc: Jaroslav Kysela, Takashi Iwai, linux-sound, patches, linux-kernel
On 28/04/2026 9:01 am, Shuhao Fu wrote:
> acpi_dev_get_first_match_dev() returns a refcounted ACPI device and
> callers are expected to balance it with acpi_dev_put().
>
> When no companion is already attached, cs35l56_hda_read_acpi() looks
> up an ACPI device and sets it with ACPI_COMPANION_SET(), but leaves
> the lookup reference held.
>
> ACPI_COMPANION_SET() does not take ownership of that reference, so
> drop it with acpi_dev_put() after attaching the companion.
Are you sure about this?
I remember when I wrote this code I checked the driver core and saw that
if there is a companion it puts it when the driver is removed.
That is why I didn't put the reference here, it would have caused a
double put.
>
> Fixes: 73cfbfa9caea ("ALSA: hda/cs35l56: Add driver for Cirrus Logic CS35L56 amplifier")
> Signed-off-by: Shuhao Fu <sfual@cse.ust.hk>
> ---
> sound/hda/codecs/side-codecs/cs35l56_hda.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/sound/hda/codecs/side-codecs/cs35l56_hda.c b/sound/hda/codecs/side-codecs/cs35l56_hda.c
> index 1ace4beef50857b..16a5a40899b2873 100644
> --- a/sound/hda/codecs/side-codecs/cs35l56_hda.c
> +++ b/sound/hda/codecs/side-codecs/cs35l56_hda.c
> @@ -1032,6 +1032,7 @@ static int cs35l56_hda_read_acpi(struct cs35l56_hda *cs35l56, int hid, int id)
> return -ENODEV;
> }
> ACPI_COMPANION_SET(cs35l56->base.dev, adev);
> + acpi_dev_put(adev);
> }
>
> /* Initialize things that could be overwritten by a fixup */
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] ALSA: hda: cs35l56: Put ACPI device after setting companion
2026-04-28 7:44 [PATCH 0/2] ALSA: hda: Fix Cirrus ACPI device reference leaks Shuhao Fu
2026-04-28 8:01 ` [PATCH 1/2] ALSA: hda: cs35l56: Put ACPI device after setting companion Shuhao Fu
2026-04-28 8:12 ` [PATCH 2/2] ALSA: hda: cs35l41: Put ACPI device on missing physical node Shuhao Fu
@ 2026-04-28 10:40 ` Shuhao Fu
2026-05-06 14:37 ` Simon Trimmer
2026-05-07 14:32 ` [PATCH 0/2] ALSA: hda: Fix Cirrus ACPI device reference leaks Takashi Iwai
3 siblings, 1 reply; 7+ messages in thread
From: Shuhao Fu @ 2026-04-28 10:40 UTC (permalink / raw)
To: David Rhodes, Richard Fitzgerald
Cc: Jaroslav Kysela, Takashi Iwai, linux-sound, patches, linux-kernel
Hi Richard,
> Are you sure about this?
> I remember when I wrote this code I checked the driver core and saw that
> if there is a companion it puts it when the driver is removed.
> That is why I didn't put the reference here, it would have caused a
> double put.
I may well be missing something here. But from my reading of the current
code, it does not seem to cause a double put.
The place where I do seem to find ACPI companion cleanup is when the device
object itself is deleted/unregistered:
`device_del()`
-> `device_platform_notify_remove()`
-> `acpi_device_notify_remove()`
-> `acpi_unbind_one()`
What makes me think this is not the matching put for
`acpi_dev_get_first_match_dev()` is that `acpi_unbind_one()` only calls
`acpi_dev_put()` after it finds a matching entry for the device in
`acpi_dev->physical_node_list`.
As far as I can tell, that list entry is created by `acpi_bind_one()`, which
also takes its own extra reference with `acpi_dev_get(acpi_dev)`. So the put
in `acpi_unbind_one()` looks to me like it is paired with that
`acpi_bind_one()` reference, rather than with the earlier
`acpi_dev_get_first_match_dev()` lookup.
If that reading is right, then I think the ownership looks like this:
- `ACPI_COMPANION_SET()` only attaches the companion pointer/fwnode
- the lookup reference from `acpi_dev_get_first_match_dev()` is still with
the caller
- `acpi_dev_put(adev)` after `ACPI_COMPANION_SET()` balances only that
lookup reference
- the later `acpi_unbind_one()` path would not be putting the same
reference again, because that put is for the separate ref taken by
`acpi_bind_one()`
Part of why I leaned that way is that I found a couple of in-tree examples
that seem to follow the same pattern:
- `drivers/platform/x86/x86-android-tablets/core.c`
does `acpi_dev_get_first_match_dev()`, `ACPI_COMPANION_SET()`, then
`acpi_dev_put()`
- `drivers/acpi/arm64/mpam.c`
does `acpi_dev_get_first_match_dev()`, `ACPI_COMPANION_SET()`, then
`acpi_dev_put()`
So from my own understanding, those examples also seem to treat
`ACPI_COMPANION_SET()` as not consuming the reference returned by
`acpi_dev_get_first_match_dev()`.
But this is only my reading of the current ownership flow, so if I am
overlooking some rule around manually assigned companions I am happy to
re-check.
Best regards,
Shuhao
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH 1/2] ALSA: hda: cs35l56: Put ACPI device after setting companion
2026-04-28 10:40 ` [PATCH 1/2] ALSA: hda: cs35l56: Put ACPI device after setting companion Shuhao Fu
@ 2026-05-06 14:37 ` Simon Trimmer
0 siblings, 0 replies; 7+ messages in thread
From: Simon Trimmer @ 2026-05-06 14:37 UTC (permalink / raw)
To: 'Shuhao Fu', 'David Rhodes',
'Richard Fitzgerald'
Cc: 'Jaroslav Kysela', 'Takashi Iwai', linux-sound,
patches, linux-kernel
On 28/04/2026 11:41 am, Shuhao Fu wrote:
> Hi Richard,
>
> > Are you sure about this?
> > I remember when I wrote this code I checked the driver core and saw that
> > if there is a companion it puts it when the driver is removed.
> > That is why I didn't put the reference here, it would have caused a
> > double put.
>
> I may well be missing something here. But from my reading of the current
> code, it does not seem to cause a double put.
>
> The place where I do seem to find ACPI companion cleanup is when the
> device
> object itself is deleted/unregistered:
>
> `device_del()`
> -> `device_platform_notify_remove()`
> -> `acpi_device_notify_remove()`
> -> `acpi_unbind_one()`
>
> What makes me think this is not the matching put for
> `acpi_dev_get_first_match_dev()` is that `acpi_unbind_one()` only calls
> `acpi_dev_put()` after it finds a matching entry for the device in
> `acpi_dev->physical_node_list`.
>
> As far as I can tell, that list entry is created by `acpi_bind_one()`,
which
> also takes its own extra reference with `acpi_dev_get(acpi_dev)`. So the
put
> in `acpi_unbind_one()` looks to me like it is paired with that
> `acpi_bind_one()` reference, rather than with the earlier
> `acpi_dev_get_first_match_dev()` lookup.
>
> If that reading is right, then I think the ownership looks like this:
>
> - `ACPI_COMPANION_SET()` only attaches the companion pointer/fwnode
> - the lookup reference from `acpi_dev_get_first_match_dev()` is still with
> the caller
> - `acpi_dev_put(adev)` after `ACPI_COMPANION_SET()` balances only that
> lookup reference
> - the later `acpi_unbind_one()` path would not be putting the same
> reference again, because that put is for the separate ref taken by
> `acpi_bind_one()`
>
> Part of why I leaned that way is that I found a couple of in-tree examples
> that seem to follow the same pattern:
>
> - `drivers/platform/x86/x86-android-tablets/core.c`
> does `acpi_dev_get_first_match_dev()`, `ACPI_COMPANION_SET()`, then
> `acpi_dev_put()`
>
> - `drivers/acpi/arm64/mpam.c`
> does `acpi_dev_get_first_match_dev()`, `ACPI_COMPANION_SET()`, then
> `acpi_dev_put()`
>
> So from my own understanding, those examples also seem to treat
> `ACPI_COMPANION_SET()` as not consuming the reference returned by
> `acpi_dev_get_first_match_dev()`.
>
> But this is only my reading of the current ownership flow, so if I am
> overlooking some rule around manually assigned companions I am happy to
> re-check.
>
> Best regards,
> Shuhao
Tested-by: Simon Trimmer <simont@opensource.cirrus.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 0/2] ALSA: hda: Fix Cirrus ACPI device reference leaks
2026-04-28 7:44 [PATCH 0/2] ALSA: hda: Fix Cirrus ACPI device reference leaks Shuhao Fu
` (2 preceding siblings ...)
2026-04-28 10:40 ` [PATCH 1/2] ALSA: hda: cs35l56: Put ACPI device after setting companion Shuhao Fu
@ 2026-05-07 14:32 ` Takashi Iwai
3 siblings, 0 replies; 7+ messages in thread
From: Takashi Iwai @ 2026-05-07 14:32 UTC (permalink / raw)
To: Shuhao Fu
Cc: David Rhodes, Richard Fitzgerald, Jaroslav Kysela, Takashi Iwai,
linux-sound, patches, linux-kernel
On Tue, 28 Apr 2026 09:44:15 +0200,
Shuhao Fu wrote:
>
> This series fixes two missing acpi_dev_put() cases in Cirrus HDA side-codec
> ACPI lookup paths.
>
> Patch 1 drops the lookup reference after synthesizing the companion in
> cs35l56_hda_read_acpi().
>
> Patch 2 drops the lookup reference on the early !physdev error path in
> cs35l41_hda_read_acpi().
>
> Shuhao Fu (2):
> ALSA: hda: cs35l56: Put ACPI device after setting companion
> ALSA: hda: cs35l41: Fix ACPI device leak on missing physical node
Assuming that the logic is correct, I applied both now.
thanks,
Takashi
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-05-07 14:32 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-28 7:44 [PATCH 0/2] ALSA: hda: Fix Cirrus ACPI device reference leaks Shuhao Fu
2026-04-28 8:01 ` [PATCH 1/2] ALSA: hda: cs35l56: Put ACPI device after setting companion Shuhao Fu
2026-04-28 9:05 ` Richard Fitzgerald
2026-04-28 8:12 ` [PATCH 2/2] ALSA: hda: cs35l41: Put ACPI device on missing physical node Shuhao Fu
2026-04-28 10:40 ` [PATCH 1/2] ALSA: hda: cs35l56: Put ACPI device after setting companion Shuhao Fu
2026-05-06 14:37 ` Simon Trimmer
2026-05-07 14:32 ` [PATCH 0/2] ALSA: hda: Fix Cirrus ACPI device reference leaks Takashi Iwai
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.