From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932181AbcCSJro (ORCPT ); Sat, 19 Mar 2016 05:47:44 -0400 Received: from mx2.suse.de ([195.135.220.15]:50224 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752200AbcCSJrf (ORCPT ); Sat, 19 Mar 2016 05:47:35 -0400 From: Jiri Slaby To: johan.hedberg@gmail.com Cc: linux-kernel@vger.kernel.org, Jiri Slaby , Marcel Holtmann , Gustavo Padovan , linux-bluetooth@vger.kernel.org, Dmitry Vyukov , "stable 3 . 13+" Subject: [PATCH] Bluetooth: vhci, fix open_timeout vs. hdev race Date: Sat, 19 Mar 2016 10:47:32 +0100 Message-Id: <1458380852-7533-1-git-send-email-jslaby@suse.cz> X-Mailer: git-send-email 2.7.4 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Both vhci_get_user and vhci_release race with open_timeout work. They both contain cancel_delayed_work_sync, but do not test whether the work actually created hdev or not. Since the work can be in progress and _sync will wait for finishing it, we can have data->hdev allocated when cancel_delayed_work_sync returns. But the call sites do 'if (data->hdev)' *before* cancel_delayed_work_sync. As a result: * vhci_get_user allocates a second hdev and puts it into data->hdev. The former is leaked. * vhci_release does not release data->hdev properly as it thinks there is none. Fix both cases by moving the actual test *after* the call to cancel_delayed_work_sync. This can be hit by this program: #include #include #include #include #include #include #include #include int main(int argc, char **argv) { char buf[] = { 0xff, 0 }; struct iovec iov = { .iov_base = buf, .iov_len = sizeof(buf), }; int fd; srand(time(NULL)); while (1) { const int delta = (rand() % 200 - 100) * 100; fd = open("/dev/vhci", O_RDWR); if (fd < 0) err(1, "open"); usleep(100000 + delta); close(fd); } return 0; } 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: Dmitry Vyukov Cc: stable 3.13+ --- drivers/bluetooth/hci_vhci.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/bluetooth/hci_vhci.c b/drivers/bluetooth/hci_vhci.c index 7287d774e107..f67ea1c090cb 100644 --- a/drivers/bluetooth/hci_vhci.c +++ b/drivers/bluetooth/hci_vhci.c @@ -189,13 +189,13 @@ static inline ssize_t vhci_get_user(struct vhci_data *data, break; case HCI_VENDOR_PKT: + cancel_delayed_work_sync(&data->open_timeout); + if (data->hdev) { kfree_skb(skb); return -EBADFD; } - cancel_delayed_work_sync(&data->open_timeout); - opcode = *((__u8 *) skb->data); skb_pull(skb, 1); @@ -333,10 +333,12 @@ static int vhci_open(struct inode *inode, struct file *file) static int vhci_release(struct inode *inode, struct file *file) { struct vhci_data *data = file->private_data; - struct hci_dev *hdev = data->hdev; + struct hci_dev *hdev; cancel_delayed_work_sync(&data->open_timeout); + hdev = data->hdev; + if (hdev) { hci_unregister_dev(hdev); hci_free_dev(hdev); -- 2.7.4