From patchwork Thu Mar 22 09:30:36 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ming Lei X-Patchwork-Id: 148199 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id BEEB9B6EF3 for ; Thu, 22 Mar 2012 20:30:51 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757050Ab2CVJak (ORCPT ); Thu, 22 Mar 2012 05:30:40 -0400 Received: from mail-pz0-f46.google.com ([209.85.210.46]:32793 "EHLO mail-pz0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754867Ab2CVJah (ORCPT ); Thu, 22 Mar 2012 05:30:37 -0400 Received: by dajr28 with SMTP id r28so2877790daj.19 for ; Thu, 22 Mar 2012 02:30:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=puCfisgpnfZPVqh8hkvEKfDSMoAFarUBmTEpr361V2U=; b=yhy7el2Sv/2gueaGabxBsw3DA/BHu9G37ZRKCgeCv8pwVRvVjXBSVbeYQ+O03cAUAY SLL9ajgHHrI6Dw7a6R1CxgIAisd+Qx7VQ3UqyXWJbB8rigj/7FYXd+BD9Aquy6Ob8BRt gfGgxFxbRF7jkVMePbm0gB/UId0vhrLoOonmMSm7xA56Qrf8Dufclxt1oaRIlQyhKECb z2MFupAqu2MbhFQ0qUcWb9/dmVDNWIbtHBQpS5268PSgEsAUG3Yw75jCoZSmzMfc1yOT UjMRraao7hV0gakwv88UQly+KVSxZc71J1b1Lb+CGGGAGm0bpl7lTa60FvTXO94FJZCa rLog== MIME-Version: 1.0 Received: by 10.68.136.69 with SMTP id py5mr19217462pbb.71.1332408637038; Thu, 22 Mar 2012 02:30:37 -0700 (PDT) Received: by 10.143.41.13 with HTTP; Thu, 22 Mar 2012 02:30:36 -0700 (PDT) In-Reply-To: <201203221008.46882.oneukum@suse.de> References: <201203221008.46882.oneukum@suse.de> Date: Thu, 22 Mar 2012 17:30:36 +0800 Message-ID: Subject: Re: use-after-free in usbnet From: Ming Lei To: Oliver Neukum Cc: Alan Stern , netdev@vger.kernel.org, linux-usb@vger.kernel.org, Fedora Kernel Team , Dave Jones Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Thu, Mar 22, 2012 at 5:08 PM, Oliver Neukum wrote: > > this looks good, but could you add a comment explaining the reason for > taking a reference? OK, I will post a formal one if you have no objection on the below. // these (async) unlinks complete immediately @@ -597,6 +605,7 @@ static int unlink_urbs (struct usbnet *dev, struct sk_buff_head *q) netdev_dbg(dev->net, "unlink urb err, %d\n", retval); else count++; + usb_put_urb(urb); spin_lock_irqsave(&q->lock, flags); } spin_unlock_irqrestore (&q->lock, flags); Thanks, diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c index 4b8b52c..febfdce 100644 --- a/drivers/net/usb/usbnet.c +++ b/drivers/net/usb/usbnet.c @@ -589,6 +589,14 @@ static int unlink_urbs (struct usbnet *dev, struct sk_buff_head *q) entry = (struct skb_data *) skb->cb; urb = entry->urb; + /* + * Get a reference count of the URB to avoid it to be + * freed during usb_unlink_urb, which may trigger + * use-after-free problem inside usb_unlink_urb since + * usb_unlink_urb is always racing with .complete + * handler(include defer_bh). + */ + usb_get_urb(urb); spin_unlock_irqrestore(&q->lock, flags); // during some PM-driven resume scenarios,