From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756252AbcDNOpO (ORCPT ); Thu, 14 Apr 2016 10:45:14 -0400 Received: from mx2.suse.de ([195.135.220.15]:42585 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755835AbcDNOpL (ORCPT ); Thu, 14 Apr 2016 10:45:11 -0400 Date: Thu, 14 Apr 2016 16:45:08 +0200 Message-ID: From: Takashi Iwai To: Marcel Holtmann Cc: Jiri Slaby , Johan Hedberg , LKML , "Gustavo F. Padovan" , linux-bluetooth@vger.kernel.org, "stable 3 . 13+" Subject: Re: [PATCH] Bluetooth: purge unhandled skbs In-Reply-To: References: <1458384583-10005-1-git-send-email-jslaby@suse.cz> <90C446FD-8335-4AD7-B4A2-4DEAE610D33A@holtmann.org> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 Emacs/24.5 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 14 Apr 2016 16:41:34 +0200, Marcel Holtmann wrote: > > Hi Takashi, > > >>> The write handler allocates skbs and queues them into data->readq. > >>> Read side should read them, if there is any. If there is none, skbs > >>> should be dropped by hdev->flush. But this happens only if the device > >>> is HCI_UP, i.e. hdev->power_on work was triggered already. When it was > >>> not, skbs stay allocated in the queue when /dev/vhci is closed. So > >>> purge the queue in ->release. > >>> > >>> Program to reproduce: > >>> #include > >>> #include > >>> #include > >>> #include > >>> > >>> #include > >>> #include > >>> #include > >>> > >>> int main() > >>> { > >>> char buf[] = { 0xff, 0 }; > >>> struct iovec iov = { > >>> .iov_base = buf, > >>> .iov_len = sizeof(buf), > >>> }; > >>> int fd; > >>> > >>> while (1) { > >>> fd = open("/dev/vhci", O_RDWR); > >>> if (fd < 0) > >>> err(1, "open"); > >>> > >>> usleep(50); > >>> > >>> if (writev(fd, &iov, 1) < 0) > >>> err(1, "writev"); > >>> > >>> usleep(50); > >>> > >>> close(fd); > >>> } > >>> > >>> return 0; > >>> } > >>> > >>> Result: > >>> kmemleak: 4609 new suspected memory leaks > >>> unreferenced object 0xffff88059f4d5440 (size 232): > >>> comm "vhci", pid 1084, jiffies 4294912542 (age 37569.296s) > >>> hex dump (first 32 bytes): > >>> 20 f0 23 87 05 88 ff ff 20 f0 23 87 05 88 ff ff .#..... .#..... > >>> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ > >>> backtrace: > >>> ... > >>> [] __alloc_skb+0x0/0x5a0 > >>> [] vhci_create_device+0x5c/0x580 [hci_vhci] > >>> [] vhci_write+0x306/0x4c8 [hci_vhci] > >>> > >>> Fixes: 23424c0d31 (Bluetooth: Add support creating virtual AMP controllers) > >>> Signed-off-by: Jiri Slaby > >>> Cc: Marcel Holtmann > >>> Cc: Gustavo Padovan > >>> Cc: Johan Hedberg > >>> Cc: > >>> Cc: stable 3.13+ > >>> --- > >>> drivers/bluetooth/hci_vhci.c | 1 + > >>> 1 file changed, 1 insertion(+) > >> > >> patch has been applied to bluetooth-next tree. > > > > Could you check another race of vhci mentioned in the thread? > > https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1106134.html > > > > If the suggested fix is OK, I'll submit a properly cooked patch. > > lets get the patch send to the mailing list and then I can have a look at it. I might need to test the final patch so that it works with legacy userspace. Alright, will submit it soon later. thanks, Takashi