diff options
| author | Peter Hurley <peter@hurleysoftware.com> | 2014-02-09 20:59:15 -0500 |
|---|---|---|
| committer | Marcel Holtmann <marcel@holtmann.org> | 2014-02-14 13:39:31 -0800 |
| commit | b92483d54abb4ff288accc36bf1daef44dea9fbe (patch) | |
| tree | 784f48a3535ba09ef487d580bc5a31d330ac246a /net/bluetooth | |
| parent | c4fd318d6ebc16bd64ca7b9c06f21b7f33bb462e (diff) | |
Bluetooth: Fix unsafe RFCOMM device parenting
Accessing the results of hci_conn_hash_lookup_ba() is unsafe without
holding the hci_dev_lock() during the lookup. For example:
CPU 0 | CPU 1
hci_conn_hash_lookup_ba | hci_conn_del
rcu_read_lock | hci_conn_hash_del
list_for_each_entry_rcu | list_del_rcu
if (.....) | synchronize_rcu
rcu_read_unlock |
| hci_conn_del_sysfs
| hci_dev_put
| hci_conn_put
| put_device (last reference)
| bt_link_release
| kfree(conn)
return p << just freed |
Even if a hci_conn reference were taken (via hci_conn_get), would
not guarantee the lifetime of the sysfs device, but only safe
access to the in-memory structure.
Ensure the hci_conn device stays valid while the rfcomm device
is reparented; rename rfcomm_get_device() to rfcomm_reparent_device()
and perform the reparenting within the function while holding the
hci_dev_lock.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Tested-By: Alexander Holler <holler@ahsoftware.de>
Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
Diffstat (limited to 'net/bluetooth')
| -rw-r--r-- | net/bluetooth/rfcomm/tty.c | 20 |
1 files changed, 14 insertions, 6 deletions
diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c index a58d693e1e61..2975bc4b9188 100644 --- a/net/bluetooth/rfcomm/tty.c +++ b/net/bluetooth/rfcomm/tty.c @@ -167,20 +167,29 @@ static struct rfcomm_dev *rfcomm_dev_get(int id) return dev; } -static struct device *rfcomm_get_device(struct rfcomm_dev *dev) +static void rfcomm_reparent_device(struct rfcomm_dev *dev) { struct hci_dev *hdev; struct hci_conn *conn; hdev = hci_get_route(&dev->dst, &dev->src); if (!hdev) - return NULL; + return; + /* The lookup results are unsafe to access without the + * hci device lock (FIXME: why is this not documented?) + */ + hci_dev_lock(hdev); conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK, &dev->dst); - hci_dev_put(hdev); + /* Just because the acl link is in the hash table is no + * guarantee the sysfs device has been added ... + */ + if (conn && device_is_registered(&conn->dev)) + device_move(dev->tty_dev, &conn->dev, DPM_ORDER_DEV_AFTER_PARENT); - return conn ? &conn->dev : NULL; + hci_dev_unlock(hdev); + hci_dev_put(hdev); } static ssize_t show_address(struct device *tty_dev, struct device_attribute *attr, char *buf) @@ -589,8 +598,7 @@ static void rfcomm_dev_state_change(struct rfcomm_dlc *dlc, int err) dev->err = err; if (dlc->state == BT_CONNECTED) { - device_move(dev->tty_dev, rfcomm_get_device(dev), - DPM_ORDER_DEV_AFTER_PARENT); + rfcomm_reparent_device(dev); wake_up_interruptible(&dev->port.open_wait); } else if (dlc->state == BT_CLOSED) |