From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755832AbcLUPDE (ORCPT ); Wed, 21 Dec 2016 10:03:04 -0500 Received: from mailout2.samsung.com ([203.254.224.25]:56352 "EHLO mailout2.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751294AbcLUPDD (ORCPT ); Wed, 21 Dec 2016 10:03:03 -0500 X-AuditID: cbfee61b-f79d86d00000197e-ea-585a99a47671 Subject: Re: [PATCH] usbip: vudc: check for NULL before use To: Shuah Khan , Sudip Mukherjee , Felipe Balbi References: <1482101091-3969-1-git-send-email-sudipm.mukherjee@gmail.com> <20161221133315.GA13517@sudip-tp> Cc: Shuah Khan , Valentina Manea , Greg Kroah-Hartman , LKML , linux-usb@vger.kernel.org, Shuah Khan , Krzysztof Opasiak From: Krzysztof Opasiak Message-id: <89d1dfdd-605e-0337-ac6c-b0266bc13fe5@samsung.com> Date: Wed, 21 Dec 2016 16:02:55 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 MIME-version: 1.0 In-reply-to: Content-type: text/plain; charset=utf-8 Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFjrAIsWRmVeSWpSXmKPExsVy+t9jAd0lM6MiDDbP1LM41vaE3aJ58Xo2 i2en8ywu75rDZrFoWSuzxZSX69gtpn75wGLx9aeDxYHTU5gt3l2ay+7A5bFz1l12j02rOtk8 9s9dw+6xpR/I/bxJLoA1ys0mIzUxJbVIITUvOT8lMy/dVik0xE3XQkkhLzE31VYpQtc3JEhJ oSwxpxTIMzJAAw7OAe7BSvp2CW4ZG6b8ZS74JFKxu3cxcwPjdYEuRk4OCQETiW3zr7JA2GIS F+6tZ+ti5OIQEljKKPH+8gRmCOcho0Tv4leMIFXCAtYSv862A1VxcIgIlErsWZYHUbOHSeLP p8dg3cwCM5kkPr28yQ5SxCagLzFvlyhIL6+AncT5Zy1g21gEVCU+XN3PBmKLCkRI3HrYwQJR IyjxY/I9MJtTIFhi06E5YLuYBdQlpkzJBQkzC8hLbF7zlnkCo8AsJB2zEKpmIalawMi8ilEi tSC5oDgpPdcoL7Vcrzgxt7g0L10vOT93EyM49p5J72A8vMv9EKMAB6MSD++MaVERQqyJZcWV uYcYJTiYlUR42SYDhXhTEiurUovy44tKc1KLDzGaAr0xkVlKNDkfmBbySuINTcxNzI0NLMwt LU2MlMR5G2c/CxcSSE8sSc1OTS1ILYLpY+LglGpgnCLcO7+yJOtJhm9a5Jr9/0I/OAW2Lqg+ cdr2C1vFNplzczN3Jh/zDtTls9wRsuvEvw3XLdRvxTJ9SblVc0NxeX5ANvdkv8BLKgZPpgmH vped7mbT+uD1P6czO2tW5v/b8jeh1U5clk3nxZN9ITrsjiEJztfTZj4/duW65qmHvdqbzf22 /EquVWIpzkg01GIuKk4EALHdhprTAgAA X-MTR: 20000000000000000@CPGS Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, On 12/21/2016 03:38 PM, Shuah Khan wrote: > Hi Sudip, > > On Wed, Dec 21, 2016 at 6:33 AM, Sudip Mukherjee > wrote: >> On Tue, Dec 20, 2016 at 07:31:44AM -0700, Shuah Khan wrote: >>> On 12/18/2016 03:44 PM, Sudip Mukherjee wrote: >>>> to_vep() is doing a container_of() on _ep. It is better to do the NULL >>>> check first and then use it. >>>> >>>> Signed-off-by: Sudip Mukherjee >>>> --- >>>> drivers/usb/usbip/vudc_dev.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/usb/usbip/vudc_dev.c b/drivers/usb/usbip/vudc_dev.c >>>> index 968471b..32ea604 100644 >>>> --- a/drivers/usb/usbip/vudc_dev.c >>>> +++ b/drivers/usb/usbip/vudc_dev.c >>>> @@ -388,10 +388,10 @@ static int vep_dequeue(struct usb_ep *_ep, struct usb_request *_req) >>>> unsigned long flags; >>>> int ret = 0; >>>> >>>> - ep = to_vep(_ep); >>>> if (!_ep) >>>> return -EINVAL; >>> >>> Hmm. Linus's latest checks _ep and _req. Are you sure you are working >>> with the latest tree? >> >> I checked with next-20161221 and its still there. > > This is for vep_dequeue() - Are you sure both linux-next and Linus's tree show > the following: > > http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/usb/usbip/vudc_dev.c > > static int vep_dequeue(struct usb_ep *_ep, struct usb_request *_req) > { > struct vep *ep; > struct vrequest *req; > struct vudc *udc; > struct vrequest *lst; > unsigned long flags; > int ret = -EINVAL; > > if (!_ep || !_req) > return ret; > > ep = to_vep(_ep); > req = to_vrequest(_req); > udc = req->udc; > > if (!udc->driver) > return -ESHUTDOWN; > > There are other routines in this file that don't check null. I am confused. > generally this file contains implementation of gadget/endpoint callbacks. Those methods will be called by udc-core with values passed from USB gadget (usually directly from function). I check other udc in kernel and there is no agreement if we should check in those callbacks if our params are NULL or not. I have also run through udc-core implementation and generally it doesn't check if params are NULL or not and just dereference some of them and pass them to our callbacks. I think that the best option is just to ask Felipe (USB gadget maintainer) if we should check this or not. @Felipe Should each UDC check if values passed to gadget/endpoint callbacks is not NULL or just simply dereference them? Best regards, -- Krzysztof Opasiak Samsung R&D Institute Poland Samsung Electronics