From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757700AbbCDCYF (ORCPT ); Tue, 3 Mar 2015 21:24:05 -0500 Received: from mail-bn1bon0138.outbound.protection.outlook.com ([157.56.111.138]:3053 "EHLO na01-bn1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1756819AbbCDCYD (ORCPT ); Tue, 3 Mar 2015 21:24:03 -0500 Date: Wed, 4 Mar 2015 09:11:19 +0800 From: Peter Chen To: Tapasweni Pathak CC: , , , , , , Subject: Re: [PATCH] drivers: usb: gadget: udc: Fix NULL dereference Message-ID: <20150304011118.GB23399@shlinux2> References: <20150303125841.GA9671@kt-Inspiron-3542> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <20150303125841.GA9671@kt-Inspiron-3542> User-Agent: Mutt/1.5.21 (2010-09-15) X-EOPAttributedMessage: 0 Authentication-Results: spf=fail (sender IP is 192.88.158.2) smtp.mailfrom=peter.chen@freescale.com; lip6.fr; dkim=none (message not signed) header.d=none; X-Forefront-Antispam-Report: CIP:192.88.158.2;CTRY:US;IPV:NLI;EFV:NLI;SFV:NSPM;SFS:(10019020)(6009001)(339900001)(24454002)(51704005)(110136001)(92566002)(86362001)(575784001)(50466002)(104016003)(23726002)(62966003)(2950100001)(77096005)(77156002)(97756001)(33656002)(105606002)(19580395003)(87936001)(6806004)(85426001)(19580405001)(46102003)(83506001)(106466001)(33716001)(76176999)(54356999)(47776003)(50986999)(46406003);DIR:OUT;SFP:1102;SCL:1;SRVR:BY2PR03MB191;H:az84smr01.freescale.net;FPR:;SPF:Fail;MLV:sfv;LANG:en; X-Microsoft-Antispam: UriScan:;UriScan:;BCL:0;PCL:0;RULEID:;SRVR:BY2PR03MB362; X-Microsoft-Antispam: BCL:0;PCL:0;RULEID:;SRVR:BY2PR03MB191; X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(601004)(5005006);SRVR:BY2PR03MB191; X-Forefront-PRVS: 0505147DDB X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:;SRVR:BY2PR03MB191; X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 Mar 2015 02:23:57.7167 (UTC) X-MS-Exchange-CrossTenant-Id: 710a03f5-10f6-4d38-9ff4-a80b81da590d X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=710a03f5-10f6-4d38-9ff4-a80b81da590d;Ip=[192.88.158.2] X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY2PR03MB191 X-OriginatorOrg: freescale.com Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Mar 03, 2015 at 06:28:42PM +0530, Tapasweni Pathak wrote: > This patch fixes multiple instances of null pointer dereference in this code. > > ep->udc is assigned to udc. ep is just an offset from _ep. _ep is then > checked for NULL. udc is dereferenced under the NULL check for _ep, making > an invalid pointer reference. > > udc is then checked for NULL, if NULL, it is then dereferenced as > udc->dev. > > To fix these issues, shift assignment of udc by dereferencing ep after > null check for _ep, replace both dev_dbg statements with pr_debug. > > Found using Coccinelle. > > Signed-off-by: Tapasweni Pathak > Suggested-by : Julia Lawall > Reviewed-by : Julia Lawall > --- > drivers/usb/gadget/udc/lpc32xx_udc.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/drivers/usb/gadget/udc/lpc32xx_udc.c b/drivers/usb/gadget/udc/lpc32xx_udc.c > index 27fd413..6398539 100644 > --- a/drivers/usb/gadget/udc/lpc32xx_udc.c > +++ b/drivers/usb/gadget/udc/lpc32xx_udc.c > @@ -1807,17 +1807,16 @@ static int lpc32xx_ep_queue(struct usb_ep *_ep, > !list_empty(&req->queue)) > return -EINVAL; > > - udc = ep->udc; > - > if (!_ep) { > - dev_dbg(udc->dev, "invalid ep\n"); > + pr_debug("invalid ep\n"); > return -EINVAL; > } > > + udc = ep->udc; > > if ((!udc) || (!udc->driver) || > (udc->gadget.speed == USB_SPEED_UNKNOWN)) { > - dev_dbg(udc->dev, "invalid device\n"); > + pr_debug("invalid device\n"); > return -EINVAL; > } > It is driver code, we'd better use dev_dbg. If _ep exists, both ep and udc should exist. So, how about just checking _ep at the beginning: diff --git a/drivers/usb/gadget/udc/lpc32xx_udc.c b/drivers/usb/gadget/udc/lpc32xx_udc.c index 27fd413..c65de0e 100644 --- a/drivers/usb/gadget/udc/lpc32xx_udc.c +++ b/drivers/usb/gadget/udc/lpc32xx_udc.c @@ -1803,7 +1803,7 @@ static int lpc32xx_ep_queue(struct usb_ep *_ep, req = container_of(_req, struct lpc32xx_request, req); ep = container_of(_ep, struct lpc32xx_ep, ep); - if (!_req || !_req->complete || !_req->buf || + if (!_ep || !_req || !_req->complete || !_req->buf || !list_empty(&req->queue)) return -EINVAL; @@ -1815,8 +1815,7 @@ static int lpc32xx_ep_queue(struct usb_ep *_ep, } - if ((!udc) || (!udc->driver) || - (udc->gadget.speed == USB_SPEED_UNKNOWN)) { + if ((!udc->driver) || (udc->gadget.speed == USB_SPEED_UNKNOWN)) { dev_dbg(udc->dev, "invalid device\n"); return -EINVAL; } -- Best Regards, Peter Chen