From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751037AbdBHTki (ORCPT ); Wed, 8 Feb 2017 14:40:38 -0500 Received: from mga09.intel.com ([134.134.136.24]:21346 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750968AbdBHTkf (ORCPT ); Wed, 8 Feb 2017 14:40:35 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.35,348,1484035200"; d="scan'208";a="41643757" Message-ID: <1486582639.2133.412.camel@linux.intel.com> Subject: Re: [PATCH] drivers: usb: gadget: udc: remove pointer dereference after free From: Andy Shevchenko To: "Gustavo A. R. Silva" , gregkh@linuxfoundation.org, balbi@kernel.org, bhelgaas@google.com, heikki.krogerus@linux.intel.com, mail@iagoabal.eu, mina86@mina86.com Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org Date: Wed, 08 Feb 2017 21:37:19 +0200 In-Reply-To: <20170208191549.GA3998@embeddedgus> References: <20170208191549.GA3998@embeddedgus> Organization: Intel Finland Oy Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.22.3-1 Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 2017-02-08 at 13:15 -0600, Gustavo A. R. Silva wrote: > Remove pointer dereference and write after free. It's wrong description. There is no write after free. The memory is still in pool and one may access it. Though the access is *formally* illegal. Code itself looks interesting. > > Addresses-Coverity-ID: 1091173 > Signed-off-by: Gustavo A. R. Silva > --- >  drivers/usb/gadget/udc/pch_udc.c | 1 - >  1 file changed, 1 deletion(-) > > diff --git a/drivers/usb/gadget/udc/pch_udc.c > b/drivers/usb/gadget/udc/pch_udc.c > index a97da64..8a365aa 100644 > --- a/drivers/usb/gadget/udc/pch_udc.c > +++ b/drivers/usb/gadget/udc/pch_udc.c > @@ -1523,7 +1523,6 @@ static void pch_udc_free_dma_chain(struct > pch_udc_dev *dev, >   td = phys_to_virt(addr); >   addr2 = (dma_addr_t)td->next; >   pci_pool_free(dev->data_requests, td, addr); > - td->next = 0x00; I think the better fix is to move this line before pci_pool_free() call. I dunno those td->next = 0x00; make any sense there. Is it done under some lock / serialization? >   addr = addr2; >   } >   req->chain_len = 1; -- Andy Shevchenko Intel Finland Oy