unit/test-gatt broken again (as always :-)

unit/test-gatt broken again (as always :-)

[Stefan Seyfried]

Hi all,

in bluez-5.30, the testsuite again does not complete with

MALLOC_CHECK_=3 MALLOC_PERTURB_=69 make check

Valgrind complains about a use-after-free.

I simply bisected the issue, this is the broken commit.

commit 9a0d6d99c79e5cecb76b7a61b53556447f0ccbd1
Author: Gowtham Anandha Babu <gowtham.ab@samsung.com>
Date:   Thu Mar 26 18:29:06 2015 +0530

    unit/test-gatt: Add TP/GAW/CL/BV-05-C test

    Verify that a Generic Attribute Profile client can write a long
    Characteristic Value selected by handle.

This is on plain x86_64, gcc-4.8.3 (old)

Best regards,

	Stefan
-- 
Stefan Seyfried
Linux Consultant & Developer -- GPG Key: 0x731B665B

B1 Systems GmbH
Osterfeldstraße 7 / 85088 Vohburg / http://www.b1-systems.de
GF: Ralph Dehner / Unternehmenssitz: Vohburg / AG: Ingolstadt,HRB 3537

Re: unit/test-gatt broken again (as always :-)

[Luiz Augusto von Dentz]

Hi Stefan,

On Wed, Apr 1, 2015 at 2:42 PM, Stefan Seyfried
<stefan.seyfried@googlemail.com> wrote:
> Hi all,
>
> in bluez-5.30, the testsuite again does not complete with
>
> MALLOC_CHECK_=3 MALLOC_PERTURB_=69 make check
>
> Valgrind complains about a use-after-free.
>
> I simply bisected the issue, this is the broken commit.
>
> commit 9a0d6d99c79e5cecb76b7a61b53556447f0ccbd1
> Author: Gowtham Anandha Babu <gowtham.ab@samsung.com>
> Date:   Thu Mar 26 18:29:06 2015 +0530
>
>     unit/test-gatt: Add TP/GAW/CL/BV-05-C test
>
>     Verify that a Generic Attribute Profile client can write a long
>     Characteristic Value selected by handle.
>
> This is on plain x86_64, gcc-4.8.3 (old)

This is actually a fault in our code that the test uncovered which is
good but we failed to see it happening, I actually have some plans to
have a check-valgrind target to catch this type of problems or maybe
there is a way to add MALLOC_CHECK_=3 MALLOC_PERTURB_=69 directly.


-- 
Luiz Augusto von Dentz

RE: unit/test-gatt broken again (as always :-)

[Gowtham Anandha Babu]

Hi,

> -----Original Message-----
> From: linux-bluetooth-owner@vger.kernel.org [mailto:linux-bluetooth-
> owner@vger.kernel.org] On Behalf Of Luiz Augusto von Dentz
> Sent: Wednesday, April 01, 2015 5:38 PM
> To: Stefan Seyfried
> Cc: BlueZ devel list; Gowtham Anandha Babu
> Subject: Re: unit/test-gatt broken again (as always :-)
> 
> Hi Stefan,
> 
> On Wed, Apr 1, 2015 at 2:42 PM, Stefan Seyfried
> <stefan.seyfried@googlemail.com> wrote:
> > Hi all,
> >
> > in bluez-5.30, the testsuite again does not complete with
> >
> > MALLOC_CHECK_=3 MALLOC_PERTURB_=69 make check
> >
> > Valgrind complains about a use-after-free.
> >
> > I simply bisected the issue, this is the broken commit.
> >
> > commit 9a0d6d99c79e5cecb76b7a61b53556447f0ccbd1
> > Author: Gowtham Anandha Babu <gowtham.ab@samsung.com>
> > Date:   Thu Mar 26 18:29:06 2015 +0530
> >
> >     unit/test-gatt: Add TP/GAW/CL/BV-05-C test
> >
> >     Verify that a Generic Attribute Profile client can write a long
> >     Characteristic Value selected by handle.
> >
> > This is on plain x86_64, gcc-4.8.3 (old)
> 
> This is actually a fault in our code that the test uncovered which is good but
> we failed to see it happening, I actually have some plans to have a check-
> valgrind target to catch this type of problems or maybe there is a way to add
> MALLOC_CHECK_=3 MALLOC_PERTURB_=69 directly.
> 
> 

Good catch. I got the above error, after updating my gcc. The patch sent by Luiz resolved this issue.
Yes there may be a way for adding valgrind directly as done in
 "android/system-emulator.c" & "android/bluetoothd-wrapper.c".

Regards,
Gowtham Anandha Babu

> --
> Luiz Augusto von Dentz
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org More majordomo
> info at  http://vger.kernel.org/majordomo-info.html

Re: unit/test-gatt broken again (as always :-)

[Stefan Seyfried]

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

Hi Luiz,

Am 01.04.2015 um 14:08 schrieb Luiz Augusto von Dentz:

> This is actually a fault in our code that the test uncovered which is
> good but we failed to see it happening, I actually have some plans to

Ok. Then it's good that the openSUSE Buildservice setup has these
parameters set as default for building :-)

> have a check-valgrind target to catch this type of problems or maybe
> there is a way to add MALLOC_CHECK_=3 MALLOC_PERTURB_=69 directly.

I can help with that, see attached patch (sorry for the attachment, but
making thunderbird behave well is out of scope for now :-)

Best regards,

	Stefan
-- 
Stefan Seyfried
Linux Consultant & Developer -- GPG Key: 0x731B665B

B1 Systems GmbH
Osterfeldstraße 7 / 85088 Vohburg / http://www.b1-systems.de
GF: Ralph Dehner / Unternehmenssitz: Vohburg / AG: Ingolstadt,HRB 3537

[-- Attachment #2: 0001-enable-glibc-s-malloc-checks-for-the-test-suite.patch --]
[-- Type: text/x-patch, Size: 947 bytes --]

>From f76391a35bd168a31d62ab0ea16dfdc3bdeb52e1 Mon Sep 17 00:00:00 2001
From: Stefan Seyfried <seife+dev@b1-systems.com>
Date: Wed, 1 Apr 2015 22:04:41 +0200
Subject: [PATCH] enable glibc's malloc checks for the test suite

Setting MALLOC_CHECK_=3 causes additional checks in glibc for malloc()
usage errors, abort()ing the test if those trigger. MALLOC_PERTURB_=69
causes free()'d memory to be poisoned with 0x45, leading to crashes in
case of use-after-free.
Together, both are useful for spotting more errors during "make check"

Signed-off-by: Stefan Seyfried <seife+dev@b1-systems.com>
---
 Makefile.am | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Makefile.am b/Makefile.am
index 2255ce8..b6beb42 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -413,6 +413,7 @@ noinst_PROGRAMS += $(unit_tests)
 endif
 
 TESTS = $(unit_tests)
+AM_TESTS_ENVIRONMENT = MALLOC_CHECK_=3 MALLOC_PERTURB_=69
 
 pkgconfigdir = $(libdir)/pkgconfig
 
-- 
2.3.3

Re: unit/test-gatt broken again (as always :-)

[Luiz Augusto von Dentz]

Hi Stefan,

On Wed, Apr 1, 2015 at 11:15 PM, Stefan Seyfried
<stefan.seyfried@googlemail.com> wrote:
> Hi Luiz,
>
> Am 01.04.2015 um 14:08 schrieb Luiz Augusto von Dentz:
>
>> This is actually a fault in our code that the test uncovered which is
>> good but we failed to see it happening, I actually have some plans to
>
> Ok. Then it's good that the openSUSE Buildservice setup has these
> parameters set as default for building :-)
>
>> have a check-valgrind target to catch this type of problems or maybe
>> there is a way to add MALLOC_CHECK_=3 MALLOC_PERTURB_=69 directly.
>
> I can help with that, see attached patch (sorry for the attachment, but
> making thunderbird behave well is out of scope for now :-)

Ive applied this along with the patches to fix the problem, note that
Ive edited you description since we don't use Signed-off-by in
userspace. Thanks a lot for the contribution.


-- 
Luiz Augusto von Dentz