[PATCH] Bluetooth: vhci, fix open_timeout vs. hdev race
From: Jiri Slaby <hidden>
Date: 2016-03-19 09:47:44
Also in:
lkml, stable
Subsystem:
bluetooth drivers, the rest · Maintainers:
Marcel Holtmann, Luiz Augusto von Dentz, Linus Torvalds
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 <err.h>
#include <fcntl.h>
#include <stdio.h>
#include <stdlib.h>
#include <time.h>
#include <unistd.h>
#include <sys/stat.h>
#include <sys/types.h>
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 <redacted>
Cc: Marcel Holtmann <marcel@holtmann.org>
Cc: Gustavo Padovan <redacted>
Cc: Johan Hedberg <redacted>
Cc: <redacted>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: stable 3.13+ <redacted>
---
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