[PATCH] Fix possible crash on startup

[PATCH] Fix possible crash on startup

[Bastien Nocera]

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

I can provide the file that caused the crash, if needed.

Cheers

[-- Attachment #2: 0001-Fix-possible-crash-on-startup.patch --]
[-- Type: text/x-patch, Size: 2165 bytes --]

>From 8515a34842a0aee14d7d0c5725caf15d9d2d7308 Mon Sep 17 00:00:00 2001
From: Bastien Nocera <hadess@hadess.net>
Date: Sun, 12 Oct 2008 00:01:43 +0100
Subject: [PATCH] Fix possible crash on startup

0  0x0000000000a5cf71 in sdp_data_get (rec=0x0, attrId=0) at sdp.c:1444
1  0x000000008c704721 in hid_device_probe (device=0x7f1bed2a80b0, uuids=0x7f1bed29fa90) at manager.c:70
2  0x00007f1becb5bc33 in device_probe_drivers (device=0x7f1bed2a80b0, uuids=0x7f1bed2a2980) at device.c:647
3  0x00007f1becb58a23 in create_stored_device_from_profiles (key=0x7f1bed2a7360 "00:1E:45:AD:F1:96",
   value=0x7f1bed2a7aa0 "00000002-0000-1000-8000-0002ee000002 00001101-0000-1000-8000-00805f9b34fb 00001103-0000-1000-8000-00805f9b34fb 00001104-0000-1000-8000-00805f9b34fb 00001105-0000-1000-8000-00805f9b34fb 00001106-0000-1"..., user_data=0x7f1bed29edd0) at adapter.c:2296
4  0x00007f1becb61d12 in textfile_foreach (pathname=0x7ffff4b6f2a0 "/var/lib/bluetooth/00:13:EF:F1:42:B7/profiles", func=0x7f1becb58955 <create_stored_device_from_profiles>, data=0x7f1bed29edd0) at textfile.c:447
5  0x00007f1becb58b36 in load_devices (adapter=0x7f1bed29edd0) at adapter.c:2327
6  0x00007f1becb58fa0 in adapter_up (adapter=0x7f1bed29edd0, dd=18) at adapter.c:2462
7  0x00007f1becb59479 in adapter_start (adapter=0x7f1bed29edd0) at adapter.c:2569
8  0x00007f1becb5427f in manager_start_adapter (id=0) at manager.c:424
9  0x00007f1becb48e02 in device_devup_setup (dev_id=0) at main.c:481
10 0x00007f1becb48f35 in init_all_devices (ctl=7) at main.c:512
11 0x00007f1becb496d0 in main (argc=1, argv=0x7ffff4b70788) at main.c:748

The HID device's SDP record can be NULL, so don't crash when that happens
---
 input/manager.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/input/manager.c b/input/manager.c
index 51634e3..25b64e4 100644
--- a/input/manager.c
+++ b/input/manager.c
@@ -65,6 +65,9 @@ static int hid_device_probe(struct btd_device *device, GSList *uuids)
 
 	DBG("path %s", path);
 
+	if (!rec)
+		return -1;
+
 	adapter_get_address(adapter, &src);
 	device_get_address(device, &dst);
 	pdlist = sdp_data_get(rec, SDP_SERVER_RECORD_HANDLE);
-- 
1.5.5.2

Re: [PATCH] Fix possible crash on startup

[Johan Hedberg]

Hi Bastien,

Thanks for the patch. It has been pushed upstream. However, now that I  
looked at the code in question there are several things I have to  
wonder about:
1. Why is the code using sdp_data_get(rec, SDP_SERVER_RECORD_HANDLE)  
instead of rec->handle?
2. Is having the record really mandatory for allowing a device to  
connect? I.e. could we somehow make input_probe() work even if the  
record isn't available?

I think it was either Claudio or Luiz who has been touching this part  
of bluez code recently (at least git-blame shows Claudio being  
responsible for the SDP_SERVER_RECORD_HANDLE thing) so maybe they can  
shed some more light on these issues?

Johan

Re: [PATCH] Fix possible crash on startup

[Luiz Augusto von Dentz]

Hi,

I guess we can make all driver to do the discover on Connect if the
record is not available, similar to what is done in audio.

-- 
Luiz Augusto von Dentz
Engenheiro de Computação

Re: [PATCH] Fix possible crash on startup

[Bastien Nocera]

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

On Mon, 2008-10-13 at 10:43 -0300, Luiz Augusto von Dentz wrote:
> Hi,
> 
> I guess we can make all driver to do the discover on Connect if the
> record is not available, similar to what is done in audio.

I'm not certain whether the problem is no record, or a broken one.

This is the file causing problems, the device line that causes the crash
is the one starting with 00:1E:45:AD:F1:96. So there is data, but it
probably isn't parsed properly.

Cheers

[-- Attachment #2: profiles --]
[-- Type: text/plain, Size: 739 bytes --]

00:1E:45:AD:F1:96 00000002-0000-1000-8000-0002ee000002 00001101-0000-1000-8000-00805f9b34fb 00001103-0000-1000-8000-00805f9b34fb 00001104-0000-1000-8000-00805f9b34fb 00001105-0000-1000-8000-00805f9b34fb 00001106-0000-1000-8000-00805f9b34fb 0000110a-0000-1000-8000-00805f9b34fb 0000110c-0000-1000-8000-00805f9b34fb 0000110e-0000-1000-8000-00805f9b34fb 00001112-0000-1000-8000-00805f9b34fb 00001115-0000-1000-8000-00805f9b34fb 00001116-0000-1000-8000-00805f9b34fb 0000111f-0000-1000-8000-00805f9b34fb 00001124-0000-1000-8000-00805f9b34fb 0000112f-0000-1000-8000-00805f9b34fb 00001200-0000-1000-8000-00805f9b34fb 8e771301-0000-1000-8000-00805f9b34fb 8e771401-0000-1000-8000-00805f9b34fb
00:0A:94:C0:AB:9B 00001124-0000-1000-8000-00805f9b34fb

Re: [PATCH] Fix possible crash on startup

[Claudio Takahasi]

2008/10/13 Bastien Nocera <hadess@hadess.net>:
> On Mon, 2008-10-13 at 10:43 -0300, Luiz Augusto von Dentz wrote:
>> Hi,
>>
>> I guess we can make all driver to do the discover on Connect if the
>> record is not available, similar to what is done in audio.
>
> I'm not certain whether the problem is no record, or a broken one.
>
> This is the file causing problems, the device line that causes the crash
> is the one starting with 00:1E:45:AD:F1:96. So there is data, but it
> probably isn't parsed properly.
>
> Cheers
>

Hi,

The source of this problem is an invalid sequence in the service
record. See entry 00:1E:45:AD:F1:96#02008007 in sdp record file.
About the Johan's questions:
1. we can access rec->handle directly, we have to fix it
2. service record is necessary, at least HIDDescriptorList

Claudio
-- 
--
Claudio Takahasi
Instituto Nokia de Tecnologia
Recife - Pernambuco - Brasil
+55 81 30879999

Re: [PATCH] Fix possible crash on startup

[Claudio Takahasi]

On Tue, Oct 14, 2008 at 3:47 PM, Claudio Takahasi
<claudio.takahasi@openbossa.org> wrote:
> 2008/10/13 Bastien Nocera <hadess@hadess.net>:
>> On Mon, 2008-10-13 at 10:43 -0300, Luiz Augusto von Dentz wrote:
>>> Hi,
>>>
>>> I guess we can make all driver to do the discover on Connect if the
>>> record is not available, similar to what is done in audio.
>>
>> I'm not certain whether the problem is no record, or a broken one.
>>
>> This is the file causing problems, the device line that causes the crash
>> is the one starting with 00:1E:45:AD:F1:96. So there is data, but it
>> probably isn't parsed properly.
>>
>> Cheers
>>
>
> Hi,
>
> The source of this problem is an invalid sequence in the service
> record. See entry 00:1E:45:AD:F1:96#02008007 in sdp record file.
> About the Johan's questions:
> 1. we can access rec->handle directly, we have to fix it
> 2. service record is necessary, at least HIDDescriptorList
>
> Claudio
> --
> --
> Claudio Takahasi
> Instituto Nokia de Tecnologia
> Recife - Pernambuco - Brasil
> +55 81 30879999

Hi Bastien,

The patch that you sent is enough to fix this problem. There isn't
error when parsing the service record, the illegal multibyte sequence
error that I saw when debugging is a normal error when parsing
"storage" files, it is exit condition.

I am just wondering how you created the sdp file that you sent me, did
you remove some entries? My sdp file contains 18 entries, all of them
are related to the Sony Ericsson K850.

Regards,
Claudio.

Re: [PATCH] Fix possible crash on startup

[Bastien Nocera]

On Tue, 2008-10-14 at 18:44 -0300, Claudio Takahasi wrote:
<snip>
> Hi Bastien,
> 
> The patch that you sent is enough to fix this problem. There isn't
> error when parsing the service record, the illegal multibyte sequence
> error that I saw when debugging is a normal error when parsing
> "storage" files, it is exit condition.
> 
> I am just wondering how you created the sdp file that you sent me, did
> you remove some entries? My sdp file contains 18 entries, all of them
> are related to the Sony Ericsson K850.

None of those entries were created by hand. I believe this entry was
probably created with bluez 3.x, I only upgraded to bluez 4.x on my main
machine in the past week or so.

Cheers