linux-kernel - Re: [BUG] 2.6.30-rc4 hid bluetooth not working

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <a8e1da0905020708j54cda260tac256469498f6ce3@mail.gmail.com>
Date:	Sat, 2 May 2009 22:08:50 +0800
From:	Dave Young <hidave.darkstar@...il.com>
To:	Marc Pignat <Marc.Pignat@...s.ch>
Cc:	marcel@...tmann.org, jkosina@...e.cz, ext-roger.quadros@...ia.com,
	linux-bluetooth@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [BUG] 2.6.30-rc4 hid bluetooth not working

On Sat, May 2, 2009 at 4:45 PM, Marc Pignat <Marc.Pignat@...s.ch> wrote:
>>>> Marcel Holtmann <marcel@...tmann.org> 05/02/09 12:57 AM >>>
>>Hi Jiri,
>>
>>> > Subject: bluetooth: Fix serialization when adding/deleting connections in hci_sysfs
>>> >
>>> > add_conn and del_conn should be serialized, but flush_workqueue can't be used
>>> > by the worker thread on it's own queue, so use flush_work to serialize add_conn
>>> > and del_conn against each other.
>>> >
>>> > Signed-off-by: Marc Pignat <marc.pignat@...s.ch>
>>>
>>> Acked-by: Jiri Kosina <jkosina@...e.cz>
>>>
>>> FWIW.
>>
>>nak from my side since I think it is the wrong fix. We really wanna wait
>>for all works to finish here. This includes work from other connection
>>attempts or terminations.
>
> IMHO, there is no need to wait for work currently running, since this is a
> singlethread workqueue.

Yes, sounds right.

>
> But it is perhaps simpler to use a lock (mutex or watherver locking primitive).

I'm here a little bit late. Marcel, I'm quite busy recently, I just
see the commit and then this thread.

Let me explain why I add two workqueue originally,  because workqueue
will be defered, so   we must guarantee "connection deletion" finished
before "connection adding with same bt addr",  or the "connection
adding" will fail.

On the other hand flush "adding" workqueue in "connection deletion"
function is not necessary.

To fix this bug, I think we can just use the two work struct for
add/del, at the same time keeping the original two workqueue.

Please see following patch for this, (building-test only, I have no
bluetooth device at hand, I can test this the day after tommorrow)

--- linux-2.6.orig/net/bluetooth/hci_sysfs.c	2009-04-30 11:35:54.000000000 +0800
+++ linux-2.6/net/bluetooth/hci_sysfs.c	2009-05-02 21:54:40.000000000 +0800
@@ -9,7 +9,8 @@
 struct class *bt_class = NULL;
 EXPORT_SYMBOL_GPL(bt_class);

-static struct workqueue_struct *bluetooth;
+static struct workqueue_struct *btaddconn;
+static struct workqueue_struct *btdelconn;

 static inline char *link_typetostr(int type)
 {
@@ -89,8 +90,7 @@
 {
 	struct hci_conn *conn = container_of(work, struct hci_conn, work_add);

-	/* ensure previous add/del is complete */
-	flush_workqueue(bluetooth);
+	flush_workqueue(btdelconn);

 	if (device_add(&conn->dev) < 0) {
 		BT_ERR("Failed to register connection device");
@@ -116,7 +116,7 @@

 	INIT_WORK(&conn->work_add, add_conn);

-	queue_work(bluetooth, &conn->work_add);
+	queue_work(btaddconn, &conn->work_add);
 }

 /*
@@ -134,9 +134,6 @@
 	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);
-
 	while (1) {
 		struct device *dev;

@@ -161,7 +158,7 @@

 	INIT_WORK(&conn->work_del, del_conn);

-	queue_work(bluetooth, &conn->work_del);
+	queue_work(btdelconn, &conn->work_del);
 }

 static inline char *host_typetostr(int type)
@@ -438,13 +435,20 @@

 int __init bt_sysfs_init(void)
 {
-	bluetooth = create_singlethread_workqueue("bluetooth");
-	if (!bluetooth)
+	btaddconn = create_singlethread_workqueue("btaddconn");
+	if (!btaddconn)
+		return -ENOMEM;
+
+	btdelconn = create_singlethread_workqueue("btdelconn");
+	if (!btdelconn) {
+		destroy_workqueue(btaddconn);
 		return -ENOMEM;
+	}

 	bt_class = class_create(THIS_MODULE, "bluetooth");
 	if (IS_ERR(bt_class)) {
-		destroy_workqueue(bluetooth);
+		destroy_workqueue(btdelconn);
+		destroy_workqueue(btaddconn);
 		return PTR_ERR(bt_class);
 	}

@@ -453,7 +457,8 @@

 void bt_sysfs_cleanup(void)
 {
-	destroy_workqueue(bluetooth);
+	destroy_workqueue(btaddconn);
+	destroy_workqueue(btdelconn);

 	class_destroy(bt_class);
 }
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/