ASoC: Intel: avs: Add NULL check in avs_component_probe()

[PATCH v1] ASoC: Intel: avs: Add NULL check in avs_component_probe()

Posted by Henry Martin 2 days, 20 hours ago

devm_kasprintf() returns NULL when memory allocation fails. Currently,
avs_component_probe() does not check for this case, which results in a
NULL pointer dereference.

Add NULL check after devm_kasprintf() to prevent this issue.

Fixes: 739c031110da ("ASoC: Intel: avs: Provide support for fallback topology")
Signed-off-by: Henry Martin <bsdhenrymartin@gmail.com>
---
 sound/soc/intel/avs/pcm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/sound/soc/intel/avs/pcm.c b/sound/soc/intel/avs/pcm.c
index dac463390da1..7072bcf4e56f 100644
--- a/sound/soc/intel/avs/pcm.c
+++ b/sound/soc/intel/avs/pcm.c
@@ -927,7 +927,8 @@ static int avs_component_probe(struct snd_soc_component *component)
 		else
 			mach->tplg_filename = devm_kasprintf(adev->dev, GFP_KERNEL,
 							     "hda-generic-tplg.bin");
-
+		if (!mach->tplg_filename)
+			return -ENOMEM;
 		filename = kasprintf(GFP_KERNEL, "%s/%s", component->driver->topology_name_prefix,
 				     mach->tplg_filename);
 		if (!filename)
-- 
2.34.1

Re: [PATCH v1] ASoC: Intel: avs: Add NULL check in avs_component_probe()

Posted by Cezary Rojewski 1 day, 20 hours ago

On 2025-04-01 4:32 PM, Henry Martin wrote:
> devm_kasprintf() returns NULL when memory allocation fails. Currently,
> avs_component_probe() does not check for this case, which results in a
> NULL pointer dereference.
> 
> Add NULL check after devm_kasprintf() to prevent this issue.

This basically repeats the title, I'd suggest to drop this very line. In 
regard to the title, I believe:

	ASoC: Intel: avs: Fix null-ptr-deref in avs_component_probe()

is more straightforward. That's what you're doing here afterall, fixing 
stuff.

> 
> Fixes: 739c031110da ("ASoC: Intel: avs: Provide support for fallback topology")
> Signed-off-by: Henry Martin <bsdhenrymartin@gmail.com>
Good catch, thanks for sending the fix, Martin. I'm rather strict when 
it comes to wording title/message but nothing more than a nitpick in 
this case.

Whether you would be willing to provide v2 or not, feel free to add my tag:

Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>

> ---
>   sound/soc/intel/avs/pcm.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/sound/soc/intel/avs/pcm.c b/sound/soc/intel/avs/pcm.c
> index dac463390da1..7072bcf4e56f 100644
> --- a/sound/soc/intel/avs/pcm.c
> +++ b/sound/soc/intel/avs/pcm.c
> @@ -927,7 +927,8 @@ static int avs_component_probe(struct snd_soc_component *component)
>   		else
>   			mach->tplg_filename = devm_kasprintf(adev->dev, GFP_KERNEL,
>   							     "hda-generic-tplg.bin");
> -
> +		if (!mach->tplg_filename)
> +			return -ENOMEM;
>   		filename = kasprintf(GFP_KERNEL, "%s/%s", component->driver->topology_name_prefix,
>   				     mach->tplg_filename);
>   		if (!filename)

[PATCH v2] ASoC: Intel: avs: Fix null-ptr-deref in avs_component_probe()

Posted by Henry Martin 1 day, 20 hours ago

devm_kasprintf() returns NULL when memory allocation fails. Currently,
avs_component_probe() does not check for this case, which results in a
NULL pointer dereference.

Fixes: 739c031110da ("ASoC: Intel: avs: Provide support for fallback topology")
Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
Reviewed-by: Ethan Carter Edwards <ethan@ethancedwards.com>
Signed-off-by: Henry Martin <bsdhenrymartin@gmail.com>
---
V1 -> V2: No functional changes, just correct title and redundant commit
message.

 sound/soc/intel/avs/pcm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/sound/soc/intel/avs/pcm.c b/sound/soc/intel/avs/pcm.c
index dac463390da1..7072bcf4e56f 100644
--- a/sound/soc/intel/avs/pcm.c
+++ b/sound/soc/intel/avs/pcm.c
@@ -927,7 +927,8 @@ static int avs_component_probe(struct snd_soc_component *component)
 		else
 			mach->tplg_filename = devm_kasprintf(adev->dev, GFP_KERNEL,
 							     "hda-generic-tplg.bin");
-
+		if (!mach->tplg_filename)
+			return -ENOMEM;
 		filename = kasprintf(GFP_KERNEL, "%s/%s", component->driver->topology_name_prefix,
 				     mach->tplg_filename);
 		if (!filename)
-- 
2.34.1

Re: [PATCH v2] ASoC: Intel: avs: Fix null-ptr-deref in avs_component_probe()

Posted by Markus Elfring 1 day, 15 hours ago

> devm_kasprintf() returns NULL when memory allocation fails. Currently,
…
                                                       failed?

Can a summary phrase like “Prevent null pointer dereference
in avs_component_probe()” a bit nicer?


…
> +++ b/sound/soc/intel/avs/pcm.c
> @@ -927,7 +927,8 @@ static int avs_component_probe(struct snd_soc_component *component)
>  		else
>  			mach->tplg_filename = devm_kasprintf(adev->dev, GFP_KERNEL,
>  							     "hda-generic-tplg.bin");
> -
> +		if (!mach->tplg_filename)
> +			return -ENOMEM;

Can a blank line be desirable after such a statement?


Would another source code transformation become helpful with an additional update step?

-		if (((vendor_id >> 16) & 0xFFFF) == 0x8086)
-			mach->tplg_filename = devm_kasprintf(adev->dev, GFP_KERNEL,
-							     "hda-8086-generic-tplg.bin");
-		else
-			mach->tplg_filename = devm_kasprintf(adev->dev, GFP_KERNEL,
-							     "hda-generic-tplg.bin");
+		mach->tplg_filename = devm_kasprintf(adev->dev, GFP_KERNEL,
+						     (((vendor_id >> 16) & 0xFFFF) == 0x8086)
+						     ? "hda-8086-generic-tplg.bin"
+						     : "hda-generic-tplg.bin");


Regards,
Markus

Re: [PATCH v2] ASoC: Intel: avs: Fix null-ptr-deref in avs_component_probe()

Posted by Mark Brown 1 day, 14 hours ago

On Wed, Apr 02, 2025 at 09:18:27PM +0200, Markus Elfring wrote:
> > devm_kasprintf() returns NULL when memory allocation fails. Currently,
> …
>                                                        failed?
> 
> Can a summary phrase like “Prevent null pointer dereference
> in avs_component_probe()” a bit nicer?

Feel free to ignore Markus, he has a long history of sending
unhelpful review comments and continues to ignore repeated requests
to stop.

Re: [PATCH] ASoC: Intel: avs: Add NULL check in avs_component_probe()

Posted by Markus Elfring 1 day, 22 hours ago

> devm_kasprintf() returns NULL when memory allocation fails. Currently,
…
                call?                                  failed?


> Add NULL check after devm_kasprintf() to prevent this issue.

Do you generally propose here to improve the error handling?


…
> +++ b/sound/soc/intel/avs/pcm.c
> @@ -927,7 +927,8 @@ static int avs_component_probe(struct snd_soc_component *component)
>  		else
>  			mach->tplg_filename = devm_kasprintf(adev->dev, GFP_KERNEL,
>  							     "hda-generic-tplg.bin");
> -
> +		if (!mach->tplg_filename)
> +			return -ENOMEM;

Can a blank line be desirable after such a statement?


Would another source code transformation become helpful with an additional update step?

-		if (((vendor_id >> 16) & 0xFFFF) == 0x8086)
-			mach->tplg_filename = devm_kasprintf(adev->dev, GFP_KERNEL,
-							     "hda-8086-generic-tplg.bin");
-		else
-			mach->tplg_filename = devm_kasprintf(adev->dev, GFP_KERNEL,
-							     "hda-generic-tplg.bin");
+		mach->tplg_filename = devm_kasprintf(adev->dev, GFP_KERNEL,
+						     (((vendor_id >> 16) & 0xFFFF) == 0x8086)
+						     ? "hda-8086-generic-tplg.bin"
+						     : "hda-generic-tplg.bin");


Regards,
Markus

Re: [PATCH] ASoC: Intel: avs: Add NULL check in avs_component_probe()

Posted by Mark Brown 1 day, 20 hours ago

On Wed, Apr 02, 2025 at 02:00:31PM +0200, Markus Elfring wrote:
> > devm_kasprintf() returns NULL when memory allocation fails. Currently,
> …
>                 call?                                  failed?
> 
> 
> > Add NULL check after devm_kasprintf() to prevent this issue.
> 
> Do you generally propose here to improve the error handling?

Feel free to ignore Markus, he has a long history of sending
unhelpful review comments and continues to ignore repeated requests
to stop.

Re: [PATCH] ASoC: Intel: avs: Add NULL check in avs_component_probe()

Posted by Ethan Carter Edwards 1 day, 21 hours ago

On 25/04/02 02:00PM, Markus Elfring wrote:
> > devm_kasprintf() returns NULL when memory allocation fails. Currently,
> …
>                 call?                                  failed?
> 
> 
> > Add NULL check after devm_kasprintf() to prevent this issue.
> 
> Do you generally propose here to improve the error handling?
> 
> 
> …
> > +++ b/sound/soc/intel/avs/pcm.c
> > @@ -927,7 +927,8 @@ static int avs_component_probe(struct snd_soc_component *component)
> >  		else
> >  			mach->tplg_filename = devm_kasprintf(adev->dev, GFP_KERNEL,
> >  							     "hda-generic-tplg.bin");
> > -
> > +		if (!mach->tplg_filename)
> > +			return -ENOMEM;
> 
> Can a blank line be desirable after such a statement?
> 
> 
> Would another source code transformation become helpful with an additional update step?
> 
> -		if (((vendor_id >> 16) & 0xFFFF) == 0x8086)
> -			mach->tplg_filename = devm_kasprintf(adev->dev, GFP_KERNEL,
> -							     "hda-8086-generic-tplg.bin");
> -		else
> -			mach->tplg_filename = devm_kasprintf(adev->dev, GFP_KERNEL,
> -							     "hda-generic-tplg.bin");
> +		mach->tplg_filename = devm_kasprintf(adev->dev, GFP_KERNEL,
> +						     (((vendor_id >> 16) & 0xFFFF) == 0x8086)
> +						     ? "hda-8086-generic-tplg.bin"
> +						     : "hda-generic-tplg.bin");
> 
> 
> Regards,
> Markus


Please feel free to ignore Markus. He has a tendency to provide unclear
and nonsense feedback. He has done this several times with my patch
sets. I almost wonder if he is an internet troll.

In any case, this change makes sense to me.

Reviewed-by: Ethan Carter Edwards <ethan@ethancedwards.com>

Re: ASoC: Intel: avs: Add NULL check in avs_component_probe()

Posted by Markus Elfring 1 day, 20 hours ago

> Please feel free to ignore Markus. He has a tendency to provide unclear
> and nonsense feedback. He has done this several times with my patch
> sets. I almost wonder if he is an internet troll.
Can any of the published contributions change your mind?

Regards,
Markus