[PATCH] Bluetooth: vhci, fix open_timeout vs. hdev race

[Jiri Slaby]

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 <jsl...@suse.cz>
Cc: Marcel Holtmann <mar...@holtmann.org>
Cc: Gustavo Padovan <gust...@padovan.org>
Cc: Johan Hedberg <johan.hedb...@gmail.com>
Cc: <linux-blueto...@vger.kernel.org>
Cc: Dmitry Vyukov <dvyu...@google.com>
Cc: stable 3.13+ <sta...@vger.kernel.org>
---
 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