aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMarcel Holtmann <marcel@holtmann.org>2009-05-02 18:24:06 -0700
committerMarcel Holtmann <marcel@holtmann.org>2009-05-04 14:29:02 -0700
commita67e899cf38ae542d1a028ccd021f9189f76fb74 (patch)
treed0bb79ccbdd5737745e99acbc569605bc74bc446
parent9f722c0978b04acba209f8ca1896ad05814bc3a3 (diff)
Bluetooth: Fix issue with sysfs handling for connections
Due to a semantic changes in flush_workqueue() the current approach of synchronizing the sysfs handling for connections doesn't work anymore. The whole approach is actually fully broken and based on assumptions that are no longer valid. With the introduction of Simple Pairing support, the creation of low-level ACL links got changed. This change invalidates the reason why in the past two independent work queues have been used for adding/removing sysfs devices. The adding of the actual sysfs device is now postponed until the host controller successfully assigns an unique handle to that link. So the real synchronization happens inside the controller and not the host. The only left-over problem is that some internals of the sysfs device handling are not initialized ahead of time. This leaves potential access to invalid data and can cause various NULL pointer dereferences. To fix this a new function makes sure that all sysfs details are initialized when an connection attempt is made. The actual sysfs device is only registered when the connection has been successfully established. To avoid a race condition with the registration, the check if a device is registered has been moved into the removal work. As an extra protection two flush_work() calls are left in place to make sure a previous add/del work has been completed first. Based on a report by Marc Pignat <marc.pignat@hevs.ch> Signed-off-by: Marcel Holtmann <marcel@holtmann.org> Tested-by: Justin P. Mattock <justinmattock@gmail.com> Tested-by: Roger Quadros <ext-roger.quadros@nokia.com> Tested-by: Marc Pignat <marc.pignat@hevs.ch>
-rw-r--r--include/net/bluetooth/hci_core.h1
-rw-r--r--net/bluetooth/hci_conn.c2
-rw-r--r--net/bluetooth/hci_sysfs.c74
3 files changed, 43 insertions, 34 deletions
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index be5bd713d2c..73aead222b3 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -457,6 +457,7 @@ int hci_recv_fragment(struct hci_dev *hdev, int type, void *data, int count);
int hci_register_sysfs(struct hci_dev *hdev);
void hci_unregister_sysfs(struct hci_dev *hdev);
+void hci_conn_init_sysfs(struct hci_conn *conn);
void hci_conn_add_sysfs(struct hci_conn *conn);
void hci_conn_del_sysfs(struct hci_conn *conn);
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 375f4b4f7f7..61309b26f27 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -248,6 +248,8 @@ struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst)
if (hdev->notify)
hdev->notify(hdev, HCI_NOTIFY_CONN_ADD);
+ hci_conn_init_sysfs(conn);
+
tasklet_enable(&hdev->tx_task);
return conn;
diff --git a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c
index b7c51082dde..582d8877078 100644
--- a/net/bluetooth/hci_sysfs.c
+++ b/net/bluetooth/hci_sysfs.c
@@ -9,7 +9,7 @@
struct class *bt_class = NULL;
EXPORT_SYMBOL_GPL(bt_class);
-static struct workqueue_struct *bluetooth;
+static struct workqueue_struct *bt_workq;
static inline char *link_typetostr(int type)
{
@@ -89,8 +89,8 @@ static void add_conn(struct work_struct *work)
{
struct hci_conn *conn = container_of(work, struct hci_conn, work_add);
- /* ensure previous add/del is complete */
- flush_workqueue(bluetooth);
+ /* ensure previous del is complete */
+ flush_work(&conn->work_del);
if (device_add(&conn->dev) < 0) {
BT_ERR("Failed to register connection device");
@@ -98,27 +98,6 @@ static void add_conn(struct work_struct *work)
}
}
-void hci_conn_add_sysfs(struct hci_conn *conn)
-{
- struct hci_dev *hdev = conn->hdev;
-
- BT_DBG("conn %p", conn);
-
- conn->dev.type = &bt_link;
- conn->dev.class = bt_class;
- conn->dev.parent = &hdev->dev;
-
- dev_set_name(&conn->dev, "%s:%d", hdev->name, conn->handle);
-
- dev_set_drvdata(&conn->dev, conn);
-
- device_initialize(&conn->dev);
-
- INIT_WORK(&conn->work_add, add_conn);
-
- queue_work(bluetooth, &conn->work_add);
-}
-
/*
* The rfcomm tty device will possibly retain even when conn
* is down, and sysfs doesn't support move zombie device,
@@ -134,8 +113,11 @@ static void del_conn(struct work_struct *work)
struct hci_conn *conn = container_of(work, struct hci_conn, work_del);
struct hci_dev *hdev = conn->hdev;
- /* ensure previous add/del is complete */
- flush_workqueue(bluetooth);
+ /* ensure previous add is complete */
+ flush_work(&conn->work_add);
+
+ if (!device_is_registered(&conn->dev))
+ return;
while (1) {
struct device *dev;
@@ -152,16 +134,40 @@ static void del_conn(struct work_struct *work)
hci_dev_put(hdev);
}
-void hci_conn_del_sysfs(struct hci_conn *conn)
+void hci_conn_init_sysfs(struct hci_conn *conn)
{
+ struct hci_dev *hdev = conn->hdev;
+
BT_DBG("conn %p", conn);
- if (!device_is_registered(&conn->dev))
- return;
+ conn->dev.type = &bt_link;
+ conn->dev.class = bt_class;
+ conn->dev.parent = &hdev->dev;
+
+ dev_set_drvdata(&conn->dev, conn);
+ device_initialize(&conn->dev);
+
+ INIT_WORK(&conn->work_add, add_conn);
INIT_WORK(&conn->work_del, del_conn);
+}
+
+void hci_conn_add_sysfs(struct hci_conn *conn)
+{
+ struct hci_dev *hdev = conn->hdev;
+
+ BT_DBG("conn %p", conn);
+
+ dev_set_name(&conn->dev, "%s:%d", hdev->name, conn->handle);
+
+ queue_work(bt_workq, &conn->work_add);
+}
+
+void hci_conn_del_sysfs(struct hci_conn *conn)
+{
+ BT_DBG("conn %p", conn);
- queue_work(bluetooth, &conn->work_del);
+ queue_work(bt_workq, &conn->work_del);
}
static inline char *host_typetostr(int type)
@@ -438,13 +444,13 @@ void hci_unregister_sysfs(struct hci_dev *hdev)
int __init bt_sysfs_init(void)
{
- bluetooth = create_singlethread_workqueue("bluetooth");
- if (!bluetooth)
+ bt_workq = create_singlethread_workqueue("bluetooth");
+ if (!bt_workq)
return -ENOMEM;
bt_class = class_create(THIS_MODULE, "bluetooth");
if (IS_ERR(bt_class)) {
- destroy_workqueue(bluetooth);
+ destroy_workqueue(bt_workq);
return PTR_ERR(bt_class);
}
@@ -453,7 +459,7 @@ int __init bt_sysfs_init(void)
void bt_sysfs_cleanup(void)
{
- destroy_workqueue(bluetooth);
+ destroy_workqueue(bt_workq);
class_destroy(bt_class);
}