On Thu, 22 Aug 2019 10:35:01 +0200
Takashi Iwai <tiwai@xxxxxxx> wrote:
> On Thu, 22 Aug 2019 10:31:48 +0200,
> Amadeusz Sławiński wrote:
> >
> > On Thu, 22 Aug 2019 09:32:03 +0200
> > Takashi Iwai <tiwai@xxxxxxx> wrote:
> >
> > > The bmControls (for UAC1) or bmMixerControls (for UAC2/3) bitmap has a
> > > variable size depending on both input and output pins. Its size is to
> > > fit with input * output bits. The problem is that the input size
> > > can't be determined simply from the unit descriptor itself but it
> > > needs to parse the whole connected sources. Although the
> > > uac_mixer_unit_get_channels() tries to check some possible overflow of
> > > this bitmap, it's incomplete due to the lack of the evaluation of
> > > input pins.
> > >
> > > For covering possible overflows, this patch adds the bitmap overflow
> > > check in the loop of input pins in parse_audio_mixer_unit().
> > >
> > > Fixes: 0bfe5e434e66 ("ALSA: usb-audio: Check mixer unit descriptors
> > > more strictly") Cc: <stable@xxxxxxxxxxxxxxx>
> > > Signed-off-by: Takashi Iwai <tiwai@xxxxxxx>
> > > ---
> > > sound/usb/mixer.c | 34 ++++++++++++++++++++++++++--------
> > > 1 file changed, 26 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
> > > index b5927c3d5bc0..27ee68a507a2 100644
> > > --- a/sound/usb/mixer.c
> > > +++ b/sound/usb/mixer.c
> > > @@ -739,7 +739,6 @@ static int uac_mixer_unit_get_channels(struct
> > > mixer_build *state, struct uac_mixer_unit_descriptor *desc)
> > > {
> > > int mu_channels;
> > > - void *c;
> > >
> > > if (desc->bLength < sizeof(*desc))
> > > return -EINVAL;
> > > @@ -762,13 +761,6 @@ static int uac_mixer_unit_get_channels(struct
> > > mixer_build *state, break;
> > > }
> > >
> > > - if (!mu_channels)
> > > - return 0;
> > > -
> > > - c = uac_mixer_unit_bmControls(desc, state->mixer->protocol);
> > > - if (c - (void *)desc + (mu_channels - 1) / 8 >=
> > > desc->bLength)
> > > - return 0; /* no bmControls -> skip */
> > > -
> > > return mu_channels;
> > > }
> > >
> > > @@ -2009,6 +2001,29 @@ static int parse_audio_feature_unit(struct
> > > mixer_build *state, int unitid,
> > > * Mixer Unit
> > > */
> > >
> > > +/* check whether the given in/out overflows bmMixerControls matrix */
> > > +static bool mixer_bitmap_overflow(struct uac_mixer_unit_descriptor
> > > *desc,
> > > + int protocol, int num_ins, int
> > > num_outs) +{
> > > + u8 *hdr = (u8 *)desc;
> > > + u8 *c = uac_mixer_unit_bmControls(desc, protocol);
> > > + size_t rest; /* remaining bytes after bmMixerControls */
> > > +
> > > + switch (protocol) {
> > > + case UAC_VERSION_1:
> > > + default:
> >
> > Won't this trigger implicit fall through warning?
>
> Ah indeed, I seem to have built with a little bit older kernel.
> Thanks for catching it.
>
> The revised patch is below.
>
Oh... didn't even notice the missing breaks ;)
I was asking about:
> + case UAC_VERSION_1:
> + default:
Unless default is handled specially it will probably warn on
UAC_VERSION_1 line.
>
> Takashi
>
> -- 8< --
> From: Takashi Iwai <tiwai@xxxxxxx>
> Subject: [PATCH v2] ALSA: usb-audio: Check mixer unit bitmap yet more strictly
>
> The bmControls (for UAC1) or bmMixerControls (for UAC2/3) bitmap has a
> variable size depending on both input and output pins. Its size is to
> fit with input * output bits. The problem is that the input size
> can't be determined simply from the unit descriptor itself but it
> needs to parse the whole connected sources. Although the
> uac_mixer_unit_get_channels() tries to check some possible overflow of
> this bitmap, it's incomplete due to the lack of the evaluation of
> input pins.
>
> For covering possible overflows, this patch adds the bitmap overflow
> check in the loop of input pins in parse_audio_mixer_unit().
>
> Fixes: 0bfe5e434e66 ("ALSA: usb-audio: Check mixer unit descriptors more strictly")
> Cc: <stable@xxxxxxxxxxxxxxx>
> Signed-off-by: Takashi Iwai <tiwai@xxxxxxx>
> ---
> V1->v2: Fix the missing break in mixer_bitmap_overflow().
>
> sound/usb/mixer.c | 36 ++++++++++++++++++++++++++++--------
> 1 file changed, 28 insertions(+), 8 deletions(-)
>
> diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
> index b5927c3d5bc0..eceab19766db 100644
> --- a/sound/usb/mixer.c
> +++ b/sound/usb/mixer.c
> @@ -739,7 +739,6 @@ static int uac_mixer_unit_get_channels(struct mixer_build *state,
> struct uac_mixer_unit_descriptor *desc)
> {
> int mu_channels;
> - void *c;
>
> if (desc->bLength < sizeof(*desc))
> return -EINVAL;
> @@ -762,13 +761,6 @@ static int uac_mixer_unit_get_channels(struct mixer_build *state,
> break;
> }
>
> - if (!mu_channels)
> - return 0;
> -
> - c = uac_mixer_unit_bmControls(desc, state->mixer->protocol);
> - if (c - (void *)desc + (mu_channels - 1) / 8 >= desc->bLength)
> - return 0; /* no bmControls -> skip */
> -
> return mu_channels;
> }
>
> @@ -2009,6 +2001,31 @@ static int parse_audio_feature_unit(struct mixer_build *state, int unitid,
> * Mixer Unit
> */
>
> +/* check whether the given in/out overflows bmMixerControls matrix */
> +static bool mixer_bitmap_overflow(struct uac_mixer_unit_descriptor *desc,
> + int protocol, int num_ins, int num_outs)
> +{
> + u8 *hdr = (u8 *)desc;
> + u8 *c = uac_mixer_unit_bmControls(desc, protocol);
> + size_t rest; /* remaining bytes after bmMixerControls */
> +
> + switch (protocol) {
> + case UAC_VERSION_1:
> + default:
> + rest = 1; /* iMixer */
> + break;
> + case UAC_VERSION_2:
> + rest = 2; /* bmControls + iMixer */
> + break;
> + case UAC_VERSION_3:
> + rest = 6; /* bmControls + wMixerDescrStr */
> + break;
> + }
> +
> + /* overflow? */
> + return c + (num_ins * num_outs + 7) / 8 + rest > hdr + hdr[0];
> +}
> +
> /*
> * build a mixer unit control
> *
> @@ -2137,6 +2154,9 @@ static int parse_audio_mixer_unit(struct mixer_build *state, int unitid,
> if (err < 0)
> return err;
> num_ins += iterm.channels;
> + if (mixer_bitmap_overflow(desc, state->mixer->protocol,
> + num_ins, num_outs))
> + break;
> for (; ich < num_ins; ich++) {
> int och, ich_has_controls = 0;
>
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
