| Message ID | 20250401143222.30344-1-bsdhenrymartin@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v1] ASoC: Intel: avs: Add NULL check in avs_component_probe() | expand |
> 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
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>
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)
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.
> 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
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)
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(-)