diff mbox series

[X/B,SRU,CVE-2019-15118,2/2] ALSA: usb-audio: Fix a stack buffer overflow bug in check_input_term

Message ID 20190830001351.3686-5-connor.kuehl@canonical.com
State New
Headers show
Series Fixes for CVE-2019-15117 & CVE-2019-15118 | expand

Commit Message

Connor Kuehl Aug. 30, 2019, 12:13 a.m. UTC
From: Hui Peng <benquike@gmail.com>

CVE-2019-15118

`check_input_term` recursively calls itself with input from
device side (e.g., uac_input_terminal_descriptor.bCSourceID)
as argument (id). In `check_input_term`, if `check_input_term`
is called with the same `id` argument as the caller, it triggers
endless recursive call, resulting kernel space stack overflow.

This patch fixes the bug by adding a bitmap to `struct mixer_build`
to keep track of the checked ids and stop the execution if some id
has been checked (similar to how parse_audio_unit handles unitid
argument).

Reported-by: Hui Peng <benquike@gmail.com>
Reported-by: Mathias Payer <mathias.payer@nebelwelt.net>
Signed-off-by: Hui Peng <benquike@gmail.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
(backported from commit 19bce474c45be69a284ecee660aa12d8f1e88f18)
[ Connor Kuehl: Manually replaced the while loop with the new for
  loop due to context adjustments. Some hunks were dropped as they
  were targeting bits of code that aren't added until mainline commit:
  9a2fe9b801f5 "ALSA: usb: initial USB Audio Device Class 3.0 support"
Signed-off-by: Connor Kuehl <connor.kuehl@canonical.com>
---
 sound/usb/mixer.c | 30 +++++++++++++++++++++++++-----
 1 file changed, 25 insertions(+), 5 deletions(-)
diff mbox series

Patch

diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
index 713020741289..f743fd91a138 100644
--- a/sound/usb/mixer.c
+++ b/sound/usb/mixer.c
@@ -82,6 +82,7 @@  struct mixer_build {
 	unsigned char *buffer;
 	unsigned int buflen;
 	DECLARE_BITMAP(unitbitmap, MAX_ID_ELEMS);
+	DECLARE_BITMAP(termbitmap, MAX_ID_ELEMS);
 	struct usb_audio_term oterm;
 	const struct usbmix_name_map *map;
 	const struct usbmix_selector_map *selector_map;
@@ -716,16 +717,26 @@  static int get_term_name(struct mixer_build *state, struct usb_audio_term *iterm
  * parse the source unit recursively until it reaches to a terminal
  * or a branched unit.
  */
-static int check_input_term(struct mixer_build *state, int id,
+static int __check_input_term(struct mixer_build *state, int id,
 			    struct usb_audio_term *term)
 {
 	int err;
 	void *p1;
+	unsigned char *hdr;
 
 	memset(term, 0, sizeof(*term));
-	while ((p1 = find_audio_control_unit(state, id)) != NULL) {
-		unsigned char *hdr = p1;
+	for (;;) {
+		/* a loop in the terminal chain? */
+		if (test_and_set_bit(id, state->termbitmap))
+			return -EINVAL;
+
+		p1 = find_audio_control_unit(state, id);
+		if (!p1)
+			break;
+
+		hdr = p1;
 		term->id = id;
+
 		switch (hdr[2]) {
 		case UAC_INPUT_TERMINAL:
 			if (state->mixer->protocol == UAC_VERSION_1) {
@@ -739,7 +750,7 @@  static int check_input_term(struct mixer_build *state, int id,
 
 				/* call recursively to verify that the
 				 * referenced clock entity is valid */
-				err = check_input_term(state, d->bCSourceID, term);
+				err = __check_input_term(state, d->bCSourceID, term);
 				if (err < 0)
 					return err;
 
@@ -771,7 +782,7 @@  static int check_input_term(struct mixer_build *state, int id,
 		case UAC2_CLOCK_SELECTOR: {
 			struct uac_selector_unit_descriptor *d = p1;
 			/* call recursively to retrieve the channel info */
-			err = check_input_term(state, d->baSourceID[0], term);
+			err = __check_input_term(state, d->baSourceID[0], term);
 			if (err < 0)
 				return err;
 			term->type = d->bDescriptorSubtype << 16; /* virtual type */
@@ -818,6 +829,15 @@  static int check_input_term(struct mixer_build *state, int id,
 	return -ENODEV;
 }
 
+
+static int check_input_term(struct mixer_build *state, int id,
+			    struct usb_audio_term *term)
+{
+	memset(term, 0, sizeof(*term));
+	memset(state->termbitmap, 0, sizeof(state->termbitmap));
+	return __check_input_term(state, id, term);
+}
+
 /*
  * Feature Unit
  */