From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752613AbbIRFsb (ORCPT ); Fri, 18 Sep 2015 01:48:31 -0400 Received: from mail-pa0-f52.google.com ([209.85.220.52]:34181 "EHLO mail-pa0-f52.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751045AbbIRFsa (ORCPT ); Fri, 18 Sep 2015 01:48:30 -0400 Date: Fri, 18 Sep 2015 11:18:19 +0530 From: Sudip Mukherjee To: Alan Stern Cc: Greg Kroah-Hartman , linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org, Fabio Estevam , Sergei Shtylyov Subject: Re: [PATCH v2] USB: EHCI: fix dereference of ERR_PTR Message-ID: <20150918054819.GA31139@sudip-pc> References: <1442420559-5671-1-git-send-email-sudipm.mukherjee@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Sep 16, 2015 at 12:54:03PM -0400, Alan Stern wrote: > On Wed, 16 Sep 2015, Sudip Mukherjee wrote: > > > On error find_tt() returns either a NULL pointer or the error value in > > ERR_PTR. But we were dereferencing it directly without even checking if > > find_tt() returned a valid pointer or not. > > > > Signed-off-by: Sudip Mukherjee > > --- > > @@ -1373,6 +1375,8 @@ static void reserve_release_iso_bandwidth(struct ehci_hcd *ehci, > > } > > > > tt = find_tt(stream->ps.udev); > > + if (IS_ERR_OR_NULL(tt)) > > + return; > > if (sign > 0) > > list_add_tail(&stream->ps.ps_list, &tt->ps_list); > > else > > This patch isn't needed. In both reserve_release_intr_bandwidth() and > reserve_release_iso_bandwidth() it is known that find_tt() will return > a valid pointer. > > This is because each of those functions is called from only one place. > For example, reserve_release_intr_bandwidth() is called only at the end > of qh_schedule(). But near the start of qh_schedule() there is earlier > call to tt_find(), and there we do test for error pointers. If the > first call doesn't return an error then the second call won't either. > > The same sort of thing happens in reserve_release_iso_bandwidth(). Yes, I should have looked more before sending. Sorry for the noise. But in those checkes for find_tt() only IS_ERR is checked, shouldn't we check for IS_ERR_OR_NULL as find_tt() can return NULL also? regards sudip