lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Sat, 17 Aug 2019 08:55:32 +0200 From: Takashi Iwai <tiwai@...e.de> To: "Hui Peng" <benquike@...il.com> Cc: <security@...nel.org>, <alsa-devel@...a-project.org>, "YueHaibing" <yuehaibing@...wei.com>, "Thomas Gleixner" <tglx@...utronix.de>, "Mathias Payer" <mathias.payer@...elwelt.net>, "Jaroslav Kysela" <perex@...ex.cz>, "Takashi Iwai" <tiwai@...e.com>, "Wenwen Wang" <wang6495@....edu>, <linux-kernel@...r.kernel.org> Subject: Re: [PATCH] Fix an OOB bug in uac_mixer_unit_bmControls On Sat, 17 Aug 2019 06:32:07 +0200, Hui Peng wrote: > > `uac_mixer_unit_get_channels` calls `uac_mixer_unit_bmControls` > to get pointer to bmControls field. The current implementation of > `uac_mixer_unit_get_channels` does properly check the size of > uac_mixer_unit_descriptor descriptor and may allow OOB access > in `uac_mixer_unit_bmControls`. > > Reported-by: Hui Peng <benquike@...il.com> > Reported-by: Mathias Payer <mathias.payer@...elwelt.net> > Signed-off-by: Hui Peng <benquike@...il.com> Ah a good catch. One easier fix in this case would be to get the offset from uac_mixer_unit_bmControls(), e.g. /* calculate the offset of bmControls field */ size_t bmc_offset = uac_mixer_unit_bmControls(NULL, protocol) - NULL; .... if (desc->bLength < bmc_offset) return 0; thanks, Takashi > --- > sound/usb/mixer.c | 25 ++++++++++++++++++------- > 1 file changed, 18 insertions(+), 7 deletions(-) > > diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c > index b5927c3d5bc0..00e6274a63c3 100644 > --- a/sound/usb/mixer.c > +++ b/sound/usb/mixer.c > @@ -738,28 +738,39 @@ static int get_cluster_channels_v3(struct mixer_build *state, unsigned int clust > static int uac_mixer_unit_get_channels(struct mixer_build *state, > struct uac_mixer_unit_descriptor *desc) > { > - int mu_channels; > + int mu_channels = 0; > void *c; > > - if (desc->bLength < sizeof(*desc)) > - return -EINVAL; > if (!desc->bNrInPins) > return -EINVAL; > - if (desc->bLength < sizeof(*desc) + desc->bNrInPins) > - return -EINVAL; > > switch (state->mixer->protocol) { > case UAC_VERSION_1: > + // limit derived from uac_mixer_unit_bmControls > + if (desc->bLength < sizeof(*desc) + desc->bNrInPins + 4) > + return 0; > + > + mu_channels = uac_mixer_unit_bNrChannels(desc); > + break; > + > case UAC_VERSION_2: > - default: > - if (desc->bLength < sizeof(*desc) + desc->bNrInPins + 1) > + // limit derived from uac_mixer_unit_bmControls > + if (desc->bLength < sizeof(*desc) + desc->bNrInPins + 6) > return 0; /* no bmControls -> skip */ > + > mu_channels = uac_mixer_unit_bNrChannels(desc); > break; > case UAC_VERSION_3: > + // limit derived from uac_mixer_unit_bmControls > + if (desc->bLength < sizeof(*desc) + desc->bNrInPins + 2) > + return 0; /* no bmControls -> skip */ > + > mu_channels = get_cluster_channels_v3(state, > uac3_mixer_unit_wClusterDescrID(desc)); > break; > + > + default: > + break; > } > > if (!mu_channels) > -- > 2.22.1 > >
Powered by blists - more mailing lists