2008-10-11 23:03:22

by Bastien Nocera

[permalink] [raw]
Subject: [PATCH] Fix possible crash on startup

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

Cheers


Attachments:
0001-Fix-possible-crash-on-startup.patch (2.11 kB)

2008-10-14 22:51:58

by Bastien Nocera

[permalink] [raw]
Subject: Re: [PATCH] Fix possible crash on startup

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


2008-10-14 21:44:20

by Claudio Takahasi

[permalink] [raw]
Subject: Re: [PATCH] Fix possible crash on startup

On Tue, Oct 14, 2008 at 3:47 PM, Claudio Takahasi
<[email protected]> wrote:
> 2008/10/13 Bastien Nocera <[email protected]>:
>> 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.

2008-10-14 18:47:29

by Claudio Takahasi

[permalink] [raw]
Subject: Re: [PATCH] Fix possible crash on startup

2008/10/13 Bastien Nocera <[email protected]>:
> 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

2008-10-13 14:16:51

by Bastien Nocera

[permalink] [raw]
Subject: Re: [PATCH] Fix possible crash on startup

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


Attachments:
profiles (739.00 B)

2008-10-13 13:43:09

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH] Fix possible crash on startup

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

2008-10-12 18:15:22

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCH] Fix possible crash on startup

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