Re: Linux 2.4.27rc3 and Bluetooth

Re: Linux 2.4.27rc3 and Bluetooth

[J.D.]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 3326 bytes --]

Marcel,

Great!  I'll CC this message to the mailing list.

Regarding the bt3c_cs error message, I merely have a personal 
preference for error messages *without* a kernel panic. (aiee!)
I do appreciate you sharing your view on the specific problem.

I finally burned some time, studied the code, and created a
working patch for bt3c_cs.c (patch attached).  It adds a test
in bt3c_release to check whether the DEV_CONFIG_PENDING bit of
link->state is clear before a call to bt3c_close.  If the
DEV_CONFIG_PENDING bit is not clear, bt3c_open has failed,
and never succeeded in a call to hci_register_dev and friends.

When loading the firmware fails, the unpatched code ignores the
failure of bt3c_open, goes ahead with bt3c_close, and invokes 
hci_unregister_dev and friends (despite the fact that
hci_register_dev was never called), thus provoking a kernel panic
with the NULL pointer dereference. (aiee!)

I suspect the 2.6 kernel may have the same problem by casual
inspection, but I have neither confirmed the existence of a problem
through testing of the 2.6 kernel, nor carefully traced that code
of Linux 2.6 for possible key differences.

With Linux 2.4 and the patch, I was able to temporarily erase 
"/usr/lib/hotplug/firmware/BT3CPCC.bin", and insert the Bluetooth
card with a kernel error message, but no panic.  After replacing
the file, and re-inserting the card, everything worked again.

(BTW, thank you Marcel for the tip on disabling the SCO support;
network transfers via bluetooth do not stall on that machine like
they did with SCO enabled.)

Kind regards,

- John

--
http://jodarom.sdf1.org/



On Sat, 17 Jul 2004, Marcel Holtmann wrote:

> Hi John,
>
>> Thank you for the information.  I just wrote you because your name was on
>> the patch that caused my kernel to panic.  What's the mailing list of
>> choice for bluetooth bugs in the Linux kernel?
>
> check the MAINTAINERS file in the Linux kernel source code. It says the
> BlueZ developer mailing list for the Bluetooth subsystem.
>
>> I issued the commands,
>>
>>    cd /usr/lib/hotplug/firmware
>>    ln -s /etc/bluetooth/firmware/BT3CPCC.bin BT3CPCC.bin
>>
>> and all was happy again.
>>
>> If you ever feel the initiative, making the system spit out a somewhat
>> succinct and relevant error message, as opposed to a full kernel panic
>> (aiee!) is much preferred by Linux users who desire to maintain the
>> illusion that Linux is a stable operating system.  =)
>
> the error message was there:
>
> 	bt3c_cs: Firmware request failed.
> 	Unable to handle kernel NULL pointer ...
>
> So there must be a NULL pointer somewhere when the firmware can't be
> loaded into kernel space. I don't know if it is in the Bluetooth or the
> PCMCIA subsystem. I have no 2.4 system where I can check this.
>
>> ... but personally I'm satisfied at the moment.  At least on a different
>> machine (running 2.4.27rc2), when the bluetooth USB device randomly locks
>> during repeated or extended transmissions, a nice message appears in the
>> log, ("kernel: NETDEV WATCHDOG: bnep0: transmit timed out"), and the
>> device works again after physically being unplugged and plugged back in
>> again.
>
> Maybe it helps if you compile your kernel without the SCO support option
> for the hci_usb driver. Disable CONFIG_BLUEZ_HCIUSB_SCO.
>
> Regards
>
> Marcel
>
>

[-- Attachment #2: kernel patch - bt3c_cs - missing firmware?  Don't panic! --]
[-- Type: TEXT/PLAIN, Size: 504 bytes --]

diff -urN kernel-source-2.4.27-rc3.orig/drivers/bluetooth/bt3c_cs.c kernel-source-2.4.27-rc3/drivers/bluetooth/bt3c_cs.c
--- kernel-source-2.4.27-rc3.orig/drivers/bluetooth/bt3c_cs.c	Sat Jun 26 03:53:03 2004
+++ kernel-source-2.4.27-rc3/drivers/bluetooth/bt3c_cs.c	Fri Jul 23 23:16:03 2004
@@ -896,7 +896,8 @@
 	bt3c_info_t *info = link->priv;
 
 	if (link->state & DEV_PRESENT)
-		bt3c_close(info);
+		if (!(link->state & DEV_CONFIG_PENDING))
+			bt3c_close(info);
 
 	MOD_DEC_USE_COUNT;
 

[Bluez-devel] Re: Linux 2.4.27rc3 and Bluetooth

[Marcel Holtmann]

[-- Attachment #1: Type: text/plain, Size: 1220 bytes --]

Hi John,

> I finally burned some time, studied the code, and created a
> working patch for bt3c_cs.c (patch attached).  It adds a test
> in bt3c_release to check whether the DEV_CONFIG_PENDING bit of
> link->state is clear before a call to bt3c_close.  If the
> DEV_CONFIG_PENDING bit is not clear, bt3c_open has failed,
> and never succeeded in a call to hci_register_dev and friends.
> 
> When loading the firmware fails, the unpatched code ignores the
> failure of bt3c_open, goes ahead with bt3c_close, and invokes 
> hci_unregister_dev and friends (despite the fact that
> hci_register_dev was never called), thus provoking a kernel panic
> with the NULL pointer dereference. (aiee!)

you are right. That is the problem. I fixed it in a different way.
Please try out the attached patch and report if it also works.

> I suspect the 2.6 kernel may have the same problem by casual
> inspection, but I have neither confirmed the existence of a problem
> through testing of the 2.6 kernel, nor carefully traced that code
> of Linux 2.6 for possible key differences.

No. The 2.6 kernel is not affected, because the hdev is a pointer in
that kernel series and we have the NULL pointer check in place.

Regards

Marcel


[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 1688 bytes --]

===== drivers/bluetooth/dtl1_cs.c 1.2 vs edited =====
--- 1.2/drivers/bluetooth/dtl1_cs.c	2002-08-03 13:53:56 +02:00
+++ edited/drivers/bluetooth/dtl1_cs.c	2004-07-24 22:30:33 +02:00
@@ -535,6 +535,9 @@
 	unsigned int iobase = info->link.io.BasePort1;
 	struct hci_dev *hdev = &(info->hdev);
 
+	if (info->link.state & DEV_CONFIG_PENDING)
+		return -ENODEV;
+
 	dtl1_hci_close(hdev);
 
 	spin_lock_irqsave(&(info->lock), flags);
===== drivers/bluetooth/bluecard_cs.c 1.1 vs edited =====
--- 1.1/drivers/bluetooth/bluecard_cs.c	2002-08-03 13:54:07 +02:00
+++ edited/drivers/bluetooth/bluecard_cs.c	2004-07-24 22:30:53 +02:00
@@ -803,6 +803,9 @@
 	unsigned int iobase = info->link.io.BasePort1;
 	struct hci_dev *hdev = &(info->hdev);
 
+	if (info->link.state & DEV_CONFIG_PENDING)
+		return -ENODEV;
+
 	bluecard_hci_close(hdev);
 
 	clear_bit(CARD_READY, &(info->hw_state));
===== drivers/bluetooth/bt3c_cs.c 1.2 vs edited =====
--- 1.2/drivers/bluetooth/bt3c_cs.c	2004-04-16 14:05:25 +02:00
+++ edited/drivers/bluetooth/bt3c_cs.c	2004-07-24 22:31:11 +02:00
@@ -643,6 +643,9 @@
 {
 	struct hci_dev *hdev = &(info->hdev);
 
+	if (info->link.state & DEV_CONFIG_PENDING)
+		return -ENODEV;
+
 	bt3c_hci_close(hdev);
 
 	if (hci_unregister_dev(hdev) < 0)
===== drivers/bluetooth/btuart_cs.c 1.1 vs edited =====
--- 1.1/drivers/bluetooth/btuart_cs.c	2002-10-20 21:42:54 +02:00
+++ edited/drivers/bluetooth/btuart_cs.c	2004-07-24 22:30:46 +02:00
@@ -556,6 +556,9 @@
 	unsigned int iobase = info->link.io.BasePort1;
 	struct hci_dev *hdev = &(info->hdev);
 
+	if (info->link.state & DEV_CONFIG_PENDING)
+		return -ENODEV;
+
 	btuart_hci_close(hdev);
 
 	spin_lock_irqsave(&(info->lock), flags);

Re: Linux 2.4.27rc3 and Bluetooth

[John Dahlstrom]

Marcel,

Yes, your new patch works!

When the firmware file is removed and the Bluetooth card is inserted,
the machine humbly protests as expected, without a kernel panic.
Then, with the file copied back into place, everything runs smoothly
upon subsequent insertions.

Thank you again for your assistance.

Kind regards,

- John

--
http://jodarom.sdf1.org/



On Sat, 24 Jul 2004, Marcel Holtmann wrote:

> Hi John,
>
>> [...]
>
> you are right. That is the problem. I fixed it in a different way.
> Please try out the attached patch and report if it also works.
>
>> [...]
>
> No. The 2.6 kernel is not affected, because the hdev is a pointer in
> that kernel series and we have the NULL pointer check in place.
>
> Regards
>
> Marcel
>