[PATCH] ALSA: usb-audio: Fix missing error handling for get_min_max*()

[PATCH] ALSA: usb-audio: Fix missing error handling for get_min_max*()

[Takashi Iwai]

The recent fix to add the error return value check from get_min_max*()
missed one case in build_audio_procunit() where no error value is set.
This may lead to an uninitialized variable and confuse the caller
(although this wouldn't happen practically because err is set for the
loop of num_ins at the beginning of the funciton).

Fix it by setting "err = 0" properly at the missing case, too.

Fixes: 4f55a85cd4fc ("ALSA: usb-audio: Add error checks against get_min_max*()")
Reported-by: Dan Carpenter <error27@gmail.com>
Closes: https://lore.kernel.org/ad36dGpCBTGsyFr_@stanley.mountain
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/usb/mixer.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
index d4ef45bf53d7..aa6ea3be100a 100644
--- a/sound/usb/mixer.c
+++ b/sound/usb/mixer.c
@@ -2687,6 +2687,7 @@ static int build_audio_procunit(struct mixer_build *state, int unitid,
 				cval->max = control_spec[0];
 				cval->res = 1;
 				cval->initialized = 1;
+				err = 0;
 				break;
 			}
 
-- 
2.53.0

Re: [PATCH] ALSA: usb-audio: Fix missing error handling for get_min_max*()

[Rong Zhang]

Hi Takashi,

On Tue, 2026-04-14 at 11:33 +0200, Takashi Iwai wrote:
> The recent fix to add the error return value check from get_min_max*()
> missed one case in build_audio_procunit() where no error value is set.
> This may lead to an uninitialized variable and confuse the caller
> (although this wouldn't happen practically because err is set for the
> loop of num_ins at the beginning of the funciton).
> 
> Fix it by setting "err = 0" properly at the missing case, too.
> 
> Fixes: 4f55a85cd4fc ("ALSA: usb-audio: Add error checks against get_min_max*()")
> Reported-by: Dan Carpenter <error27@gmail.com>
> Closes: https://lore.kernel.org/ad36dGpCBTGsyFr_@stanley.mountain
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>  sound/usb/mixer.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
> index d4ef45bf53d7..aa6ea3be100a 100644
> --- a/sound/usb/mixer.c
> +++ b/sound/usb/mixer.c
> @@ -2687,6 +2687,7 @@ static int build_audio_procunit(struct mixer_build *state, int unitid,
>  				cval->max = control_spec[0];
>  				cval->res = 1;
>  				cval->initialized = 1;
> +				err = 0;

Whoops, it's my fault that I didn't noticed the code path.

I think we can make the UAC_PROCESS_UP_DOWNMIX case fall through the
default case so that we can move the error check into the latter and get
rid of all `err = 0'. Doing so also improves readability and prevents
similar mistakes in future changes. Perhaps I should have done this in
my previous refactoring patch...

I just noticed that you've applied this patch to your tree. If my
proposed refactor makes sense to you, I will send a patch for it.

Thanks,
Rong

>  				break;
>  			}
>  

Re: [PATCH] ALSA: usb-audio: Fix missing error handling for get_min_max*()

[Takashi Iwai]

On Tue, 14 Apr 2026 14:17:28 +0200,
Rong Zhang wrote:
> 
> Hi Takashi,
> 
> On Tue, 2026-04-14 at 11:33 +0200, Takashi Iwai wrote:
> > The recent fix to add the error return value check from get_min_max*()
> > missed one case in build_audio_procunit() where no error value is set.
> > This may lead to an uninitialized variable and confuse the caller
> > (although this wouldn't happen practically because err is set for the
> > loop of num_ins at the beginning of the funciton).
> > 
> > Fix it by setting "err = 0" properly at the missing case, too.
> > 
> > Fixes: 4f55a85cd4fc ("ALSA: usb-audio: Add error checks against get_min_max*()")
> > Reported-by: Dan Carpenter <error27@gmail.com>
> > Closes: https://lore.kernel.org/ad36dGpCBTGsyFr_@stanley.mountain
> > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> > ---
> >  sound/usb/mixer.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
> > index d4ef45bf53d7..aa6ea3be100a 100644
> > --- a/sound/usb/mixer.c
> > +++ b/sound/usb/mixer.c
> > @@ -2687,6 +2687,7 @@ static int build_audio_procunit(struct mixer_build *state, int unitid,
> >  				cval->max = control_spec[0];
> >  				cval->res = 1;
> >  				cval->initialized = 1;
> > +				err = 0;
> 
> Whoops, it's my fault that I didn't noticed the code path.
> 
> I think we can make the UAC_PROCESS_UP_DOWNMIX case fall through the
> default case so that we can move the error check into the latter and get
> rid of all `err = 0'. Doing so also improves readability and prevents
> similar mistakes in future changes. Perhaps I should have done this in
> my previous refactoring patch...
> 
> I just noticed that you've applied this patch to your tree. If my
> proposed refactor makes sense to you, I will send a patch for it.

That was my another idea, too.  OTOH, I thought handling the success
error code explicitly makes it clearer that the code handles it.

It's pretty nuance, and I'm open for either option, so feel free to
submit a cleanup patch on the top.


thanks,

Takashi