Hi, Takashi: Here the problem is that `desc->bLength` is controlled by the device side, so `desc->bLength` may not represent the real length of the descriptor. That is why I use pointer arithmetic operations to derive the real size of the buffer in my patch.
On Wed, Aug 14, 2019 at 2:36 AM Takashi Iwai tiwai@suse.de wrote:
On Wed, 14 Aug 2019 04:36:24 +0200, Hui Peng wrote:
The `uac_mixer_unit_descriptor` shown as below is read from the device side. In `parse_audio_mixer_unit`, `baSourceID` field is accessed from index 0 to `bNrInPins` - 1, the current implementation assumes that descriptor is always valid (the length of descriptor is no shorter than 5 + `bNrInPins`). If a descriptor read from the device side is invalid, it may trigger out-of-bound memory access.
struct uac_mixer_unit_descriptor { __u8 bLength; __u8 bDescriptorType; __u8 bDescriptorSubtype; __u8 bUnitID; __u8 bNrInPins; __u8 baSourceID[]; }This patch fixes the bug by add a sanity check on the length of the descriptor.
Signed-off-by: Hui Peng benquike@gmail.com Reported-by: Hui Peng benquike@gmail.com Reported-by: Mathias Payer mathias.payer@nebelwelt.net
sound/usb/mixer.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c index 7498b5191b68..38202ce67237 100644 --- a/sound/usb/mixer.c +++ b/sound/usb/mixer.c @@ -2091,6 +2091,15 @@ static int parse_audio_mixer_unit(struct
mixer_build *state, int unitid,
struct usb_audio_term iterm; int input_pins, num_ins, num_outs; int pin, ich, err;
int desc_len = (int) ((unsigned long) state->buffer +state->buflen - (unsigned long) raw_desc);if (desc_len < sizeof(*desc) + desc->bNrInPins) {usb_audio_err(state->chip,"descriptor %d too short\n",unitid);return -EINVAL;} err = uac_mixer_unit_get_channels(state, desc); if (err < 0) {Hm, what is the desc->bLength value in the error case?
Basically the buffer boundary is already checked against bLength in snd_usb_find_desc() which is called from obtaining the raw_desc in the caller of this function (parse_audio_unit()).
So, if any, we need to check bLength for the possible overflow like below.
thanks,
Takashi
--- a/sound/usb/mixer.c +++ b/sound/usb/mixer.c @@ -744,6 +744,8 @@ static int uac_mixer_unit_get_channels(struct mixer_build *state, return -EINVAL; if (!desc->bNrInPins) return -EINVAL;
if (desc->bLength < sizeof(*desc) + desc->bNrInPins)return -EINVAL; switch (state->mixer->protocol) { case UAC_VERSION_1: