From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753607AbbCWXAm (ORCPT ); Mon, 23 Mar 2015 19:00:42 -0400 Received: from mail-ig0-f170.google.com ([209.85.213.170]:37697 "EHLO mail-ig0-f170.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753336AbbCWXAj (ORCPT ); Mon, 23 Mar 2015 19:00:39 -0400 Message-ID: <55109B15.9080800@google.com> Date: Mon, 23 Mar 2015 16:00:37 -0700 From: Badhri Jagan Sridharan User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0 MIME-Version: 1.0 To: Peter Chen CC: Felipe Balbi , Greg Kroah-Hartman , linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] usb: gadget: Check for NULL pointer in disconnect References: <1426894852-4256-1-git-send-email-Badhri@google.com> <20150322074353.GA25278@shlinux2> In-Reply-To: <20150322074353.GA25278@shlinux2> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org > Do you really see some udc drivers call it after composite_unbind is > called? If it is, you may add dump_stack() to track that error. > > Besides this, function suspended_show is needed to add cdev NULL pointer > checking. We see this happening occasionally in *not yet* upstreamed UDC code of some vendors (Yes, disconnect being called after unbind) After reviewing the entire composite.c file, I did notice that none of the functions check for NULL pointer when cdev is obtained from get_gadget_data. Is crashing/bringing down the whole kernel intentionally left to happen ? Isn't printing a WARN/ERROR msg and returning not the preferable approach ? On 03/22/2015 12:43 AM, Peter Chen wrote: > On Fri, Mar 20, 2015 at 04:40:52PM -0700, Badhri Jagan Sridharan wrote: >> Added a safety net to make sure that >> composite_disconnect does not end up disconneting >> a NULL device. Prevents NULL pointer crash. >> >> Signed-off-by: Badhri Jagan Sridharan >> --- >> drivers/usb/gadget/composite.c | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c >> index 13adfd1..90b37bd 100644 >> --- a/drivers/usb/gadget/composite.c >> +++ b/drivers/usb/gadget/composite.c >> @@ -1823,6 +1823,11 @@ void composite_disconnect(struct usb_gadget *gadget) >> struct usb_composite_dev *cdev = get_gadget_data(gadget); >> unsigned long flags; >> >> + if (!cdev) { >> + WARN(1, "Trying to disconnect a NULL composite device\n"); >> + return; >> + } >> + > > Do you really see some udc drivers call it after composite_unbind is > called? If it is, you may add dump_stack() to track that error. > > Besides this, function suspended_show is needed to add cdev NULL pointer > checking. > >> /* REVISIT: should we have config and device level >> * disconnect callbacks? >> */ >> -- >> 2.2.0.rc0.207.ga3a616c >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-usb" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >