From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751877AbdIUOMj (ORCPT ); Thu, 21 Sep 2017 10:12:39 -0400 Received: from mail-io0-f182.google.com ([209.85.223.182]:46115 "EHLO mail-io0-f182.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751711AbdIUOMi (ORCPT ); Thu, 21 Sep 2017 10:12:38 -0400 X-Google-Smtp-Source: AOwi7QAUDugxxIkHxI1ZeIrms68dLPjc9Fn6XYbgCsQ8w6dCZUvV0bjRaPTrz9XRBxNYDAAWtrQtCEoaomMfaLOMsho= MIME-Version: 1.0 In-Reply-To: <20170921140731.GA20874@kroah.com> References: <20170921073154.GA11294@kroah.com> <20170921080428.GA31635@kroah.com> <20170921140731.GA20874@kroah.com> From: Andrey Konovalov Date: Thu, 21 Sep 2017 16:12:36 +0200 Message-ID: Subject: Re: usb/core: slab-out-of-bounds read in cdc_parse_cdc_header To: Greg Kroah-Hartman Cc: Jonathan Corbet , Mauro Carvalho Chehab , Jaejoong Kim , USB list , LKML , Dmitry Vyukov , Kostya Serebryany , syzkaller Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Sep 21, 2017 at 4:07 PM, Greg Kroah-Hartman wrote: > On Thu, Sep 21, 2017 at 03:51:44PM +0200, Andrey Konovalov wrote: >> On Thu, Sep 21, 2017 at 10:04 AM, Greg Kroah-Hartman >> wrote: >> > On Thu, Sep 21, 2017 at 09:31:54AM +0200, Greg Kroah-Hartman wrote: >> >> On Wed, Sep 20, 2017 at 04:45:08PM +0200, Andrey Konovalov wrote: >> >> > Hi! >> >> > >> >> > I've got the following crash while fuzzing the kernel with syzkaller. >> >> > >> >> > On commit ebb2c2437d8008d46796902ff390653822af6cc4 (Sep 18). >> >> > >> >> > It looks like cdc_parse_cdc_header() doesn't validate buflen before >> >> > accessing buffer[1], buffer[2] and so on. The only check present is >> >> > while (buflen > 0). >> >> >> >> Ugh, you are right, let me go work on a patch, thanks for the report... >> > >> > Here's a first cut at a fix for this. I think this should solve it, but >> > it's early and my coffee has not fully kicked in... >> > >> > thanks, >> > >> > greg k-h >> > ----------------- >> > >> > diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c >> > index 4c38ea41ae96..028feaf01aa5 100644 >> > --- a/drivers/usb/core/message.c >> > +++ b/drivers/usb/core/message.c >> > @@ -2069,6 +2069,10 @@ int cdc_parse_cdc_header(struct usb_cdc_parsed_header *hdr, >> > elength = 1; >> > goto next_desc; >> > } >> > + if ((buflen < elength) || (elength < 2)) { >> >> Hi Greg, >> >> I think this should check (elength < 3), since we access both >> buffer[1] and buffer[2] after this check. > > {sigh} yes, you are right, counted this one wrong. > > With this patch, updated one below, does it fix your crash? It does, thanks! Tested-by: Andrey Konovalov > > thanks, > > greg k-h > > > diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c > index 4c38ea41ae96..028feaf01aa5 100644 > --- a/drivers/usb/core/message.c > +++ b/drivers/usb/core/message.c > @@ -2069,6 +2069,10 @@ int cdc_parse_cdc_header(struct usb_cdc_parsed_header *hdr, > elength = 1; > goto next_desc; > } > + if ((buflen < elength) || (elength < 3)) { > + dev_err(&intf->dev, "invalid descriptor buffer length\n"); > + break; > + } > if (buffer[1] != USB_DT_CS_INTERFACE) { > dev_err(&intf->dev, "skipping garbage\n"); > goto next_desc;