[PATCH v2] ALSA: usb-audio: Recurse before saving terminal properties

[PATCH v2] ALSA: usb-audio: Recurse before saving terminal properties

[Julian Scheel]

The input terminal parser recurses into the referenced clock entity to verify
it is existant and thus the terminal descriptor is valid. The actual property
values of the term instance which is initially parsed must not be overriden by
the recursion. For this to work the term properties have to be assigned after
recursing into the referenced clock entity descriptors.

Signed-off-by: Julian Scheel <julian@jusst.de>
---
Changes in v2:
- Store term->id after recursion as well, as it gets overriden in the
  recursion
- Add comments explaining that the recursion is only for validation of the
  descriptor validity
---
 sound/usb/mixer.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
index 81055d3..c50790c 100644
--- a/sound/usb/mixer.c
+++ b/sound/usb/mixer.c
@@ -731,15 +731,21 @@ static int check_input_term(struct mixer_build *state, int id,
 				term->name = d->iTerminal;
 			} else { /* UAC_VERSION_2 */
 				struct uac2_input_terminal_descriptor *d = p1;
-				term->type = le16_to_cpu(d->wTerminalType);
-				term->channels = d->bNrChannels;
-				term->chconfig = le32_to_cpu(d->bmChannelConfig);
-				term->name = d->iTerminal;
 
-				/* call recursively to get the clock selectors */
+				/* call recursively to verify that the
+				 * referenced clock entity is valid */
 				err = check_input_term(state, d->bCSourceID, term);
 				if (err < 0)
 					return err;
+
+				/* save input term properties after recursion,
+				 * to ensure they are not overriden by the
+				 * recursion calls */
+				term->id = id;
+				term->type = le16_to_cpu(d->wTerminalType);
+				term->channels = d->bNrChannels;
+				term->chconfig = le32_to_cpu(d->bmChannelConfig);
+				term->name = d->iTerminal;
 			}
 			return 0;
 		case UAC_FEATURE_UNIT: {
-- 
2.5.0

Re: [PATCH v2] ALSA: usb-audio: Recurse before saving terminal properties

[Daniel Mack]

On 08/19/2015 09:28 AM, Julian Scheel wrote:
> The input terminal parser recurses into the referenced clock entity to verify
> it is existant and thus the terminal descriptor is valid. The actual property
> values of the term instance which is initially parsed must not be overriden by
> the recursion. For this to work the term properties have to be assigned after
> recursing into the referenced clock entity descriptors.
> 
> Signed-off-by: Julian Scheel <julian@jusst.de>

Given your explanations, I believe this is the right thing to do.

Acked-by: Daniel Mack <daniel@zonque.org>


Thanks!


> ---
> Changes in v2:
> - Store term->id after recursion as well, as it gets overriden in the
>   recursion
> - Add comments explaining that the recursion is only for validation of the
>   descriptor validity
> ---
>  sound/usb/mixer.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
> index 81055d3..c50790c 100644
> --- a/sound/usb/mixer.c
> +++ b/sound/usb/mixer.c
> @@ -731,15 +731,21 @@ static int check_input_term(struct mixer_build *state, int id,
>  				term->name = d->iTerminal;
>  			} else { /* UAC_VERSION_2 */
>  				struct uac2_input_terminal_descriptor *d = p1;
> -				term->type = le16_to_cpu(d->wTerminalType);
> -				term->channels = d->bNrChannels;
> -				term->chconfig = le32_to_cpu(d->bmChannelConfig);
> -				term->name = d->iTerminal;
>  
> -				/* call recursively to get the clock selectors */
> +				/* call recursively to verify that the
> +				 * referenced clock entity is valid */
>  				err = check_input_term(state, d->bCSourceID, term);
>  				if (err < 0)
>  					return err;
> +
> +				/* save input term properties after recursion,
> +				 * to ensure they are not overriden by the
> +				 * recursion calls */
> +				term->id = id;
> +				term->type = le16_to_cpu(d->wTerminalType);
> +				term->channels = d->bNrChannels;
> +				term->chconfig = le32_to_cpu(d->bmChannelConfig);
> +				term->name = d->iTerminal;
>  			}
>  			return 0;
>  		case UAC_FEATURE_UNIT: {
> 

Re: [PATCH v2] ALSA: usb-audio: Recurse before saving terminal properties

[Takashi Iwai]

On Wed, 19 Aug 2015 17:17:19 +0200,
Daniel Mack wrote:
> 
> On 08/19/2015 09:28 AM, Julian Scheel wrote:
> > The input terminal parser recurses into the referenced clock entity to verify
> > it is existant and thus the terminal descriptor is valid. The actual property
> > values of the term instance which is initially parsed must not be overriden by
> > the recursion. For this to work the term properties have to be assigned after
> > recursing into the referenced clock entity descriptors.
> > 
> > Signed-off-by: Julian Scheel <julian@jusst.de>
> 
> Given your explanations, I believe this is the right thing to do.
> 
> Acked-by: Daniel Mack <daniel@zonque.org>

Thanks.  It's no urgent fix, I'm going to queue it to for-next
branch.


Takashi

> 
> 
> Thanks!
> 
> 
> > ---
> > Changes in v2:
> > - Store term->id after recursion as well, as it gets overriden in the
> >   recursion
> > - Add comments explaining that the recursion is only for validation of the
> >   descriptor validity
> > ---
> >  sound/usb/mixer.c | 16 +++++++++++-----
> >  1 file changed, 11 insertions(+), 5 deletions(-)
> > 
> > diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
> > index 81055d3..c50790c 100644
> > --- a/sound/usb/mixer.c
> > +++ b/sound/usb/mixer.c
> > @@ -731,15 +731,21 @@ static int check_input_term(struct mixer_build *state, int id,
> >  				term->name = d->iTerminal;
> >  			} else { /* UAC_VERSION_2 */
> >  				struct uac2_input_terminal_descriptor *d = p1;
> > -				term->type = le16_to_cpu(d->wTerminalType);
> > -				term->channels = d->bNrChannels;
> > -				term->chconfig = le32_to_cpu(d->bmChannelConfig);
> > -				term->name = d->iTerminal;
> >  
> > -				/* call recursively to get the clock selectors */
> > +				/* call recursively to verify that the
> > +				 * referenced clock entity is valid */
> >  				err = check_input_term(state, d->bCSourceID, term);
> >  				if (err < 0)
> >  					return err;
> > +
> > +				/* save input term properties after recursion,
> > +				 * to ensure they are not overriden by the
> > +				 * recursion calls */
> > +				term->id = id;
> > +				term->type = le16_to_cpu(d->wTerminalType);
> > +				term->channels = d->bNrChannels;
> > +				term->chconfig = le32_to_cpu(d->bmChannelConfig);
> > +				term->name = d->iTerminal;
> >  			}
> >  			return 0;
> >  		case UAC_FEATURE_UNIT: {
> > 
>