From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1751342AbdGQP31 (ORCPT <rfc822;w@1wt.eu>);
        Mon, 17 Jul 2017 11:29:27 -0400
Received: from mx2.suse.de ([195.135.220.15]:46161 "EHLO mx1.suse.de"
        rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP
        id S1751280AbdGQP30 (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
        Mon, 17 Jul 2017 11:29:26 -0400
Message-ID: <1500305216.16072.2.camel@suse.com>
Subject: Re: [RFC PATCH] Bluetooth: btusb: Fix memory leak in play_deferred
From: Oliver Neukum <oneukum@suse.com>
To: jeffy <jeffy.chen@rock-chips.com>,
        Marcel Holtmann <marcel@holtmann.org>
Cc: LKML <linux-kernel@vger.kernel.org>, xiyou.wangcong@gmail.com,
        Brian Norris <briannorris@chromium.org>,
        Douglas Anderson <dianders@chromium.org>,
        Johan Hedberg <johan.hedberg@gmail.com>,
        "Gustavo F. Padovan" <gustavo@padovan.org>
Date: Mon, 17 Jul 2017 17:26:56 +0200
In-Reply-To: <596588F8.5070402@rock-chips.com>
References: <1498126243-4771-1-git-send-email-jeffy.chen@rock-chips.com>
         <65CF80EF-EA58-4126-889D-07A8FF9D52DB@holtmann.org>
         <594C8F0D.4000100@rock-chips.com> <1499168300.17946.3.camel@neukum.org>
         <596588F8.5070402@rock-chips.com>
Content-Type: text/plain; charset="UTF-8"
X-Mailer: Evolution 3.20.5 
Mime-Version: 1.0
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Am Mittwoch, den 12.07.2017, 10:27 +0800 schrieb jeffy:
> Hi Oliver,
> 
> Thanx for your comments, and sorry for reply late.
> 
> 
> > If you do that you have to change submit_tx_urb() to be called under a
> > spinlock.
> 
> sorry, why we need that? since submit_tx_urb is basically 
> usb_anchor_urb/usb_submit_urb/usb_free_urb

You need to fix the GFP_KERNEL therein.

> > > or referenced, but the caller would unref it himself
> > > later?
> > 
> > The caller is responsible for its own references.
> hmm, maybe unref it in the complete callback(btusb_tx_complete?), and if 
> we do so, we may need to detect which urb came from here...

I do not get your reasoning there. If an URB has executed, it belongs
onto the anchor for URBs to be used again. 

> > > and for tx_anchor, we put urb in it, and kill them all during suspending
> > > to prevent transfer. so i guess it would be safe to put deferred urb in
> > > to it after resume too?
> > > but i don't know much about usb/btusb, so i could be wrong all about that :)
> > 
> > IIRC the reason for directly submitting them was the spinlock.
> sorry, i'm not clear about this, could you help to explain more? do you 
> mean txlock?

Yes

	Regards
		Oliver