[PATCH v3 1/2] ASoC: samsung: tm2_wm5110: check of of_parse return value

[PATCH v3 1/2] ASoC: samsung: tm2_wm5110: check of of_parse return value

[Pierre-Louis Bossart]

cppcheck warning:

sound/soc/samsung/tm2_wm5110.c:605:6: style: Variable 'ret' is
reassigned a value before the old one has been
used. [redundantAssignment]
 ret = devm_snd_soc_register_component(dev, &tm2_component,
     ^
sound/soc/samsung/tm2_wm5110.c:554:7: note: ret is assigned
  ret = of_parse_phandle_with_args(dev->of_node, "i2s-controller",
      ^
sound/soc/samsung/tm2_wm5110.c:605:6: note: ret is overwritten
 ret = devm_snd_soc_register_component(dev, &tm2_component,
     ^

The args is a stack variable, so it could have junk (uninitialized)
therefore args.np could have a non-NULL and random value even though
property was missing. Later could trigger invalid pointer dereference.

There's no need to check for args.np because args.np won't be
initialized on errors.

Fixes: 8d1513cef51a ("ASoC: samsung: Add support for HDMI audio on TM2board")
Cc: <stable@vger.kernel.org>
Suggested-by: Krzysztof Kozlowski <krzk@kernel.org>
Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/samsung/tm2_wm5110.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/samsung/tm2_wm5110.c b/sound/soc/samsung/tm2_wm5110.c
index 9300fef9bf26..125e07f65d2b 100644
--- a/sound/soc/samsung/tm2_wm5110.c
+++ b/sound/soc/samsung/tm2_wm5110.c
@@ -553,7 +553,7 @@ static int tm2_probe(struct platform_device *pdev)
 
 		ret = of_parse_phandle_with_args(dev->of_node, "i2s-controller",
 						 cells_name, i, &args);
-		if (!args.np) {
+		if (ret) {
 			dev_err(dev, "i2s-controller property parse error: %d\n", i);
 			ret = -EINVAL;
 			goto dai_node_put;
-- 
2.25.1

Re: [PATCH v3 1/2] ASoC: samsung: tm2_wm5110: check of of_parse return value

[Krzysztof Kozlowski]

On 11/03/2021 01:35, Pierre-Louis Bossart wrote:
> cppcheck warning:
> 
> sound/soc/samsung/tm2_wm5110.c:605:6: style: Variable 'ret' is
> reassigned a value before the old one has been
> used. [redundantAssignment]
>  ret = devm_snd_soc_register_component(dev, &tm2_component,
>      ^
> sound/soc/samsung/tm2_wm5110.c:554:7: note: ret is assigned
>   ret = of_parse_phandle_with_args(dev->of_node, "i2s-controller",
>       ^
> sound/soc/samsung/tm2_wm5110.c:605:6: note: ret is overwritten
>  ret = devm_snd_soc_register_component(dev, &tm2_component,
>      ^
> 
> The args is a stack variable, so it could have junk (uninitialized)
> therefore args.np could have a non-NULL and random value even though
> property was missing. Later could trigger invalid pointer dereference.
> 
> There's no need to check for args.np because args.np won't be
> initialized on errors.
> 
> Fixes: 8d1513cef51a ("ASoC: samsung: Add support for HDMI audio on TM2board")
> Cc: <stable@vger.kernel.org>
> Suggested-by: Krzysztof Kozlowski <krzk@kernel.org>
> Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> ---
>  sound/soc/samsung/tm2_wm5110.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>


Best regards,
Krzysztof

Re: [PATCH v3 1/2] ASoC: samsung: tm2_wm5110: check of of_parse return value

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 483 bytes --]

On Wed, Mar 10, 2021 at 06:35:15PM -0600, Pierre-Louis Bossart wrote:

> Fixes: 8d1513cef51a ("ASoC: samsung: Add support for HDMI audio on TM2board")
> Cc: <stable@vger.kernel.org>

Commit: 11bc3bb24003 ("ASoC: samsung: tm2_wm5110: check of of_parse return value")
	Fixes tag: Fixes: 8d1513cef51a ("ASoC: samsung: Add support for HDMI audio on TM2board")
	Has these problem(s):
		- Subject does not match target commit subject
		  Just use
			git log -1 --format='Fixes: %h ("%s")'

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v3 1/2] ASoC: samsung: tm2_wm5110: check of of_parse return value

[Pierre-Louis Bossart]

On 3/12/21 8:28 AM, Mark Brown wrote:
> On Wed, Mar 10, 2021 at 06:35:15PM -0600, Pierre-Louis Bossart wrote:
> 
>> Fixes: 8d1513cef51a ("ASoC: samsung: Add support for HDMI audio on TM2board")
>> Cc: <stable@vger.kernel.org>
> 
> Commit: 11bc3bb24003 ("ASoC: samsung: tm2_wm5110: check of of_parse return value")
> 	Fixes tag: Fixes: 8d1513cef51a ("ASoC: samsung: Add support for HDMI audio on TM2board")
> 	Has these problem(s):
> 		- Subject does not match target commit subject
> 		  Just use
> 			git log -1 --format='Fixes: %h ("%s")'
> 

Sorry, I don't know what to make of this. I don't see this commit 
11bc3bb24003

Something odd happened, there was an initial merge and it seems to have 
disappeared, it's no longer in the for-next branch?

Re: [PATCH v3 1/2] ASoC: samsung: tm2_wm5110: check of of_parse return value

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 740 bytes --]

On Fri, Mar 12, 2021 at 10:30:32AM -0600, Pierre-Louis Bossart wrote:
> On 3/12/21 8:28 AM, Mark Brown wrote:

> > Commit: 11bc3bb24003 ("ASoC: samsung: tm2_wm5110: check of of_parse return value")
> > 	Fixes tag: Fixes: 8d1513cef51a ("ASoC: samsung: Add support for HDMI audio on TM2board")
> > 	Has these problem(s):
> > 		- Subject does not match target commit subject
> > 		  Just use
> > 			git log -1 --format='Fixes: %h ("%s")'

> Sorry, I don't know what to make of this. I don't see this commit
> 11bc3bb24003

> Something odd happened, there was an initial merge and it seems to have
> disappeared, it's no longer in the for-next branch?

That commit is your patch being applied, which I've dropped because of
the error reported.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v3 1/2] ASoC: samsung: tm2_wm5110: check of of_parse return value

[Pierre-Louis Bossart]

On 3/12/21 10:31 AM, Mark Brown wrote:
> On Fri, Mar 12, 2021 at 10:30:32AM -0600, Pierre-Louis Bossart wrote:
>> On 3/12/21 8:28 AM, Mark Brown wrote:
> 
>>> Commit: 11bc3bb24003 ("ASoC: samsung: tm2_wm5110: check of of_parse return value")
>>> 	Fixes tag: Fixes: 8d1513cef51a ("ASoC: samsung: Add support for HDMI audio on TM2board")
>>> 	Has these problem(s):
>>> 		- Subject does not match target commit subject
>>> 		  Just use
>>> 			git log -1 --format='Fixes: %h ("%s")'
> 
>> Sorry, I don't know what to make of this. I don't see this commit
>> 11bc3bb24003
> 
>> Something odd happened, there was an initial merge and it seems to have
>> disappeared, it's no longer in the for-next branch?
> 
> That commit is your patch being applied, which I've dropped because of
> the error reported.

ack, not sure why there's a missing space.

git log -1 --format='Fixes: %h ("%s")' 8d1513cef51a
Fixes: 8d1513cef51a ("ASoC: samsung: Add support for HDMI audio on TM2 
board")