Bluetooth: Fix RFCOMM release oops when device is still in use

It turns out that the following sequence of actions will reproduce the
oops:

  1. Create a new RFCOMM device (using RFCOMMCREATEDEV ioctl)
  2. (Try to) open the device
  3. Release the RFCOMM device (using RFCOMMRELEASEDEV ioctl)

At this point, the "/dev/rfcomm*" device is still in use, but it is gone
from the internal list, so the device id can be reused.

  4. Create a new RFCOMM device with the same device id as before

And now kobject will complain that the TTY already exists.

(See http://lkml.org/lkml/2008/7/13/89 for a reproducible test-case.)

This patch attempts to correct this by only removing the device from the
internal list of devices at the final unregister stage, so that the id
won't get reused until the device has been completely destructed.

This should be safe as the RFCOMM_TTY_RELEASED bit will be set for the
device and prevent the device from being reopened after it has been
released.

Based on a report from Vegard Nossum <vegard.nossum@gmail.com>

Signed-off-by: Marcel Holtmann <marcel@holtmann.org>

diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index 1e4100b..111c6c8 100644 (file)
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -53,7 +53,7 @@ struct rfcomm_dev {
        char                    name[12];
        int                     id;
        unsigned long           flags;
        char                    name[12];
        int                     id;
        unsigned long           flags;

-       int                     opened;
+       atomic_t                opened;

        int                     err;
 
        bdaddr_t                src;
        int                     err;
 
        bdaddr_t                src;


@@ -256,6 +256,8 @@ static int rfcomm_dev_add(struct rfcomm_dev_req *req, struct rfcomm_dlc *dlc)
        dev->flags = req->flags &
                ((1 << RFCOMM_RELEASE_ONHUP) | (1 << RFCOMM_REUSE_DLC));
 
        dev->flags = req->flags &
                ((1 << RFCOMM_RELEASE_ONHUP) | (1 << RFCOMM_REUSE_DLC));
 

+       atomic_set(&dev->opened, 0);
+

        init_waitqueue_head(&dev->wait);
        tasklet_init(&dev->wakeup_task, rfcomm_tty_wakeup, (unsigned long) dev);
 
        init_waitqueue_head(&dev->wait);
        tasklet_init(&dev->wakeup_task, rfcomm_tty_wakeup, (unsigned long) dev);
 


@@ -325,10 +327,10 @@ static void rfcomm_dev_del(struct rfcomm_dev *dev)
 {
        BT_DBG("dev %p", dev);
 
 {
        BT_DBG("dev %p", dev);
 

-       if (test_bit(RFCOMM_TTY_RELEASED, &dev->flags))
-               BUG_ON(1);
-       else
-               set_bit(RFCOMM_TTY_RELEASED, &dev->flags);
+       BUG_ON(test_and_set_bit(RFCOMM_TTY_RELEASED, &dev->flags));
+
+       if (atomic_read(&dev->opened) > 0)
+               return;

 
        write_lock_bh(&rfcomm_dev_lock);
        list_del_init(&dev->list);
 
        write_lock_bh(&rfcomm_dev_lock);
        list_del_init(&dev->list);


@@ -684,9 +686,10 @@ static int rfcomm_tty_open(struct tty_struct *tty, struct file *filp)
        if (!dev)
                return -ENODEV;
 
        if (!dev)
                return -ENODEV;
 

-       BT_DBG("dev %p dst %s channel %d opened %d", dev, batostr(&dev->dst), dev->channel, dev->opened);
+       BT_DBG("dev %p dst %s channel %d opened %d", dev, batostr(&dev->dst),
+                               dev->channel, atomic_read(&dev->opened));

 
 

-       if (dev->opened++ != 0)
+       if (atomic_inc_return(&dev->opened) > 1)

                return 0;
 
        dlc = dev->dlc;
                return 0;
 
        dlc = dev->dlc;


@@ -742,9 +745,10 @@ static void rfcomm_tty_close(struct tty_struct *tty, struct file *filp)
        if (!dev)
                return;
 
        if (!dev)
                return;
 

-       BT_DBG("tty %p dev %p dlc %p opened %d", tty, dev, dev->dlc, dev->opened);
+       BT_DBG("tty %p dev %p dlc %p opened %d", tty, dev, dev->dlc,
+                                               atomic_read(&dev->opened));

 
 

-       if (--dev->opened == 0) {
+       if (atomic_dec_and_test(&dev->opened)) {

                if (dev->tty_dev->parent)
                        device_move(dev->tty_dev, NULL);
 
                if (dev->tty_dev->parent)
                        device_move(dev->tty_dev, NULL);
 


@@ -758,6 +762,14 @@ static void rfcomm_tty_close(struct tty_struct *tty, struct file *filp)
                tty->driver_data = NULL;
                dev->tty = NULL;
                rfcomm_dlc_unlock(dev->dlc);
                tty->driver_data = NULL;
                dev->tty = NULL;
                rfcomm_dlc_unlock(dev->dlc);

+
+               if (test_bit(RFCOMM_TTY_RELEASED, &dev->flags)) {
+                       write_lock_bh(&rfcomm_dev_lock);
+                       list_del_init(&dev->list);
+                       write_unlock_bh(&rfcomm_dev_lock);
+
+                       rfcomm_dev_put(dev);
+               }

        }
 
        rfcomm_dev_put(dev);
        }
 
        rfcomm_dev_put(dev);