[PATCH] ASoC: core: add snd_BUG_ON() check in devm_snd_soc_register_card()

[PATCH] ASoC: core: add snd_BUG_ON() check in devm_snd_soc_register_card()

[phucduc.bui]

From: bui duc phuc <phucduc.bui@gmail.com>

card is dereferenced immediately, so passing NULL results in a NULL
pointer dereference. A NULL dev is silently accepted, defeating the
documented automatic-unregistration semantics of this devm variant.

Add snd_BUG_ON() checks on both arguments to catch API misuse early.

Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
 sound/soc/soc-devres.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/sound/soc/soc-devres.c b/sound/soc/soc-devres.c
index 718165ba84ac..3e946196022a 100644
--- a/sound/soc/soc-devres.c
+++ b/sound/soc/soc-devres.c
@@ -59,6 +59,9 @@ EXPORT_SYMBOL_GPL(devm_snd_soc_register_component);
  */
 int devm_snd_soc_register_card(struct device *dev, struct snd_soc_card *card)
 {
+	if (snd_BUG_ON(!dev || !card))
+		return -EINVAL;
+
 	card->devres_dev = dev;
 	return snd_soc_register_card(card);
 }
-- 
2.43.0

Re: [PATCH] ASoC: core: add snd_BUG_ON() check in devm_snd_soc_register_card()

[Cezary Rojewski]

On 8/3/2026 12:27 PM, phucduc.bui@gmail.com wrote:
> From: bui duc phuc <phucduc.bui@gmail.com>
> 
> card is dereferenced immediately, so passing NULL results in a NULL
> pointer dereference. A NULL dev is silently accepted, defeating the
> documented automatic-unregistration semantics of this devm variant.
> 
> Add snd_BUG_ON() checks on both arguments to catch API misuse early.
> 
> Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
> ---
>   sound/soc/soc-devres.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/sound/soc/soc-devres.c b/sound/soc/soc-devres.c
> index 718165ba84ac..3e946196022a 100644
> --- a/sound/soc/soc-devres.c
> +++ b/sound/soc/soc-devres.c
> @@ -59,6 +59,9 @@ EXPORT_SYMBOL_GPL(devm_snd_soc_register_component);
>    */
>   int devm_snd_soc_register_card(struct device *dev, struct snd_soc_card *card)
>   {
> +	if (snd_BUG_ON(!dev || !card))
> +		return -EINVAL;
> +
>   	card->devres_dev = dev;
>   	return snd_soc_register_card(card);
>   }

Not sure whether we need such defenses. If we are to follow such 
approach, many ASoC-driver APIs require BUG_ON/WARN_ON() update.

register_card() is typically called as a last step, once 'card' is 
already allocated. At the same time, the register_card() typically 
occurs in probe() context of given device so we're always certain the 
'dev' is not NULL.

TLDR: this is not a UAPI, (obvious) NULL-checks can be skipped.

Re: [PATCH] ASoC: core: add snd_BUG_ON() check in devm_snd_soc_register_card()

[Bui Duc Phuc]

Hi Cezary,

Thank you for your review.

>
> Not sure whether we need such defenses. If we are to follow such
> approach, many ASoC-driver APIs require BUG_ON/WARN_ON() update.
>
> register_card() is typically called as a last step, once 'card' is
> already allocated. At the same time, the register_card() typically
> occurs in probe() context of given device so we're always certain the
> 'dev' is not NULL.
>
> TLDR: this is not a UAPI, (obvious) NULL-checks can be skipped.

I completely understand where you're coming from.
My thinking was that this function is referenced quite widely
(currently in 112 files).
Given how frequently it is used, I felt that adding a simple defensive
check here
could make the API a little more robust and help avoid potential
NULL pointer dereferences for both existing and future callers.

I wasn't suggesting that every ASoC API should be updated this way.
I was only thinking of APIs that are used very frequently, especially
those commonly called from probe paths.

Also, it's not always obvious to callers what assumptions a core API
relies on or
the context in which it is expected to be used.

That was simply the motivation behind this patch.

Best regards,
Phuc

Re: [PATCH] ASoC: core: add snd_BUG_ON() check in devm_snd_soc_register_card()

[Cezary Rojewski]

On 8/3/2026 1:58 PM, Bui Duc Phuc wrote:

>> Not sure whether we need such defenses. If we are to follow such
>> approach, many ASoC-driver APIs require BUG_ON/WARN_ON() update.
>>
>> register_card() is typically called as a last step, once 'card' is
>> already allocated. At the same time, the register_card() typically
>> occurs in probe() context of given device so we're always certain the
>> 'dev' is not NULL.
>>
>> TLDR: this is not a UAPI, (obvious) NULL-checks can be skipped.
> 
> I completely understand where you're coming from.
> My thinking was that this function is referenced quite widely
> (currently in 112 files).
> Given how frequently it is used, I felt that adding a simple defensive
> check here
> could make the API a little more robust and help avoid potential
> NULL pointer dereferences for both existing and future callers.
> 
> I wasn't suggesting that every ASoC API should be updated this way.
> I was only thinking of APIs that are used very frequently, especially
> those commonly called from probe paths.
> 
> Also, it's not always obvious to callers what assumptions a core API
> relies on or
> the context in which it is expected to be used.
> 
> That was simply the motivation behind this patch.
snd_BUG_ON() translates to WARN_ON() in debug conditions.  After reading 
the macro documentation, pr_xxx() or equivalents are recommended when 
dealing with invalid arguments.  Same results after grepping for WARNs 
in sound/soc or in general references such as driver/base/core.c - it's 
clear to me the macro is not used as plain null-arg-check.

Updating just one function with the check raises the question when 
reading the file - Why just one devm_snd_xxx() has the check and the 
rest do not?


Kind regards,
Czarek

Re: [PATCH] ASoC: core: add snd_BUG_ON() check in devm_snd_soc_register_card()

[Bui Duc Phuc]

Hi Cezary,

Thank you for your feedback.

> snd_BUG_ON() translates to WARN_ON() in debug conditions.  After reading
> the macro documentation, pr_xxx() or equivalents are recommended when
> dealing with invalid arguments.  Same results after grepping for WARNs
> in sound/soc or in general references such as driver/base/core.c - it's
> clear to me the macro is not used as plain null-arg-check.
>

I couldn't find anything in the snd_BUG_ON() documentation that
explicitly says the
macro should not be used for plain NULL argument checks.

Also, there are quite a few places in the sound subsystem where
WARN_ON() is used to
validate NULL arguments, for example:

https://elixir.bootlin.com/linux/v7.2-rc5/source/sound/usb/endpoint.c#L857

https://elixir.bootlin.com/linux/v7.2-rc5/source/sound/soc/ti/davinci-mcasp.c#L2476

https://elixir.bootlin.com/linux/v7.2-rc5/source/sound/soc/meson/meson-codec-glue.c#L59

and even in the Intel code that you maintain:

https://elixir.bootlin.com/linux/v7.2-rc5/source/sound/soc/intel/atom/sst-mfld-platform-pcm.c#L31

That said, I'm also fine with not using snd_BUG_ON() here. Replacing
it with a simple argument check such as:

if (!dev || !card) {
      pr_err("invalid arguments\n");
      return -EINVAL;
}

is perfectly reasonable to me. It's not a big issue either way.

> Updating just one function with the check raises the question when
> reading the file - Why just one devm_snd_xxx() has the check and the
> rest do not?
>

I think the remaining helper functions can be updated gradually over time.

To me, this discussion itself is a good example. You and I interpreted
the intended use of snd_BUG_ON() differently.
I don't think the macro is wrong; rather, its intended usage is not
clearly documented or consistently reflected by existing code.
Therefore, I think it would be beneficial to either improve the API
documentation to clarify the intended usage,
or add explicit argument validation, such as the check I proposed for
devm_snd_soc_register_card().
Either approach would help avoid misunderstandings and make the API
easier to use correctly.

Of course, that's just my opinion. If the SOUND subsystem does not
intend to make these APIs more explicit or easier to use,
that's perfectly fine as well. I'll follow the preferred direction.

Best regards,
Phuc

Re: [PATCH] ASoC: core: add snd_BUG_ON() check in devm_snd_soc_register_card()

[Cezary Rojewski]

On 8/4/2026 5:19 AM, Bui Duc Phuc wrote:

>> snd_BUG_ON() translates to WARN_ON() in debug conditions.  After reading
>> the macro documentation, pr_xxx() or equivalents are recommended when
>> dealing with invalid arguments.  Same results after grepping for WARNs
>> in sound/soc or in general references such as driver/base/core.c - it's
>> clear to me the macro is not used as plain null-arg-check.
>>
> 
> I couldn't find anything in the snd_BUG_ON() documentation that
> explicitly says the
> macro should not be used for plain NULL argument checks.

My message reads: translates to WARN_ON() (..)
And WARN_ON's doc is what I meant.

> Also, there are quite a few places in the sound subsystem where
> WARN_ON() is used to
> validate NULL arguments, for example:
> 
> https://elixir.bootlin.com/linux/v7.2-rc5/source/sound/usb/endpoint.c#L857
> 
> https://elixir.bootlin.com/linux/v7.2-rc5/source/sound/soc/ti/davinci-mcasp.c#L2476
> 
> https://elixir.bootlin.com/linux/v7.2-rc5/source/sound/soc/meson/meson-codec-glue.c#L59

Nah, these are not plain null checks and if I missed something and any 
of them are, that's a patch candidate.

> and even in the Intel code that you maintain:
> 
> https://elixir.bootlin.com/linux/v7.2-rc5/source/sound/soc/intel/atom/sst-mfld-platform-pcm.c#L31

That's a clear example of when _not to_ use WARN_ON(), good catch. :)
Maintain != reviewed/acked. Many maintainers maintain code that they 
inherited and were not there when it was merged.

> That said, I'm also fine with not using snd_BUG_ON() here. Replacing
> it with a simple argument check such as:
> 
> if (!dev || !card) {
>        pr_err("invalid arguments\n");
>        return -EINVAL;
> }
> 
> is perfectly reasonable to me. It's not a big issue either way.

While not a fan, still better than WARN_ON().

>> Updating just one function with the check raises the question when
>> reading the file - Why just one devm_snd_xxx() has the check and the
>> rest do not?
>>
> 
> I think the remaining helper functions can be updated gradually over time.
> 
> To me, this discussion itself is a good example. You and I interpreted
> the intended use of snd_BUG_ON() differently.
> I don't think the macro is wrong; rather, its intended usage is not
> clearly documented or consistently reflected by existing code.

Not a fan of snd_BUG_ON() in general, I'd rather have it removed and 
WARN_ON(s) updated is one believes something is missing or wrong with them.

Anyhow, it's up to Mark. I'd avoid selecting just one function and 
filling it with null-arg-checks.

Re: [PATCH] ASoC: core: add snd_BUG_ON() check in devm_snd_soc_register_card()

[Bui Duc Phuc]

Hi Cezary,

>
> My message reads: translates to WARN_ON() (..)
> And WARN_ON's doc is what I meant.
>

Ah, I see. I had interpreted your comment differently.
Thanks for the clarification.

Best regards,
Phuc