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 S1755676AbcDNOgk (ORCPT <rfc822;w@1wt.eu>);
	Thu, 14 Apr 2016 10:36:40 -0400
Received: from mx2.suse.de ([195.135.220.15]:41569 "EHLO mx2.suse.de"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1755590AbcDNOgh (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
	Thu, 14 Apr 2016 10:36:37 -0400
Date: Thu, 14 Apr 2016 16:36:33 +0200
Message-ID: <s5htwj4i7f2.wl-tiwai@suse.de>
From: Takashi Iwai <tiwai@suse.de>
To: Marcel Holtmann <marcel@holtmann.org>
Cc: Jiri Slaby <jslaby@suse.cz>, Johan Hedberg <johan.hedberg@gmail.com>,
        linux-kernel@vger.kernel.org,
        "Gustavo F. Padovan" <gustavo@padovan.org>,
        linux-bluetooth@vger.kernel.org,
        "stable 3 . 13+" <stable@vger.kernel.org>
Subject: Re: [PATCH] Bluetooth: purge unhandled skbs
In-Reply-To: <90C446FD-8335-4AD7-B4A2-4DEAE610D33A@holtmann.org>
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: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Fri, 08 Apr 2016 19:23:45 +0200,
Marcel Holtmann wrote:
> 
> Hi Jiri,
> 
> > 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 <err.h>
> > 	#include <fcntl.h>
> > 	#include <stdio.h>
> > 	#include <unistd.h>
> > 
> > 	#include <sys/stat.h>
> > 	#include <sys/types.h>
> > 	#include <sys/uio.h>
> > 
> > 	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:
> > ...
> >    [<ffffffff81ece010>] __alloc_skb+0x0/0x5a0
> >    [<ffffffffa021886c>] vhci_create_device+0x5c/0x580 [hci_vhci]
> >    [<ffffffffa0219436>] vhci_write+0x306/0x4c8 [hci_vhci]
> > 
> > Fixes: 23424c0d31 (Bluetooth: Add support creating virtual AMP controllers)
> > Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> > Cc: Marcel Holtmann <marcel@holtmann.org>
> > Cc: Gustavo Padovan <gustavo@padovan.org>
> > Cc: Johan Hedberg <johan.hedberg@gmail.com>
> > Cc: <linux-bluetooth@vger.kernel.org>
> > Cc: stable 3.13+ <stable@vger.kernel.org>
> > ---
> > 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.


thanks,

Takashi