[alsa-devel] [PATCH] Fix an OOB bug in uac_mixer_unit_bmControls
`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@gmail.com Reported-by: Mathias Payer mathias.payer@nebelwelt.net Signed-off-by: Hui Peng benquike@gmail.com --- 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)
Sorry, this is the wrong patch. I will send it again.
On Mon, Aug 19, 2019 at 6:00 PM Hui Peng benquike@gmail.com 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@gmail.com Reported-by: Mathias Payer mathias.payer@nebelwelt.net Signed-off-by: Hui Peng benquike@gmail.com
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_bmControlsif (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_bmControlsif (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_bmControlsif (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
participants (1)
-
Hui Peng