Re: bluetoothd bug

[Luiz Augusto von Dentz <luiz.dentz@gmail.com>]

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

Hi Claudio,

On Tue, Jan 26, 2010 at 8:33 PM, Claudio Takahasi
<claudio.takahasi@openbossa.org> wrote:
> On Tue, Jan 26, 2010 at 9:09 AM, Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
>> Hi Claudio,
>>
>> On Mon, Jan 25, 2010 at 8:14 PM, Claudio Takahasi
>> <claudio.takahasi@openbossa.org> wrote:
>>> Hi Johan/Luiz,
>>>
>>> I found an error "invalid read". The pastebin log shows a PBAP
>>> connection in the RFCOMM channel 15. See line 195 of the first log,
>>> when the connection is refused the "temporary" device is being
>>> removed, however it seems that the "reverse" SDP query still active.
>>> PTS is establishing RFCOMM connection before "Encrypt Change" event,
>>> but this wrong procedure should not "harm" bluetoothd. In some cases I
>>> noticed segmentation fault, probably due this error.
>>>
>>> http://pastebin.com/m308f85ef
>>> http://pastebin.com/m2d80dd71
>>
>> Does this fix the problem:
>>
>> diff --git a/src/device.c b/src/device.c
>> index a2730ab..dfdacd8 100644
>> --- a/src/device.c
>> +++ b/src/device.c
>> @@ -239,6 +239,9 @@ static void device_free(gpointer user_data)
>>        if (device->disconn_timer)
>>                g_source_remove(device->disconn_timer);
>>
>> +       if (device->discov_timer)
>> +               g_source_remove(device->discov_timer);
>> +
>>        debug("device_free(%p)", device);
>>
>>        g_free(device->authr);
>>
>>
>> --
>> Luiz Augusto von Dentz
>> Engenheiro de Computação
>>
>
> Hi Luiz,
>
> yep, it is working perfectly: no warning/no segmentation fault.
>
> Claudio.
>

Great, not sure how PTS was able to trigger it since it seems to be
passing bonding phase without resetting temporary flag which would
mean bonding fails and would not even schedule the reversal service
discovery, but a segfault is always a bug no matter what stupid things
PTS is doing.

Proper git patch attached.

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

[-- Attachment #2: 0001-Fix-possible-invalid-read-when-removing-a-temporary-.patch --]
[-- Type: text/x-patch, Size: 934 bytes --]

From be0c1373499bbc480298429109124efbbe49f15a Mon Sep 17 00:00:00 2001
From: Luiz Augusto Von Dentz <luiz.dentz-von@nokia.com>
Date: Tue, 26 Jan 2010 16:17:21 +0200
Subject: [PATCH 1/6] Fix possible invalid read when removing a temporary device

An invalid read may happens when doing the reverse service discovery but
for some reason the device disconnects with the temporary flag set which
will make the device to be removed without cancelling the service.
---
 src/device.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/src/device.c b/src/device.c
index a2730ab..dfdacd8 100644
--- a/src/device.c
+++ b/src/device.c
@@ -239,6 +239,9 @@ static void device_free(gpointer user_data)
 	if (device->disconn_timer)
 		g_source_remove(device->disconn_timer);
 
+	if (device->discov_timer)
+		g_source_remove(device->discov_timer);
+
 	debug("device_free(%p)", device);
 
 	g_free(device->authr);
-- 
1.6.3.3