* bluetoothd bug @ 2010-01-25 18:14 Claudio Takahasi 2010-01-26 11:09 ` Luiz Augusto von Dentz 0 siblings, 1 reply; 6+ messages in thread From: Claudio Takahasi @ 2010-01-25 18:14 UTC (permalink / raw) To: Johan Hedberg, luiz.dentz-von; +Cc: BlueZ development 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 How to reproduce: PTS v 3.3.1.4 Simple pairing enabled Test Case: TC_PSE_SSM_BV_08_I (phonebook) Regards, Claudio. -- -- Claudio Takahasi Instituto Nokia de Tecnologia Recife - Pernambuco - Brasil +55 81 30879999 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: bluetoothd bug 2010-01-25 18:14 bluetoothd bug Claudio Takahasi @ 2010-01-26 11:09 ` Luiz Augusto von Dentz 2010-01-26 18:33 ` Claudio Takahasi 0 siblings, 1 reply; 6+ messages in thread From: Luiz Augusto von Dentz @ 2010-01-26 11:09 UTC (permalink / raw) To: Claudio Takahasi; +Cc: Johan Hedberg, luiz.dentz-von, BlueZ development 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: bluetoothd bug 2010-01-26 11:09 ` Luiz Augusto von Dentz @ 2010-01-26 18:33 ` Claudio Takahasi 2010-01-27 8:21 ` Luiz Augusto von Dentz 0 siblings, 1 reply; 6+ messages in thread From: Claudio Takahasi @ 2010-01-26 18:33 UTC (permalink / raw) To: Luiz Augusto von Dentz; +Cc: Johan Hedberg, luiz.dentz-von, BlueZ development 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. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: bluetoothd bug 2010-01-26 18:33 ` Claudio Takahasi @ 2010-01-27 8:21 ` Luiz Augusto von Dentz 2010-01-27 8:28 ` Johan Hedberg 2010-01-27 8:43 ` Marcel Holtmann 0 siblings, 2 replies; 6+ messages in thread From: Luiz Augusto von Dentz @ 2010-01-27 8:21 UTC (permalink / raw) To: Claudio Takahasi; +Cc: Johan Hedberg, luiz.dentz-von, BlueZ development [-- 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: bluetoothd bug 2010-01-27 8:21 ` Luiz Augusto von Dentz @ 2010-01-27 8:28 ` Johan Hedberg 2010-01-27 8:43 ` Marcel Holtmann 1 sibling, 0 replies; 6+ messages in thread From: Johan Hedberg @ 2010-01-27 8:28 UTC (permalink / raw) To: Luiz Augusto von Dentz Cc: Claudio Takahasi, luiz.dentz-von, BlueZ development Hi Luiz, On Wed, Jan 27, 2010, Luiz Augusto von Dentz wrote: > 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(-) The patch has been pushed upstream. Thanks for the fix! Johan ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: bluetoothd bug 2010-01-27 8:21 ` Luiz Augusto von Dentz 2010-01-27 8:28 ` Johan Hedberg @ 2010-01-27 8:43 ` Marcel Holtmann 1 sibling, 0 replies; 6+ messages in thread From: Marcel Holtmann @ 2010-01-27 8:43 UTC (permalink / raw) To: Luiz Augusto von Dentz Cc: Claudio Takahasi, Johan Hedberg, luiz.dentz-von, BlueZ development Hi Luiz, > 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. as I explained to Claudio, the PTS guys didn't even read their own specification. With Simple Pairing you are required to establish the encryption for everything except PSM 1. And the kernel will reject everything that tries to establish a L2CAP channel before encryption is in place. Regards Marcel ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-01-27 8:43 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-01-25 18:14 bluetoothd bug Claudio Takahasi 2010-01-26 11:09 ` Luiz Augusto von Dentz 2010-01-26 18:33 ` Claudio Takahasi 2010-01-27 8:21 ` Luiz Augusto von Dentz 2010-01-27 8:28 ` Johan Hedberg 2010-01-27 8:43 ` Marcel Holtmann
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).