[PATCH] ASoC: Add a sanity check before using dai driver name

[PATCH] ASoC: Add a sanity check before using dai driver name

[Jeffy Chen]

The dai driver's name is allowed to be NULL. So add a sanity check for
that.

Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
Reported-by: Donglin Peng <dolinux.peng@gmail.com>
---

 sound/soc/soc-core.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 77e7e2a11af0..8ab9ee2b460a 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -978,11 +978,13 @@ struct snd_soc_dai *snd_soc_find_dai(
 		if (dlc->name && strcmp(component->name, dlc->name))
 			continue;
 		list_for_each_entry(dai, &component->dai_list, list) {
-			if (dlc->dai_name && strcmp(dai->name, dlc->dai_name)
-			    && strcmp(dai->driver->name, dlc->dai_name))
-				continue;
-
-			return dai;
+			if (!dlc->dai_name)
+				return dai;
+			if (!strcmp(dai->name, dlc->dai_name))
+				return dai;
+			if (dai->driver->name &&
+			    !strcmp(dai->driver->name, dlc->dai_name))
+				return dai;
 		}
 	}
 
-- 
2.11.0

Re: [PATCH] ASoC: Add a sanity check before using dai driver name

[Mark Brown]

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

On Tue, Aug 22, 2017 at 10:45:12PM +0800, Jeffy Chen wrote:

> -			return dai;
> +			if (!dlc->dai_name)
> +				return dai;
> +			if (!strcmp(dai->name, dlc->dai_name))
> +				return dai;

You want (dlc->dai_name && !strcmp(dai->name, dlc->dai_name)) for this
to be equivalent don't you?

> +			if (dai->driver->name &&
> +			    !strcmp(dai->driver->name, dlc->dai_name))


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

Re: [PATCH] ASoC: Add a sanity check before using dai driver name

[jeffy]

hi Mark,

On 08/23/2017 12:17 AM, Mark Brown wrote:
> On Tue, Aug 22, 2017 at 10:45:12PM +0800, Jeffy Chen wrote:
>
>> -			return dai;
>> +			if (!dlc->dai_name)
>> +				return dai;
>> +			if (!strcmp(dai->name, dlc->dai_name))
>> +				return dai;
>
> You want (dlc->dai_name && !strcmp(dai->name, dlc->dai_name)) for this
> to be equivalent don't you?
i think the original check is allowing NULL dlc dai_name to be a match...
so we basically did:
reject when dlc dai_name is valid, but not match the dai name

and my patch is:
accept when dlc dai_name is invalid
accept when match dai name
accept when match dai driver name(only when it is valid)

so it's a "if (a && b) reject" to "if (!a || !b) accept" case..

>
>> +			if (dai->driver->name &&
>> +			    !strcmp(dai->driver->name, dlc->dai_name))
>

Re: [PATCH] ASoC: Add a sanity check before using dai driver name

[Mark Brown]

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

On Wed, Aug 23, 2017 at 07:42:55AM +0800, jeffy wrote:

> i think the original check is allowing NULL dlc dai_name to be a match...
> so we basically did:
> reject when dlc dai_name is valid, but not match the dai name

So it is, but this still looks like the wrong thing - it'll match on an
empty DAI name over an explicit match on the driver name which seems
like it's the wrong way round.

> 
> and my patch is:
> accept when dlc dai_name is invalid
> accept when match dai name
> accept when match dai driver name(only when it is valid)
> 
> so it's a "if (a && b) reject" to "if (!a || !b) accept" case..
> 
> > 
> > > +			if (dai->driver->name &&
> > > +			    !strcmp(dai->driver->name, dlc->dai_name))
> > 
> 
> 

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

Re: [PATCH] ASoC: Add a sanity check before using dai driver name

[jeffy]

Hi Mark,

On 08/23/2017 07:06 PM, Mark Brown wrote:
> On Wed, Aug 23, 2017 at 07:42:55AM +0800, jeffy wrote:
>
>> >i think the original check is allowing NULL dlc dai_name to be a match...
>> >so we basically did:
>> >reject when dlc dai_name is valid, but not match the dai name
> So it is, but this still looks like the wrong thing - it'll match on an
> empty DAI name over an explicit match on the driver name which seems
> like it's the wrong way round.
>
sorry, i don't know much about asoc, so i just trying to keep the 
original conditions and add the new one on it :)

the original one is:
if (dlc->dai_name && strcmp(dai->name, dlc->dai_name))
    continue;

and i was trying to do something like:
if (dlc->dai_name && strcmp(dai->name, dlc->dai_name)
     && (!dai->driver->name || strcmp(dai->driver->name, dlc->dai_name)))
    continue;

which is add an accept case for: dai driver name is valid and matches 
the dai name we are looking for...

Re: [PATCH] ASoC: Add a sanity check before using dai driver name

[Mark Brown]

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

On Thu, Aug 24, 2017 at 11:29:42AM +0800, jeffy wrote:

> and i was trying to do something like:
> if (dlc->dai_name && strcmp(dai->name, dlc->dai_name)
>     && (!dai->driver->name || strcmp(dai->driver->name, dlc->dai_name)))
>    continue;

> which is add an accept case for: dai driver name is valid and matches the
> dai name we are looking for...

Writing it as one if statement would at least be clearer.  I can't
remember the patch you proposed at this point but the above looks
plausible.

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

Re: [PATCH] ASoC: Add a sanity check before using dai driver name

[jeffy]

hi Mark,

On 08/24/2017 06:18 PM, Mark Brown wrote:
> On Thu, Aug 24, 2017 at 11:29:42AM +0800, jeffy wrote:
>
>> and i was trying to do something like:
>> if (dlc->dai_name && strcmp(dai->name, dlc->dai_name)
>>      && (!dai->driver->name || strcmp(dai->driver->name, dlc->dai_name)))
>>     continue;
>
>> which is add an accept case for: dai driver name is valid and matches the
>> dai name we are looking for...
>
> Writing it as one if statement would at least be clearer.  I can't
> remember the patch you proposed at this point but the above looks
> plausible.
right, i've already post a v3 for that ;)
>