From mboxrd@z Thu Jan 1 00:00:00 1970 From: Takashi Iwai Subject: Re: [PATCH 2/2] ALSA: usb-audio: Add sanity checks in v3 clock parsers Date: Wed, 04 Apr 2018 07:34:33 +0200 Message-ID: References: <20180403154808.21947-1-tiwai@suse.de> <20180403154808.21947-2-tiwai@suse.de> Mime-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by alsa0.perex.cz (Postfix) with ESMTP id DC61126760A for ; Wed, 4 Apr 2018 07:34:35 +0200 (CEST) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: alsa-devel-bounces@alsa-project.org To: Ruslan Bilovol Cc: alsa-devel@alsa-project.org List-Id: alsa-devel@alsa-project.org On Wed, 04 Apr 2018 01:15:05 +0200, Ruslan Bilovol wrote: > > On Tue, Apr 3, 2018 at 6:48 PM, Takashi Iwai wrote: > > The UAC3 clock parser codes lack of the sanity checks for malformed > > descriptors like UAC2 parser does. Without it, the driver may lead to > > a potential crash. > > > > Fixes: 9a2fe9b801f5 ("ALSA: usb: initial USB Audio Device Class 3.0 support") > > Signed-off-by: Takashi Iwai > > --- > > sound/usb/clock.c | 7 ++++--- > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > diff --git a/sound/usb/clock.c b/sound/usb/clock.c > > index c5f0cf532c0c..169fb3ac3715 100644 > > --- a/sound/usb/clock.c > > +++ b/sound/usb/clock.c > > @@ -58,7 +58,7 @@ static bool validate_clock_source_v2(void *p, int id) > > static bool validate_clock_source_v3(void *p, int id) > > { > > struct uac3_clock_source_descriptor *cs = p; > > - return cs->bClockID == id; > > + return cs->bLength >= sizeof(*cs) && cs->bClockID == id; > > I'm not sure why UAC2 checks are relaxed, but we can be more strict > here since bLength of uac3_clock_source_descriptor is defined by standard > and should be 12, so we can check for exact match in this place. > > > } > > > > static bool validate_clock_selector_v2(void *p, int id) > > @@ -71,7 +71,8 @@ static bool validate_clock_selector_v2(void *p, int id) > > static bool validate_clock_selector_v3(void *p, int id) > > { > > struct uac3_clock_selector_descriptor *cs = p; > > - return cs->bClockID == id; > > + return cs->bLength >= sizeof(*cs) && cs->bClockID == id && > > + cs->bLength >= 5 + cs->bNrInPins; > > } > > Same here, bLength is defined by spec, can be easily calculated and > must be "11+bNrInPins" > > > > > static bool validate_clock_multiplier_v2(void *p, int id) > > @@ -83,7 +84,7 @@ static bool validate_clock_multiplier_v2(void *p, int id) > > static bool validate_clock_multiplier_v3(void *p, int id) > > { > > struct uac3_clock_multiplier_descriptor *cs = p; > > - return cs->bClockID == id; > > + return cs->bLength >= sizeof(*cs) && cs->bClockID == id; > > Also here, bLength should be 11 as per spec > > By the way, we can make UAC2 bLength checks more strict as well, > assuming there is no any hw bug we try to workaroud OK, let's make the check more strict altogether. thanks, Takashi