Linux bluetooth development
 help / color / mirror / Atom feed
* [RFC] android/gatt: Fix daemon crash
@ 2014-07-16  8:08 Andrei Emeltchenko
  2014-07-16  9:28 ` Szymon Janc
  0 siblings, 1 reply; 4+ messages in thread
From: Andrei Emeltchenko @ 2014-07-16  8:08 UTC (permalink / raw)
  To: linux-bluetooth

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Due to insufficient error check bluetooth daemon goes to crash loop with
logs shown below. The patch adds error check in a case crypto failed to
initialize with the same approach as in other helper functions.

...
I/ProbeModule(24651): insmod_by_dep: parse base black list error -1
E/ProbeModule(24651): insmod_by_dep: cannot find module's dependency
info: [net-pf-38]
I/bluetoothd(24639): bluetoothd[24640]: gatt: Failed to setup crypto
...
I/DEBUG   (16237): *** *** *** *** *** *** *** *** *** *** *** *** ***
*** *** ***
I/DEBUG   (16237): Build fingerprint:
'Intel/hsb/hsb:4.4.2/KOT49H/eng.android.20140514.123328:eng/test-keys'
I/DEBUG   (16237): Revision: '0'
I/DEBUG   (16237): pid: 24640, tid: 24640, name: bluetoothd-main  >>>
/system/bin/bluetoothd-main <<<
I/DEBUG   (16237): signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr
00000004
...
I/DEBUG   (16237): stack:
I/DEBUG   (16237):          ff81a9dc  ff81aa14  [stack]
I/DEBUG   (16237):          ff81a9e0  00000000
I/DEBUG   (16237):          ff81a9e4  f75c2e06  /system/lib/libc.so
(vsyslog+6)
I/DEBUG   (16237):          ff81a9e8  f77c0cd8
/system/bin/bluetoothd-main
I/DEBUG   (16237):          ff81a9ec  f778692c
/system/bin/bluetoothd-main (gatt_db_destroy+12)
I/DEBUG   (16237):     #00  ff81a9f0  00000003
I/DEBUG   (16237):          ff81a9f4  f77865c0
/system/bin/bluetoothd-main (gatt_db_service_destroy)
I/DEBUG   (16237):          ff81a9f8  f7784709
...
---
 src/shared/gatt-db.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/shared/gatt-db.c b/src/shared/gatt-db.c
index f671480..b3f95d2 100644
--- a/src/shared/gatt-db.c
+++ b/src/shared/gatt-db.c
@@ -138,6 +138,9 @@ static void gatt_db_service_destroy(void *data)
 
 void gatt_db_destroy(struct gatt_db *db)
 {
+	if (!db)
+		return;
+
 	queue_destroy(db->services, gatt_db_service_destroy);
 	free(db);
 }
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [RFC] android/gatt: Fix daemon crash
  2014-07-16  8:08 [RFC] android/gatt: Fix daemon crash Andrei Emeltchenko
@ 2014-07-16  9:28 ` Szymon Janc
  2014-07-16 10:31   ` Andrei Emeltchenko
  0 siblings, 1 reply; 4+ messages in thread
From: Szymon Janc @ 2014-07-16  9:28 UTC (permalink / raw)
  To: Andrei Emeltchenko; +Cc: linux-bluetooth

Hi Andrei,

On Wednesday 16 of July 2014 11:08:31 Andrei Emeltchenko wrote:
> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> 
> Due to insufficient error check bluetooth daemon goes to crash loop with
> logs shown below. The patch adds error check in a case crypto failed to
> initialize with the same approach as in other helper functions.

The fix is correct, but commit message is not. Maybe something like:

shared/gatt: Fix NULL pointer dereference on destroy

This patch adds pointer check to gatt_db_destroy allowing user to call
it with NULL pointer. This follow convention used in shared code for
destroy functions. This fix following android bluetoothd crash in a case
crypto failed to initialize.


> 
> ...
> I/ProbeModule(24651): insmod_by_dep: parse base black list error -1
> E/ProbeModule(24651): insmod_by_dep: cannot find module's dependency
> info: [net-pf-38]
> I/bluetoothd(24639): bluetoothd[24640]: gatt: Failed to setup crypto
> ...
> I/DEBUG   (16237): *** *** *** *** *** *** *** *** *** *** *** *** ***
> *** *** ***
> I/DEBUG   (16237): Build fingerprint:
> 'Intel/hsb/hsb:4.4.2/KOT49H/eng.android.20140514.123328:eng/test-keys'
> I/DEBUG   (16237): Revision: '0'
> I/DEBUG   (16237): pid: 24640, tid: 24640, name: bluetoothd-main  >>>
> /system/bin/bluetoothd-main <<<
> I/DEBUG   (16237): signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr
> 00000004
> ...
> I/DEBUG   (16237): stack:
> I/DEBUG   (16237):          ff81a9dc  ff81aa14  [stack]
> I/DEBUG   (16237):          ff81a9e0  00000000
> I/DEBUG   (16237):          ff81a9e4  f75c2e06  /system/lib/libc.so
> (vsyslog+6)
> I/DEBUG   (16237):          ff81a9e8  f77c0cd8
> /system/bin/bluetoothd-main
> I/DEBUG   (16237):          ff81a9ec  f778692c
> /system/bin/bluetoothd-main (gatt_db_destroy+12)
> I/DEBUG   (16237):     #00  ff81a9f0  00000003
> I/DEBUG   (16237):          ff81a9f4  f77865c0
> /system/bin/bluetoothd-main (gatt_db_service_destroy)
> I/DEBUG   (16237):          ff81a9f8  f7784709
> ...
> ---
>  src/shared/gatt-db.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/src/shared/gatt-db.c b/src/shared/gatt-db.c
> index f671480..b3f95d2 100644
> --- a/src/shared/gatt-db.c
> +++ b/src/shared/gatt-db.c
> @@ -138,6 +138,9 @@ static void gatt_db_service_destroy(void *data)
>  
>  void gatt_db_destroy(struct gatt_db *db)
>  {
> +	if (!db)
> +		return;
> +
>  	queue_destroy(db->services, gatt_db_service_destroy);
>  	free(db);
>  }
> 

-- 
Best regards, 
Szymon Janc

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [RFC] android/gatt: Fix daemon crash
  2014-07-16  9:28 ` Szymon Janc
@ 2014-07-16 10:31   ` Andrei Emeltchenko
  2014-07-16 10:44     ` Szymon Janc
  0 siblings, 1 reply; 4+ messages in thread
From: Andrei Emeltchenko @ 2014-07-16 10:31 UTC (permalink / raw)
  To: Szymon Janc; +Cc: linux-bluetooth

Hi Szymon,

On Wed, Jul 16, 2014 at 11:28:45AM +0200, Szymon Janc wrote:
> Hi Andrei,
> 
> On Wednesday 16 of July 2014 11:08:31 Andrei Emeltchenko wrote:
> > From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> > 
> > Due to insufficient error check bluetooth daemon goes to crash loop with
> > logs shown below. The patch adds error check in a case crypto failed to
> > initialize with the same approach as in other helper functions.
> 
> The fix is correct, but commit message is not. Maybe something like:
> 
> shared/gatt: Fix NULL pointer dereference on destroy
> 
> This patch adds pointer check to gatt_db_destroy allowing user to call
> it with NULL pointer. This follow convention used in shared code for
> destroy functions. This fix following android bluetoothd crash in a case
> crypto failed to initialize.

Looks OK, do you need me to resend this patch?

Best regards 
Andrei Emeltchenko 

> 
> 
> > 
> > ...
> > I/ProbeModule(24651): insmod_by_dep: parse base black list error -1
> > E/ProbeModule(24651): insmod_by_dep: cannot find module's dependency
> > info: [net-pf-38]
> > I/bluetoothd(24639): bluetoothd[24640]: gatt: Failed to setup crypto
> > ...
> > I/DEBUG   (16237): *** *** *** *** *** *** *** *** *** *** *** *** ***
> > *** *** ***
> > I/DEBUG   (16237): Build fingerprint:
> > 'Intel/hsb/hsb:4.4.2/KOT49H/eng.android.20140514.123328:eng/test-keys'
> > I/DEBUG   (16237): Revision: '0'
> > I/DEBUG   (16237): pid: 24640, tid: 24640, name: bluetoothd-main  >>>
> > /system/bin/bluetoothd-main <<<
> > I/DEBUG   (16237): signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr
> > 00000004
> > ...
> > I/DEBUG   (16237): stack:
> > I/DEBUG   (16237):          ff81a9dc  ff81aa14  [stack]
> > I/DEBUG   (16237):          ff81a9e0  00000000
> > I/DEBUG   (16237):          ff81a9e4  f75c2e06  /system/lib/libc.so
> > (vsyslog+6)
> > I/DEBUG   (16237):          ff81a9e8  f77c0cd8
> > /system/bin/bluetoothd-main
> > I/DEBUG   (16237):          ff81a9ec  f778692c
> > /system/bin/bluetoothd-main (gatt_db_destroy+12)
> > I/DEBUG   (16237):     #00  ff81a9f0  00000003
> > I/DEBUG   (16237):          ff81a9f4  f77865c0
> > /system/bin/bluetoothd-main (gatt_db_service_destroy)
> > I/DEBUG   (16237):          ff81a9f8  f7784709
> > ...
> > ---
> >  src/shared/gatt-db.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/src/shared/gatt-db.c b/src/shared/gatt-db.c
> > index f671480..b3f95d2 100644
> > --- a/src/shared/gatt-db.c
> > +++ b/src/shared/gatt-db.c
> > @@ -138,6 +138,9 @@ static void gatt_db_service_destroy(void *data)
> >  
> >  void gatt_db_destroy(struct gatt_db *db)
> >  {
> > +	if (!db)
> > +		return;
> > +
> >  	queue_destroy(db->services, gatt_db_service_destroy);
> >  	free(db);
> >  }
> > 
> 
> -- 
> Best regards, 
> Szymon Janc

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [RFC] android/gatt: Fix daemon crash
  2014-07-16 10:31   ` Andrei Emeltchenko
@ 2014-07-16 10:44     ` Szymon Janc
  0 siblings, 0 replies; 4+ messages in thread
From: Szymon Janc @ 2014-07-16 10:44 UTC (permalink / raw)
  To: Andrei Emeltchenko; +Cc: linux-bluetooth

Hi Andrei,

On Wednesday 16 of July 2014 13:31:52 Andrei Emeltchenko wrote:
> Hi Szymon,
> 
> On Wed, Jul 16, 2014 at 11:28:45AM +0200, Szymon Janc wrote:
> > Hi Andrei,
> > 
> > On Wednesday 16 of July 2014 11:08:31 Andrei Emeltchenko wrote:
> > > From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> > > 
> > > Due to insufficient error check bluetooth daemon goes to crash loop with
> > > logs shown below. The patch adds error check in a case crypto failed to
> > > initialize with the same approach as in other helper functions.
> > 
> > The fix is correct, but commit message is not. Maybe something like:
> > 
> > shared/gatt: Fix NULL pointer dereference on destroy
> > 
> > This patch adds pointer check to gatt_db_destroy allowing user to call
> > it with NULL pointer. This follow convention used in shared code for
> > destroy functions. This fix following android bluetoothd crash in a case
> > crypto failed to initialize.
> 
> Looks OK, do you need me to resend this patch?

Yes, please send it as [PATCH].

> 
> Best regards 
> Andrei Emeltchenko 
> 
> > 
> > 
> > > 
> > > ...
> > > I/ProbeModule(24651): insmod_by_dep: parse base black list error -1
> > > E/ProbeModule(24651): insmod_by_dep: cannot find module's dependency
> > > info: [net-pf-38]
> > > I/bluetoothd(24639): bluetoothd[24640]: gatt: Failed to setup crypto
> > > ...
> > > I/DEBUG   (16237): *** *** *** *** *** *** *** *** *** *** *** *** ***
> > > *** *** ***
> > > I/DEBUG   (16237): Build fingerprint:
> > > 'Intel/hsb/hsb:4.4.2/KOT49H/eng.android.20140514.123328:eng/test-keys'
> > > I/DEBUG   (16237): Revision: '0'
> > > I/DEBUG   (16237): pid: 24640, tid: 24640, name: bluetoothd-main  >>>
> > > /system/bin/bluetoothd-main <<<
> > > I/DEBUG   (16237): signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr
> > > 00000004
> > > ...
> > > I/DEBUG   (16237): stack:
> > > I/DEBUG   (16237):          ff81a9dc  ff81aa14  [stack]
> > > I/DEBUG   (16237):          ff81a9e0  00000000
> > > I/DEBUG   (16237):          ff81a9e4  f75c2e06  /system/lib/libc.so
> > > (vsyslog+6)
> > > I/DEBUG   (16237):          ff81a9e8  f77c0cd8
> > > /system/bin/bluetoothd-main
> > > I/DEBUG   (16237):          ff81a9ec  f778692c
> > > /system/bin/bluetoothd-main (gatt_db_destroy+12)
> > > I/DEBUG   (16237):     #00  ff81a9f0  00000003
> > > I/DEBUG   (16237):          ff81a9f4  f77865c0
> > > /system/bin/bluetoothd-main (gatt_db_service_destroy)
> > > I/DEBUG   (16237):          ff81a9f8  f7784709
> > > ...
> > > ---
> > >  src/shared/gatt-db.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/src/shared/gatt-db.c b/src/shared/gatt-db.c
> > > index f671480..b3f95d2 100644
> > > --- a/src/shared/gatt-db.c
> > > +++ b/src/shared/gatt-db.c
> > > @@ -138,6 +138,9 @@ static void gatt_db_service_destroy(void *data)
> > >  
> > >  void gatt_db_destroy(struct gatt_db *db)
> > >  {
> > > +	if (!db)
> > > +		return;
> > > +
> > >  	queue_destroy(db->services, gatt_db_service_destroy);
> > >  	free(db);
> > >  }
> > > 
> > 
> 

-- 
Best regards, 
Szymon Janc

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2014-07-16 10:44 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-07-16  8:08 [RFC] android/gatt: Fix daemon crash Andrei Emeltchenko
2014-07-16  9:28 ` Szymon Janc
2014-07-16 10:31   ` Andrei Emeltchenko
2014-07-16 10:44     ` Szymon Janc

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox