[PATCH] core/gatt-database: Fix memory corruption

[PATCH] core/gatt-database: Fix memory corruption

[Andrejs Hanins]

Pointer to on-stack variable was returned from pending_write_new
---
 src/gatt-database.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/src/gatt-database.c b/src/gatt-database.c
index 35f8471..c0135b6 100644
--- a/src/gatt-database.c
+++ b/src/gatt-database.c
@@ -1461,6 +1461,7 @@ static void pending_op_free(void *data)
 	if (op->owner_queue)
 		queue_remove(op->owner_queue, op);
 
+	free(op->setup_data);
 	free(op);
 }
 
@@ -1561,19 +1562,24 @@ static struct pending_op *pending_write_new(struct queue *owner_queue,
 					size_t len)
 {
 	struct pending_op *op;
-	struct iovec iov;
+	struct iovec* iov;
 
 	op = new0(struct pending_op, 1);
 	if (!op)
 		return NULL;
+	iov = new0(struct iovec, 1);
+	if (!iov) {
+		free(op);
+		return NULL;
+	}
 
-	iov.iov_base = (uint8_t *) value;
-	iov.iov_len = len;
+	iov->iov_base = (uint8_t *) value;
+	iov->iov_len = len;
 
 	op->owner_queue = owner_queue;
 	op->attrib = attrib;
 	op->id = id;
-	op->setup_data = &iov;
+	op->setup_data = iov;
 	queue_push_tail(owner_queue, op);
 
 	return op;
-- 
1.9.1

Re: [PATCH] core/gatt-database: Fix memory corruption

[Luiz Augusto von Dentz]

Hi Andrejs,

On Wed, Mar 11, 2015 at 3:31 PM, Andrejs Hanins <andrejs.hanins@ubnt.com> wrote:
> Pointer to on-stack variable was returned from pending_write_new
> ---
>  src/gatt-database.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/src/gatt-database.c b/src/gatt-database.c
> index 35f8471..c0135b6 100644
> --- a/src/gatt-database.c
> +++ b/src/gatt-database.c
> @@ -1461,6 +1461,7 @@ static void pending_op_free(void *data)
>         if (op->owner_queue)
>                 queue_remove(op->owner_queue, op);
>
> +       free(op->setup_data);
>         free(op);
>  }
>
> @@ -1561,19 +1562,24 @@ static struct pending_op *pending_write_new(struct queue *owner_queue,
>                                         size_t len)
>  {
>         struct pending_op *op;
> -       struct iovec iov;
> +       struct iovec* iov;
>
>         op = new0(struct pending_op, 1);
>         if (!op)
>                 return NULL;
> +       iov = new0(struct iovec, 1);
> +       if (!iov) {
> +               free(op);
> +               return NULL;
> +       }

I guess we can eliminate the allocation and have the iov declared as
part of pending_op as setup_data, btw even though this is pretty
obvious fix Id include a backtrace whenever possible.

> -       iov.iov_base = (uint8_t *) value;
> -       iov.iov_len = len;
> +       iov->iov_base = (uint8_t *) value;
> +       iov->iov_len = len;
>
>         op->owner_queue = owner_queue;
>         op->attrib = attrib;
>         op->id = id;
> -       op->setup_data = &iov;
> +       op->setup_data = iov;
>         queue_push_tail(owner_queue, op);
>
>         return op;
> --
> 1.9.1
>
> --
> 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



-- 
Luiz Augusto von Dentz

Re: [PATCH] core/gatt-database: Fix memory corruption

[Andrejs Hanins]

Pointer to on-stack variable was returned from pending_write_new.

#0  0x00007ffff72047b0 in __memmove_ssse3 () from /usr/lib/libc.so.6
#1  0x00007ffff78ae440 in ?? () from /usr/lib/libdbus-1.so.3
#2  0x00007ffff78ad7cc in ?? () from /usr/lib/libdbus-1.so.3
#3  0x00007ffff789ac46 in ?? () from /usr/lib/libdbus-1.so.3
#4  0x00000000004720e8 in write_setup_cb (iter=0x7fffffffe710,
user_data=0x76c2a0) at src/gatt-database.c:1516
#5  0x00000000004b7b4c in g_dbus_proxy_method_call (proxy=0x75ade0,
method=0x4e1747 "WriteValue", setup=0x472088 <write_setup_cb>,
    function=0x4720fd <write_reply_cb>, user_data=0x76c2a0,
destroy=0x471f28 <pending_op_free>) at gdbus/client.c:875
#6  0x000000000047235f in send_write (attrib=0x764e00, proxy=0x75ade0,
owner_queue=0x764250, id=1, value=0x767ff3 "\001\377\177", len=1)
    at src/gatt-database.c:1597
#7  0x0000000000472bdf in chrc_write_cb (attrib=0x764e00, id=1,
offset=0, value=0x767ff3 "\001\377\177", len=1, opcode=82 'R',
att=0x7606d0,
    user_data=0x7641d0) at src/gatt-database.c:1865
#8  0x00000000004c96f2 in gatt_db_attribute_write (attrib=0x764e00,
offset=0, value=0x767ff3 "\001\377\177", len=1, opcode=82 'R',
att=0x7606d0,
    func=0x4c53c8 <write_complete_cb>, user_data=0x772320) at
src/shared/gatt-db.c:1570
#9  0x00000000004c5609 in write_cb (opcode=82 'R', pdu=0x767ff1,
length=3, user_data=0x7630e0) at src/shared/gatt-server.c:796
#10 0x00000000004bdb35 in handle_notify (att=0x7606d0, opcode=82 'R',
pdu=0x767ff1 "\f", pdu_len=3) at src/shared/att.c:768
#11 0x00000000004bddc9 in can_read_data (io=0x7607a0,
user_data=0x7606d0) at src/shared/att.c:849
#12 0x00000000004c9c44 in watch_callback (channel=0x7607d0,
cond=G_IO_IN, user_data=0x760940) at src/shared/io-glib.c:170
#13 0x00007ffff7b1662d in g_main_context_dispatch () from
/usr/lib/libglib-2.0.so.0
#14 0x00007ffff7b16a08 in ?? () from /usr/lib/libglib-2.0.so.0
#15 0x00007ffff7b16d32 in g_main_loop_run () from
/usr/lib/libglib-2.0.so.0
#16 0x0000000000466ba1 in main (argc=1, argv=0x7fffffffec38) at
src/main.c:661
---
 src/gatt-database.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/src/gatt-database.c b/src/gatt-database.c
index 35f8471..ee24618 100644
--- a/src/gatt-database.c
+++ b/src/gatt-database.c
@@ -118,7 +118,7 @@ struct pending_op {
 	unsigned int id;
 	struct gatt_db_attribute *attrib;
 	struct queue *owner_queue;
-	void *setup_data;
+	struct iovec write_data;
 };
 
 struct device_state {
@@ -1509,12 +1509,11 @@ error:
 static void write_setup_cb(DBusMessageIter *iter, void *user_data)
 {
 	struct pending_op *op = user_data;
-	struct iovec *iov = op->setup_data;
 	DBusMessageIter array;
 
 	dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY, "y", &array);
 	dbus_message_iter_append_fixed_array(&array, DBUS_TYPE_BYTE,
-						&iov->iov_base, iov->iov_len);
+						&op->write_data.iov_base, op->write_data.iov_len);
 	dbus_message_iter_close_container(iter, &array);
 }
 
@@ -1561,19 +1560,17 @@ static struct pending_op *pending_write_new(struct queue *owner_queue,
 					size_t len)
 {
 	struct pending_op *op;
-	struct iovec iov;
 
 	op = new0(struct pending_op, 1);
 	if (!op)
 		return NULL;
 
-	iov.iov_base = (uint8_t *) value;
-	iov.iov_len = len;
+	op->write_data.iov_base = (uint8_t *) value;
+	op->write_data.iov_len = len;
 
 	op->owner_queue = owner_queue;
 	op->attrib = attrib;
 	op->id = id;
-	op->setup_data = &iov;
 	queue_push_tail(owner_queue, op);
 
 	return op;
-- 
1.9.1

Re: [PATCH] core/gatt-database: Fix memory corruption

[Stefan Seyfried]

Am 11.03.2015 um 15:35 schrieb Andrejs Hanins:
> Pointer to on-stack variable was returned from pending_write_new.

I still get a crash in the tests when running with memory debugging
enabled (which is default in openSUSE Build Service):

$> MALLOC_CHECK_=3 MALLOC_PERTURB_=69 unit/test-gatt

/TP/GAC/CL/BV-01-C - init
/TP/GAC/CL/BV-01-C - setup
[...]
/TP/GAR/CL/BV-01-C - setup complete
/TP/GAR/CL/BV-01-C - run
/TP/GAR/CL/BV-01-C - test passed
Segmentation fault

Program received signal SIGSEGV, Segmentation fault.
0x000055555558cb70 in bt_att_send (att=0x4000000f300432d, opcode=opcode@entry=24 '\030', 
    pdu=pdu@entry=0x7fffffff4a0f, length=length@entry=1, 
    callback=callback@entry=0x55555558fc30 <cancel_long_write_cb>, user_data=0x5555557b1ca0, 
    destroy=destroy@entry=0x0) at src/shared/att.c:1135
1135            if (!att || !att->io)
(gdb) bt
#0  0x000055555558cb70 in bt_att_send (att=0x4000000f300432d, opcode=opcode@entry=24 '\030', 
    pdu=pdu@entry=0x7fffffff4a0f, length=length@entry=1, 
    callback=callback@entry=0x55555558fc30 <cancel_long_write_cb>, user_data=0x5555557b1ca0, 
    destroy=destroy@entry=0x0) at src/shared/att.c:1135
#1  0x0000555555591039 in cancel_long_write_req (client=<optimized out>, req=<optimized out>)
    at src/shared/gatt-client.c:1791
#2  0x00005555555910ab in cancel_request (data=0x5555557b1e80) at src/shared/gatt-client.c:1855
#3  0x0000555555597903 in queue_remove_all (queue=<optimized out>, function=function@entry=0x0, 
    user_data=user_data@entry=0x0, destroy=destroy@entry=0x555555591060 <cancel_request>)
    at src/shared/queue.c:387
#4  0x00005555555917cd in bt_gatt_client_cancel_all (client=client@entry=0x5555557b36f0)
    at src/shared/gatt-client.c:1866
#5  0x0000555555591839 in bt_gatt_client_free (client=0x5555557b36f0) at src/shared/gatt-client.c:1569
#6  0x0000555555589439 in destroy_context (context=0x5555557b1bb0) at unit/test-gatt.c:284
#7  context_quit (user_data=0x5555557b1bb0) at unit/test-gatt.c:312
#8  0x000055555558d59b in handle_rsp (pdu_len=<optimized out>, pdu=0x5555557c6571 "\001\002\003\001)", 
    opcode=11 '\v', att=0x5555557b2e90) at src/shared/att.c:640
#9  can_read_data (io=<optimized out>, user_data=0x5555557b2e90) at src/shared/att.c:813
#10 0x0000555555596ec5 in watch_callback (channel=<optimized out>, cond=<optimized out>, 
    user_data=<optimized out>) at src/shared/io-glib.c:170
#11 0x00007ffff7b198e5 in g_main_context_dispatch () from /usr/lib64/libglib-2.0.so.0
#12 0x00007ffff7b19c48 in ?? () from /usr/lib64/libglib-2.0.so.0
#13 0x00007ffff7b19f0a in g_main_loop_run () from /usr/lib64/libglib-2.0.so.0
#14 0x000055555558c031 in tester_run () at src/shared/tester.c:830
#15 0x0000555555587e19 in main (argc=1, argv=0x7fffffffdc58) at unit/test-gatt.c:3182

Valgrind also complains loudly:
$> valgrind unit/test-gatt > /dev/null 
==20817== Memcheck, a memory error detector
==20817== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
==20817== Using Valgrind-3.10.0 and LibVEX; rerun with -h for copyright info
==20817== Command: unit/test-gatt
==20817== 
==20817== Syscall param socketcall.bind(my_addr.sa_data) points to uninitialised byte(s)
==20817==    at 0x522A737: bind (in /lib64/libc-2.21.so)
==20817==    by 0x14BBC2: ecb_aes_setup (crypto.c:110)
==20817==    by 0x14BBC2: bt_crypto_new (crypto.c:148)
==20817==    by 0x140788: bt_att_new (att.c:937)
==20817==    by 0x13EA4B: create_context.constprop.24 (test-gatt.c:592)
==20817==    by 0x13F2E2: run_callback (tester.c:412)
==20817==    by 0x4E808E4: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.4200.1)
==20817==    by 0x4E80C47: ??? (in /usr/lib64/libglib-2.0.so.0.4200.1)
==20817==    by 0x4E80F09: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.4200.1)
==20817==    by 0x140030: tester_run (tester.c:830)
==20817==    by 0x13BE18: main (test-gatt.c:3182)
==20817==  Address 0xffeff6aa8 is on thread 1's stack
==20817==  in frame #1, created by bt_crypto_new (crypto.c:141)
==20817== 
==20817== Syscall param socketcall.bind(my_addr.sa_data) points to uninitialised byte(s)
==20817==    at 0x522A737: bind (in /lib64/libc-2.21.so)
==20817==    by 0x14BC4B: cmac_aes_setup (crypto.c:132)
==20817==    by 0x14BC4B: bt_crypto_new (crypto.c:161)
==20817==    by 0x140788: bt_att_new (att.c:937)
==20817==    by 0x13EA4B: create_context.constprop.24 (test-gatt.c:592)
==20817==    by 0x13F2E2: run_callback (tester.c:412)
==20817==    by 0x4E808E4: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.4200.1)
==20817==    by 0x4E80C47: ??? (in /usr/lib64/libglib-2.0.so.0.4200.1)
==20817==    by 0x4E80F09: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.4200.1)
==20817==    by 0x140030: tester_run (tester.c:830)
==20817==    by 0x13BE18: main (test-gatt.c:3182)
==20817==  Address 0xffeff6aa8 is on thread 1's stack
==20817==  in frame #1, created by bt_crypto_new (crypto.c:141)
==20817== 
==20817== Invalid read of size 1
==20817==    at 0x145076: cancel_request (gatt-client.c:1854)
==20817==    by 0x14B902: queue_remove_all (queue.c:387)
==20817==    by 0x1457CC: bt_gatt_client_cancel_all (gatt-client.c:1866)
==20817==    by 0x145838: bt_gatt_client_free (gatt-client.c:1569)
==20817==    by 0x13D438: destroy_context (test-gatt.c:284)
==20817==    by 0x13D438: context_quit (test-gatt.c:312)
==20817==    by 0x14159A: handle_rsp (att.c:640)
==20817==    by 0x14159A: can_read_data (att.c:813)
==20817==    by 0x14AEC4: watch_callback (io-glib.c:170)
==20817==    by 0x4E808E4: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.4200.1)
==20817==    by 0x4E80C47: ??? (in /usr/lib64/libglib-2.0.so.0.4200.1)
==20817==    by 0x4E80F09: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.4200.1)
==20817==    by 0x140030: tester_run (tester.c:830)
==20817==    by 0x13BE18: main (test-gatt.c:3182)
==20817==  Address 0x5a13908 is 8 bytes inside a block of size 40 free'd
==20817==    at 0x4C2A37C: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==20817==    by 0x140F1E: cancel_att_send_op (att.c:222)
==20817==    by 0x140F1E: bt_att_cancel (att.c:1200)
==20817==    by 0x145075: cancel_request (gatt-client.c:1852)
==20817==    by 0x14B902: queue_remove_all (queue.c:387)
==20817==    by 0x1457CC: bt_gatt_client_cancel_all (gatt-client.c:1866)
==20817==    by 0x145838: bt_gatt_client_free (gatt-client.c:1569)
==20817==    by 0x13D438: destroy_context (test-gatt.c:284)
==20817==    by 0x13D438: context_quit (test-gatt.c:312)
==20817==    by 0x14159A: handle_rsp (att.c:640)
==20817==    by 0x14159A: can_read_data (att.c:813)
==20817==    by 0x14AEC4: watch_callback (io-glib.c:170)
==20817==    by 0x4E808E4: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.4200.1)
==20817==    by 0x4E80C47: ??? (in /usr/lib64/libglib-2.0.so.0.4200.1)
==20817==    by 0x4E80F09: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.4200.1)
==20817== 
==20817== Invalid read of size 1
==20817==    at 0x14507C: cancel_request (gatt-client.c:1857)
==20817==    by 0x14B902: queue_remove_all (queue.c:387)
==20817==    by 0x1457CC: bt_gatt_client_cancel_all (gatt-client.c:1866)
==20817==    by 0x145838: bt_gatt_client_free (gatt-client.c:1569)
==20817==    by 0x13D438: destroy_context (test-gatt.c:284)
==20817==    by 0x13D438: context_quit (test-gatt.c:312)
==20817==    by 0x14159A: handle_rsp (att.c:640)
==20817==    by 0x14159A: can_read_data (att.c:813)
==20817==    by 0x14AEC4: watch_callback (io-glib.c:170)
==20817==    by 0x4E808E4: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.4200.1)
==20817==    by 0x4E80C47: ??? (in /usr/lib64/libglib-2.0.so.0.4200.1)
==20817==    by 0x4E80F09: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.4200.1)
==20817==    by 0x140030: tester_run (tester.c:830)
==20817==    by 0x13BE18: main (test-gatt.c:3182)
==20817==  Address 0x5a13909 is 9 bytes inside a block of size 40 free'd
==20817==    at 0x4C2A37C: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==20817==    by 0x140F1E: cancel_att_send_op (att.c:222)
==20817==    by 0x140F1E: bt_att_cancel (att.c:1200)
==20817==    by 0x145075: cancel_request (gatt-client.c:1852)
==20817==    by 0x14B902: queue_remove_all (queue.c:387)
==20817==    by 0x1457CC: bt_gatt_client_cancel_all (gatt-client.c:1866)
==20817==    by 0x145838: bt_gatt_client_free (gatt-client.c:1569)
==20817==    by 0x13D438: destroy_context (test-gatt.c:284)
==20817==    by 0x13D438: context_quit (test-gatt.c:312)
==20817==    by 0x14159A: handle_rsp (att.c:640)
==20817==    by 0x14159A: can_read_data (att.c:813)
==20817==    by 0x14AEC4: watch_callback (io-glib.c:170)
==20817==    by 0x4E808E4: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.4200.1)
==20817==    by 0x4E80C47: ??? (in /usr/lib64/libglib-2.0.so.0.4200.1)
==20817==    by 0x4E80F09: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.4200.1)
==20817== 
==20817== 
==20817== HEAP SUMMARY:
==20817==     in use at exit: 29,640 bytes in 618 blocks
==20817==   total heap usage: 36,545 allocs, 35,927 frees, 1,585,464 bytes allocated
==20817== 
==20817== LEAK SUMMARY:
==20817==    definitely lost: 0 bytes in 0 blocks
==20817==    indirectly lost: 0 bytes in 0 blocks
==20817==      possibly lost: 0 bytes in 0 blocks
==20817==    still reachable: 29,640 bytes in 618 blocks
==20817==         suppressed: 0 bytes in 0 blocks
==20817== Rerun with --leak-check=full to see details of leaked memory
==20817== 
==20817== For counts of detected and suppressed errors, rerun with: -v
==20817== Use --track-origins=yes to see where uninitialised values come from
==20817== ERROR SUMMARY: 358 errors from 4 contexts (suppressed: 0 from 0)

Unfortunately, my understanding of the code did not allow me
to fis this :-(

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: [PATCH] core/gatt-database: Fix memory corruption

[Andrejs Hanins]

Hi Lukasz,

On 2015.03.11. 23:19, Stefan Seyfried wrote:
> Am 11.03.2015 um 15:35 schrieb Andrejs Hanins:
>> Pointer to on-stack variable was returned from pending_write_new.
> 
> I still get a crash in the tests when running with memory debugging
> enabled (which is default in openSUSE Build Service):
> 
> $> MALLOC_CHECK_=3 MALLOC_PERTURB_=69 unit/test-gatt
> 
> /TP/GAC/CL/BV-01-C - init
> /TP/GAC/CL/BV-01-C - setup
> [...]
> /TP/GAR/CL/BV-01-C - setup complete
> /TP/GAR/CL/BV-01-C - run
> /TP/GAR/CL/BV-01-C - test passed
> Segmentation fault
> 
> Program received signal SIGSEGV, Segmentation fault.
> 0x000055555558cb70 in bt_att_send (att=0x4000000f300432d, opcode=opcode@entry=24 '\030', 
>     pdu=pdu@entry=0x7fffffff4a0f, length=length@entry=1, 
>     callback=callback@entry=0x55555558fc30 <cancel_long_write_cb>, user_data=0x5555557b1ca0, 
>     destroy=destroy@entry=0x0) at src/shared/att.c:1135
> 1135            if (!att || !att->io)
> (gdb) bt
> #0  0x000055555558cb70 in bt_att_send (att=0x4000000f300432d, opcode=opcode@entry=24 '\030', 
>     pdu=pdu@entry=0x7fffffff4a0f, length=length@entry=1, 
>     callback=callback@entry=0x55555558fc30 <cancel_long_write_cb>, user_data=0x5555557b1ca0, 
>     destroy=destroy@entry=0x0) at src/shared/att.c:1135
> #1  0x0000555555591039 in cancel_long_write_req (client=<optimized out>, req=<optimized out>)
>     at src/shared/gatt-client.c:1791
Lukasz, I think there is some "missed-ref" problem related to the code you have recently added to the gatt-client/cancel_request() to cancel long_write and prep_write. Namely, bt_att_cancel can actually free the request which is later on accessed as req->long_write and req->prep_write thus reading free'd memory. Valgrind shows it happens this way:

  Address 0x5a06fa8 is 8 bytes inside a block of size 40 free'd
    at 0x4C2B200: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
    by 0x415BC0: request_unref (gatt-client.c:160)   <<------ this one frees the request which is accessed later in cancel_request()
    by 0x410BB3: cancel_att_send_op (att.c:222)
    by 0x412700: bt_att_cancel (att.c:1194)
    by 0x418EC0: cancel_request (gatt-client.c:1852)
    by 0x421C91: queue_remove_all (queue.c:387)
    by 0x418F53: bt_gatt_client_cancel_all (gatt-client.c:1866)
    by 0x418601: bt_gatt_client_free (gatt-client.c:1569)
    by 0x418A65: bt_gatt_client_unref (gatt-client.c:1692)
    by 0x4021FB: destroy_context (test-gatt.c:284)
    by 0x40230C: context_quit (test-gatt.c:312)
    by 0x4031AC: test_read_cb (test-gatt.c:677)

I can't instantly figure out the proper fix (add_ref to the request in the beginning of cancel_request() to avoid preliminary free?), hopefully it will be easier for you, as you are the author of the original code. Probably there are also other similar issues elsewhere.

> #2  0x00005555555910ab in cancel_request (data=0x5555557b1e80) at src/shared/gatt-client.c:1855
> #3  0x0000555555597903 in queue_remove_all (queue=<optimized out>, function=function@entry=0x0, 
>     user_data=user_data@entry=0x0, destroy=destroy@entry=0x555555591060 <cancel_request>)
>     at src/shared/queue.c:387
> #4  0x00005555555917cd in bt_gatt_client_cancel_all (client=client@entry=0x5555557b36f0)
>     at src/shared/gatt-client.c:1866
> #5  0x0000555555591839 in bt_gatt_client_free (client=0x5555557b36f0) at src/shared/gatt-client.c:1569
> #6  0x0000555555589439 in destroy_context (context=0x5555557b1bb0) at unit/test-gatt.c:284
> #7  context_quit (user_data=0x5555557b1bb0) at unit/test-gatt.c:312
> #8  0x000055555558d59b in handle_rsp (pdu_len=<optimized out>, pdu=0x5555557c6571 "\001\002\003\001)", 
>     opcode=11 '\v', att=0x5555557b2e90) at src/shared/att.c:640
> #9  can_read_data (io=<optimized out>, user_data=0x5555557b2e90) at src/shared/att.c:813
> #10 0x0000555555596ec5 in watch_callback (channel=<optimized out>, cond=<optimized out>, 
>     user_data=<optimized out>) at src/shared/io-glib.c:170
> #11 0x00007ffff7b198e5 in g_main_context_dispatch () from /usr/lib64/libglib-2.0.so.0
> #12 0x00007ffff7b19c48 in ?? () from /usr/lib64/libglib-2.0.so.0
> #13 0x00007ffff7b19f0a in g_main_loop_run () from /usr/lib64/libglib-2.0.so.0
> #14 0x000055555558c031 in tester_run () at src/shared/tester.c:830
> #15 0x0000555555587e19 in main (argc=1, argv=0x7fffffffdc58) at unit/test-gatt.c:3182
> 
> Valgrind also complains loudly:
> $> valgrind unit/test-gatt > /dev/null 
> ==20817== Memcheck, a memory error detector
> ==20817== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
> ==20817== Using Valgrind-3.10.0 and LibVEX; rerun with -h for copyright info
> ==20817== Command: unit/test-gatt
> ==20817== 
> ==20817== Syscall param socketcall.bind(my_addr.sa_data) points to uninitialised byte(s)
> ==20817==    at 0x522A737: bind (in /lib64/libc-2.21.so)
> ==20817==    by 0x14BBC2: ecb_aes_setup (crypto.c:110)
> ==20817==    by 0x14BBC2: bt_crypto_new (crypto.c:148)
> ==20817==    by 0x140788: bt_att_new (att.c:937)
> ==20817==    by 0x13EA4B: create_context.constprop.24 (test-gatt.c:592)
> ==20817==    by 0x13F2E2: run_callback (tester.c:412)
> ==20817==    by 0x4E808E4: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.4200.1)
> ==20817==    by 0x4E80C47: ??? (in /usr/lib64/libglib-2.0.so.0.4200.1)
> ==20817==    by 0x4E80F09: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.4200.1)
> ==20817==    by 0x140030: tester_run (tester.c:830)
> ==20817==    by 0x13BE18: main (test-gatt.c:3182)
> ==20817==  Address 0xffeff6aa8 is on thread 1's stack
> ==20817==  in frame #1, created by bt_crypto_new (crypto.c:141)
> ==20817== 
This one looks like a Valgrind bug. It probably does not take into account, that sockaddr passed to the bind is not a 'struct sockaddr' but actually a 'struct sockaddr_alg' of different size. The code, as such, does proper initialization.

> ==20817== Syscall param socketcall.bind(my_addr.sa_data) points to uninitialised byte(s)
> ==20817==    at 0x522A737: bind (in /lib64/libc-2.21.so)
> ==20817==    by 0x14BC4B: cmac_aes_setup (crypto.c:132)
> ==20817==    by 0x14BC4B: bt_crypto_new (crypto.c:161)
> ==20817==    by 0x140788: bt_att_new (att.c:937)
> ==20817==    by 0x13EA4B: create_context.constprop.24 (test-gatt.c:592)
> ==20817==    by 0x13F2E2: run_callback (tester.c:412)
> ==20817==    by 0x4E808E4: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.4200.1)
> ==20817==    by 0x4E80C47: ??? (in /usr/lib64/libglib-2.0.so.0.4200.1)
> ==20817==    by 0x4E80F09: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.4200.1)
> ==20817==    by 0x140030: tester_run (tester.c:830)
> ==20817==    by 0x13BE18: main (test-gatt.c:3182)
> ==20817==  Address 0xffeff6aa8 is on thread 1's stack
> ==20817==  in frame #1, created by bt_crypto_new (crypto.c:141)
> ==20817== 
> ==20817== Invalid read of size 1
> ==20817==    at 0x145076: cancel_request (gatt-client.c:1854)
> ==20817==    by 0x14B902: queue_remove_all (queue.c:387)
> ==20817==    by 0x1457CC: bt_gatt_client_cancel_all (gatt-client.c:1866)
> ==20817==    by 0x145838: bt_gatt_client_free (gatt-client.c:1569)
> ==20817==    by 0x13D438: destroy_context (test-gatt.c:284)
> ==20817==    by 0x13D438: context_quit (test-gatt.c:312)
> ==20817==    by 0x14159A: handle_rsp (att.c:640)
> ==20817==    by 0x14159A: can_read_data (att.c:813)
> ==20817==    by 0x14AEC4: watch_callback (io-glib.c:170)
> ==20817==    by 0x4E808E4: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.4200.1)
> ==20817==    by 0x4E80C47: ??? (in /usr/lib64/libglib-2.0.so.0.4200.1)
> ==20817==    by 0x4E80F09: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.4200.1)
> ==20817==    by 0x140030: tester_run (tester.c:830)
> ==20817==    by 0x13BE18: main (test-gatt.c:3182)
> ==20817==  Address 0x5a13908 is 8 bytes inside a block of size 40 free'd
> ==20817==    at 0x4C2A37C: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==20817==    by 0x140F1E: cancel_att_send_op (att.c:222)
> ==20817==    by 0x140F1E: bt_att_cancel (att.c:1200)
> ==20817==    by 0x145075: cancel_request (gatt-client.c:1852)
> ==20817==    by 0x14B902: queue_remove_all (queue.c:387)
> ==20817==    by 0x1457CC: bt_gatt_client_cancel_all (gatt-client.c:1866)
> ==20817==    by 0x145838: bt_gatt_client_free (gatt-client.c:1569)
> ==20817==    by 0x13D438: destroy_context (test-gatt.c:284)
> ==20817==    by 0x13D438: context_quit (test-gatt.c:312)
> ==20817==    by 0x14159A: handle_rsp (att.c:640)
> ==20817==    by 0x14159A: can_read_data (att.c:813)
> ==20817==    by 0x14AEC4: watch_callback (io-glib.c:170)
> ==20817==    by 0x4E808E4: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.4200.1)
> ==20817==    by 0x4E80C47: ??? (in /usr/lib64/libglib-2.0.so.0.4200.1)
> ==20817==    by 0x4E80F09: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.4200.1)
> ==20817== 
> ==20817== Invalid read of size 1
> ==20817==    at 0x14507C: cancel_request (gatt-client.c:1857)
> ==20817==    by 0x14B902: queue_remove_all (queue.c:387)
> ==20817==    by 0x1457CC: bt_gatt_client_cancel_all (gatt-client.c:1866)
> ==20817==    by 0x145838: bt_gatt_client_free (gatt-client.c:1569)
> ==20817==    by 0x13D438: destroy_context (test-gatt.c:284)
> ==20817==    by 0x13D438: context_quit (test-gatt.c:312)
> ==20817==    by 0x14159A: handle_rsp (att.c:640)
> ==20817==    by 0x14159A: can_read_data (att.c:813)
> ==20817==    by 0x14AEC4: watch_callback (io-glib.c:170)
> ==20817==    by 0x4E808E4: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.4200.1)
> ==20817==    by 0x4E80C47: ??? (in /usr/lib64/libglib-2.0.so.0.4200.1)
> ==20817==    by 0x4E80F09: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.4200.1)
> ==20817==    by 0x140030: tester_run (tester.c:830)
> ==20817==    by 0x13BE18: main (test-gatt.c:3182)
> ==20817==  Address 0x5a13909 is 9 bytes inside a block of size 40 free'd
> ==20817==    at 0x4C2A37C: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==20817==    by 0x140F1E: cancel_att_send_op (att.c:222)
> ==20817==    by 0x140F1E: bt_att_cancel (att.c:1200)
> ==20817==    by 0x145075: cancel_request (gatt-client.c:1852)
> ==20817==    by 0x14B902: queue_remove_all (queue.c:387)
> ==20817==    by 0x1457CC: bt_gatt_client_cancel_all (gatt-client.c:1866)
> ==20817==    by 0x145838: bt_gatt_client_free (gatt-client.c:1569)
> ==20817==    by 0x13D438: destroy_context (test-gatt.c:284)
> ==20817==    by 0x13D438: context_quit (test-gatt.c:312)
> ==20817==    by 0x14159A: handle_rsp (att.c:640)
> ==20817==    by 0x14159A: can_read_data (att.c:813)
> ==20817==    by 0x14AEC4: watch_callback (io-glib.c:170)
> ==20817==    by 0x4E808E4: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.4200.1)
> ==20817==    by 0x4E80C47: ??? (in /usr/lib64/libglib-2.0.so.0.4200.1)
> ==20817==    by 0x4E80F09: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.4200.1)
> ==20817== 
> ==20817== 
> ==20817== HEAP SUMMARY:
> ==20817==     in use at exit: 29,640 bytes in 618 blocks
> ==20817==   total heap usage: 36,545 allocs, 35,927 frees, 1,585,464 bytes allocated
> ==20817== 
> ==20817== LEAK SUMMARY:
> ==20817==    definitely lost: 0 bytes in 0 blocks
> ==20817==    indirectly lost: 0 bytes in 0 blocks
> ==20817==      possibly lost: 0 bytes in 0 blocks
> ==20817==    still reachable: 29,640 bytes in 618 blocks
> ==20817==         suppressed: 0 bytes in 0 blocks
> ==20817== Rerun with --leak-check=full to see details of leaked memory
> ==20817== 
> ==20817== For counts of detected and suppressed errors, rerun with: -v
> ==20817== Use --track-origins=yes to see where uninitialised values come from
> ==20817== ERROR SUMMARY: 358 errors from 4 contexts (suppressed: 0 from 0)
> 
> Unfortunately, my understanding of the code did not allow me
> to fis this :-(
> 
> Best regards,
> 
> 	Stefan
> 

Re: [PATCH] core/gatt-database: Fix memory corruption

[Luiz Augusto von Dentz]

Hi Andrejs,

On Thu, Mar 12, 2015 at 10:24 AM, Andrejs Hanins
<andrejs.hanins@ubnt.com> wrote:
> Hi Lukasz,
>
> On 2015.03.11. 23:19, Stefan Seyfried wrote:
>> Am 11.03.2015 um 15:35 schrieb Andrejs Hanins:
>>> Pointer to on-stack variable was returned from pending_write_new.
>>
>> I still get a crash in the tests when running with memory debugging
>> enabled (which is default in openSUSE Build Service):
>>
>> $> MALLOC_CHECK_=3 MALLOC_PERTURB_=69 unit/test-gatt
>>
>> /TP/GAC/CL/BV-01-C - init
>> /TP/GAC/CL/BV-01-C - setup
>> [...]
>> /TP/GAR/CL/BV-01-C - setup complete
>> /TP/GAR/CL/BV-01-C - run
>> /TP/GAR/CL/BV-01-C - test passed
>> Segmentation fault
>>
>> Program received signal SIGSEGV, Segmentation fault.
>> 0x000055555558cb70 in bt_att_send (att=0x4000000f300432d, opcode=opcode@entry=24 '\030',
>>     pdu=pdu@entry=0x7fffffff4a0f, length=length@entry=1,
>>     callback=callback@entry=0x55555558fc30 <cancel_long_write_cb>, user_data=0x5555557b1ca0,
>>     destroy=destroy@entry=0x0) at src/shared/att.c:1135
>> 1135            if (!att || !att->io)
>> (gdb) bt
>> #0  0x000055555558cb70 in bt_att_send (att=0x4000000f300432d, opcode=opcode@entry=24 '\030',
>>     pdu=pdu@entry=0x7fffffff4a0f, length=length@entry=1,
>>     callback=callback@entry=0x55555558fc30 <cancel_long_write_cb>, user_data=0x5555557b1ca0,
>>     destroy=destroy@entry=0x0) at src/shared/att.c:1135
>> #1  0x0000555555591039 in cancel_long_write_req (client=<optimized out>, req=<optimized out>)
>>     at src/shared/gatt-client.c:1791
> Lukasz, I think there is some "missed-ref" problem related to the code you have recently added to the gatt-client/cancel_request() to cancel long_write and prep_write. Namely, bt_att_cancel can actually free the request which is later on accessed as req->long_write and req->prep_write thus reading free'd memory. Valgrind shows it happens this way:
>
>   Address 0x5a06fa8 is 8 bytes inside a block of size 40 free'd
>     at 0x4C2B200: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
>     by 0x415BC0: request_unref (gatt-client.c:160)   <<------ this one frees the request which is accessed later in cancel_request()
>     by 0x410BB3: cancel_att_send_op (att.c:222)
>     by 0x412700: bt_att_cancel (att.c:1194)
>     by 0x418EC0: cancel_request (gatt-client.c:1852)
>     by 0x421C91: queue_remove_all (queue.c:387)
>     by 0x418F53: bt_gatt_client_cancel_all (gatt-client.c:1866)
>     by 0x418601: bt_gatt_client_free (gatt-client.c:1569)
>     by 0x418A65: bt_gatt_client_unref (gatt-client.c:1692)
>     by 0x4021FB: destroy_context (test-gatt.c:284)
>     by 0x40230C: context_quit (test-gatt.c:312)
>     by 0x4031AC: test_read_cb (test-gatt.c:677)
>
> I can't instantly figure out the proper fix (add_ref to the request in the beginning of cancel_request() to avoid preliminary free?), hopefully it will be easier for you, as you are the author of the original code. Probably there are also other similar issues elsewhere.

Yep, this seems a regression we introduced with prepare write set, but
I did not managed to reproduce it with our unit tests how are getting
this trace?

>
>> #2  0x00005555555910ab in cancel_request (data=0x5555557b1e80) at src/shared/gatt-client.c:1855
>> #3  0x0000555555597903 in queue_remove_all (queue=<optimized out>, function=function@entry=0x0,
>>     user_data=user_data@entry=0x0, destroy=destroy@entry=0x555555591060 <cancel_request>)
>>     at src/shared/queue.c:387
>> #4  0x00005555555917cd in bt_gatt_client_cancel_all (client=client@entry=0x5555557b36f0)
>>     at src/shared/gatt-client.c:1866
>> #5  0x0000555555591839 in bt_gatt_client_free (client=0x5555557b36f0) at src/shared/gatt-client.c:1569
>> #6  0x0000555555589439 in destroy_context (context=0x5555557b1bb0) at unit/test-gatt.c:284
>> #7  context_quit (user_data=0x5555557b1bb0) at unit/test-gatt.c:312
>> #8  0x000055555558d59b in handle_rsp (pdu_len=<optimized out>, pdu=0x5555557c6571 "\001\002\003\001)",
>>     opcode=11 '\v', att=0x5555557b2e90) at src/shared/att.c:640
>> #9  can_read_data (io=<optimized out>, user_data=0x5555557b2e90) at src/shared/att.c:813
>> #10 0x0000555555596ec5 in watch_callback (channel=<optimized out>, cond=<optimized out>,
>>     user_data=<optimized out>) at src/shared/io-glib.c:170
>> #11 0x00007ffff7b198e5 in g_main_context_dispatch () from /usr/lib64/libglib-2.0.so.0
>> #12 0x00007ffff7b19c48 in ?? () from /usr/lib64/libglib-2.0.so.0
>> #13 0x00007ffff7b19f0a in g_main_loop_run () from /usr/lib64/libglib-2.0.so.0
>> #14 0x000055555558c031 in tester_run () at src/shared/tester.c:830
>> #15 0x0000555555587e19 in main (argc=1, argv=0x7fffffffdc58) at unit/test-gatt.c:3182
>>
>> Valgrind also complains loudly:
>> $> valgrind unit/test-gatt > /dev/null
>> ==20817== Memcheck, a memory error detector
>> ==20817== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
>> ==20817== Using Valgrind-3.10.0 and LibVEX; rerun with -h for copyright info
>> ==20817== Command: unit/test-gatt
>> ==20817==
>> ==20817== Syscall param socketcall.bind(my_addr.sa_data) points to uninitialised byte(s)
>> ==20817==    at 0x522A737: bind (in /lib64/libc-2.21.so)
>> ==20817==    by 0x14BBC2: ecb_aes_setup (crypto.c:110)
>> ==20817==    by 0x14BBC2: bt_crypto_new (crypto.c:148)
>> ==20817==    by 0x140788: bt_att_new (att.c:937)
>> ==20817==    by 0x13EA4B: create_context.constprop.24 (test-gatt.c:592)
>> ==20817==    by 0x13F2E2: run_callback (tester.c:412)
>> ==20817==    by 0x4E808E4: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.4200.1)
>> ==20817==    by 0x4E80C47: ??? (in /usr/lib64/libglib-2.0.so.0.4200.1)
>> ==20817==    by 0x4E80F09: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.4200.1)
>> ==20817==    by 0x140030: tester_run (tester.c:830)
>> ==20817==    by 0x13BE18: main (test-gatt.c:3182)
>> ==20817==  Address 0xffeff6aa8 is on thread 1's stack
>> ==20817==  in frame #1, created by bt_crypto_new (crypto.c:141)
>> ==20817==
> This one looks like a Valgrind bug. It probably does not take into account, that sockaddr passed to the bind is not a 'struct sockaddr' but actually a 'struct sockaddr_alg' of different size. The code, as such, does proper initialization.

Valgrind probably is expecting some other size for the sockaddr, we do
memset to 0 so it is probably just a false positive.

>> ==20817== Syscall param socketcall.bind(my_addr.sa_data) points to uninitialised byte(s)
>> ==20817==    at 0x522A737: bind (in /lib64/libc-2.21.so)
>> ==20817==    by 0x14BC4B: cmac_aes_setup (crypto.c:132)
>> ==20817==    by 0x14BC4B: bt_crypto_new (crypto.c:161)
>> ==20817==    by 0x140788: bt_att_new (att.c:937)
>> ==20817==    by 0x13EA4B: create_context.constprop.24 (test-gatt.c:592)
>> ==20817==    by 0x13F2E2: run_callback (tester.c:412)
>> ==20817==    by 0x4E808E4: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.4200.1)
>> ==20817==    by 0x4E80C47: ??? (in /usr/lib64/libglib-2.0.so.0.4200.1)
>> ==20817==    by 0x4E80F09: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.4200.1)
>> ==20817==    by 0x140030: tester_run (tester.c:830)
>> ==20817==    by 0x13BE18: main (test-gatt.c:3182)
>> ==20817==  Address 0xffeff6aa8 is on thread 1's stack
>> ==20817==  in frame #1, created by bt_crypto_new (crypto.c:141)
>> ==20817==
>> ==20817== Invalid read of size 1
>> ==20817==    at 0x145076: cancel_request (gatt-client.c:1854)
>> ==20817==    by 0x14B902: queue_remove_all (queue.c:387)
>> ==20817==    by 0x1457CC: bt_gatt_client_cancel_all (gatt-client.c:1866)
>> ==20817==    by 0x145838: bt_gatt_client_free (gatt-client.c:1569)
>> ==20817==    by 0x13D438: destroy_context (test-gatt.c:284)
>> ==20817==    by 0x13D438: context_quit (test-gatt.c:312)
>> ==20817==    by 0x14159A: handle_rsp (att.c:640)
>> ==20817==    by 0x14159A: can_read_data (att.c:813)
>> ==20817==    by 0x14AEC4: watch_callback (io-glib.c:170)
>> ==20817==    by 0x4E808E4: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.4200.1)
>> ==20817==    by 0x4E80C47: ??? (in /usr/lib64/libglib-2.0.so.0.4200.1)
>> ==20817==    by 0x4E80F09: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.4200.1)
>> ==20817==    by 0x140030: tester_run (tester.c:830)
>> ==20817==    by 0x13BE18: main (test-gatt.c:3182)
>> ==20817==  Address 0x5a13908 is 8 bytes inside a block of size 40 free'd
>> ==20817==    at 0x4C2A37C: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
>> ==20817==    by 0x140F1E: cancel_att_send_op (att.c:222)
>> ==20817==    by 0x140F1E: bt_att_cancel (att.c:1200)
>> ==20817==    by 0x145075: cancel_request (gatt-client.c:1852)
>> ==20817==    by 0x14B902: queue_remove_all (queue.c:387)
>> ==20817==    by 0x1457CC: bt_gatt_client_cancel_all (gatt-client.c:1866)
>> ==20817==    by 0x145838: bt_gatt_client_free (gatt-client.c:1569)
>> ==20817==    by 0x13D438: destroy_context (test-gatt.c:284)
>> ==20817==    by 0x13D438: context_quit (test-gatt.c:312)
>> ==20817==    by 0x14159A: handle_rsp (att.c:640)
>> ==20817==    by 0x14159A: can_read_data (att.c:813)
>> ==20817==    by 0x14AEC4: watch_callback (io-glib.c:170)
>> ==20817==    by 0x4E808E4: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.4200.1)
>> ==20817==    by 0x4E80C47: ??? (in /usr/lib64/libglib-2.0.so.0.4200.1)
>> ==20817==    by 0x4E80F09: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.4200.1)
>> ==20817==
>> ==20817== Invalid read of size 1
>> ==20817==    at 0x14507C: cancel_request (gatt-client.c:1857)
>> ==20817==    by 0x14B902: queue_remove_all (queue.c:387)
>> ==20817==    by 0x1457CC: bt_gatt_client_cancel_all (gatt-client.c:1866)
>> ==20817==    by 0x145838: bt_gatt_client_free (gatt-client.c:1569)
>> ==20817==    by 0x13D438: destroy_context (test-gatt.c:284)
>> ==20817==    by 0x13D438: context_quit (test-gatt.c:312)
>> ==20817==    by 0x14159A: handle_rsp (att.c:640)
>> ==20817==    by 0x14159A: can_read_data (att.c:813)
>> ==20817==    by 0x14AEC4: watch_callback (io-glib.c:170)
>> ==20817==    by 0x4E808E4: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.4200.1)
>> ==20817==    by 0x4E80C47: ??? (in /usr/lib64/libglib-2.0.so.0.4200.1)
>> ==20817==    by 0x4E80F09: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.4200.1)
>> ==20817==    by 0x140030: tester_run (tester.c:830)
>> ==20817==    by 0x13BE18: main (test-gatt.c:3182)
>> ==20817==  Address 0x5a13909 is 9 bytes inside a block of size 40 free'd
>> ==20817==    at 0x4C2A37C: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
>> ==20817==    by 0x140F1E: cancel_att_send_op (att.c:222)
>> ==20817==    by 0x140F1E: bt_att_cancel (att.c:1200)
>> ==20817==    by 0x145075: cancel_request (gatt-client.c:1852)
>> ==20817==    by 0x14B902: queue_remove_all (queue.c:387)
>> ==20817==    by 0x1457CC: bt_gatt_client_cancel_all (gatt-client.c:1866)
>> ==20817==    by 0x145838: bt_gatt_client_free (gatt-client.c:1569)
>> ==20817==    by 0x13D438: destroy_context (test-gatt.c:284)
>> ==20817==    by 0x13D438: context_quit (test-gatt.c:312)
>> ==20817==    by 0x14159A: handle_rsp (att.c:640)
>> ==20817==    by 0x14159A: can_read_data (att.c:813)
>> ==20817==    by 0x14AEC4: watch_callback (io-glib.c:170)
>> ==20817==    by 0x4E808E4: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.4200.1)
>> ==20817==    by 0x4E80C47: ??? (in /usr/lib64/libglib-2.0.so.0.4200.1)
>> ==20817==    by 0x4E80F09: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.4200.1)
>> ==20817==
>> ==20817==
>> ==20817== HEAP SUMMARY:
>> ==20817==     in use at exit: 29,640 bytes in 618 blocks
>> ==20817==   total heap usage: 36,545 allocs, 35,927 frees, 1,585,464 bytes allocated
>> ==20817==
>> ==20817== LEAK SUMMARY:
>> ==20817==    definitely lost: 0 bytes in 0 blocks
>> ==20817==    indirectly lost: 0 bytes in 0 blocks
>> ==20817==      possibly lost: 0 bytes in 0 blocks
>> ==20817==    still reachable: 29,640 bytes in 618 blocks
>> ==20817==         suppressed: 0 bytes in 0 blocks
>> ==20817== Rerun with --leak-check=full to see details of leaked memory
>> ==20817==
>> ==20817== For counts of detected and suppressed errors, rerun with: -v
>> ==20817== Use --track-origins=yes to see where uninitialised values come from
>> ==20817== ERROR SUMMARY: 358 errors from 4 contexts (suppressed: 0 from 0)
>>
>> Unfortunately, my understanding of the code did not allow me
>> to fis this :-(
>>
>> Best regards,
>>
>>       Stefan
>>



-- 
Luiz Augusto von Dentz

Re: [PATCH] core/gatt-database: Fix memory corruption

[Andrejs Hanins]

Hi Luiz,

On 2015.03.12. 10:39, Luiz Augusto von Dentz wrote:
> Hi Andrejs,
> 
> On Thu, Mar 12, 2015 at 10:24 AM, Andrejs Hanins
> <andrejs.hanins@ubnt.com> wrote:
>> Hi Lukasz,
>>
>> On 2015.03.11. 23:19, Stefan Seyfried wrote:
>>> Am 11.03.2015 um 15:35 schrieb Andrejs Hanins:
>>>> Pointer to on-stack variable was returned from pending_write_new.
>>>
>>> I still get a crash in the tests when running with memory debugging
>>> enabled (which is default in openSUSE Build Service):
>>>
>>> $> MALLOC_CHECK_=3 MALLOC_PERTURB_=69 unit/test-gatt
>>>
>>> /TP/GAC/CL/BV-01-C - init
>>> /TP/GAC/CL/BV-01-C - setup
>>> [...]
>>> /TP/GAR/CL/BV-01-C - setup complete
>>> /TP/GAR/CL/BV-01-C - run
>>> /TP/GAR/CL/BV-01-C - test passed
>>> Segmentation fault
>>>
>>> Program received signal SIGSEGV, Segmentation fault.
>>> 0x000055555558cb70 in bt_att_send (att=0x4000000f300432d, opcode=opcode@entry=24 '\030',
>>>     pdu=pdu@entry=0x7fffffff4a0f, length=length@entry=1,
>>>     callback=callback@entry=0x55555558fc30 <cancel_long_write_cb>, user_data=0x5555557b1ca0,
>>>     destroy=destroy@entry=0x0) at src/shared/att.c:1135
>>> 1135            if (!att || !att->io)
>>> (gdb) bt
>>> #0  0x000055555558cb70 in bt_att_send (att=0x4000000f300432d, opcode=opcode@entry=24 '\030',
>>>     pdu=pdu@entry=0x7fffffff4a0f, length=length@entry=1,
>>>     callback=callback@entry=0x55555558fc30 <cancel_long_write_cb>, user_data=0x5555557b1ca0,
>>>     destroy=destroy@entry=0x0) at src/shared/att.c:1135
>>> #1  0x0000555555591039 in cancel_long_write_req (client=<optimized out>, req=<optimized out>)
>>>     at src/shared/gatt-client.c:1791
>> Lukasz, I think there is some "missed-ref" problem related to the code you have recently added to the gatt-client/cancel_request() to cancel long_write and prep_write. Namely, bt_att_cancel can actually free the request which is later on accessed as req->long_write and req->prep_write thus reading free'd memory. Valgrind shows it happens this way:
>>
>>   Address 0x5a06fa8 is 8 bytes inside a block of size 40 free'd
>>     at 0x4C2B200: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
>>     by 0x415BC0: request_unref (gatt-client.c:160)   <<------ this one frees the request which is accessed later in cancel_request()
>>     by 0x410BB3: cancel_att_send_op (att.c:222)
>>     by 0x412700: bt_att_cancel (att.c:1194)
>>     by 0x418EC0: cancel_request (gatt-client.c:1852)
>>     by 0x421C91: queue_remove_all (queue.c:387)
>>     by 0x418F53: bt_gatt_client_cancel_all (gatt-client.c:1866)
>>     by 0x418601: bt_gatt_client_free (gatt-client.c:1569)
>>     by 0x418A65: bt_gatt_client_unref (gatt-client.c:1692)
>>     by 0x4021FB: destroy_context (test-gatt.c:284)
>>     by 0x40230C: context_quit (test-gatt.c:312)
>>     by 0x4031AC: test_read_cb (test-gatt.c:677)
>>
>> I can't instantly figure out the proper fix (add_ref to the request in the beginning of cancel_request() to avoid preliminary free?), hopefully it will be easier for you, as you are the author of the original code. Probably there are also other similar issues elsewhere.
> 
> Yep, this seems a regression we introduced with prepare write set, but
> I did not managed to reproduce it with our unit tests how are getting
> this trace?

valgrind --log-file=valgrind.txt  ./unit/test-gatt

But if you do "MALLOC_CHECK_=3 MALLOC_PERTURB_=69 unit/test-gatt" as suggested by Stefan, then core dump also happens to me.

> 
>>
>>> #2  0x00005555555910ab in cancel_request (data=0x5555557b1e80) at src/shared/gatt-client.c:1855
>>> #3  0x0000555555597903 in queue_remove_all (queue=<optimized out>, function=function@entry=0x0,
>>>     user_data=user_data@entry=0x0, destroy=destroy@entry=0x555555591060 <cancel_request>)
>>>     at src/shared/queue.c:387
>>> #4  0x00005555555917cd in bt_gatt_client_cancel_all (client=client@entry=0x5555557b36f0)
>>>     at src/shared/gatt-client.c:1866
>>> #5  0x0000555555591839 in bt_gatt_client_free (client=0x5555557b36f0) at src/shared/gatt-client.c:1569
>>> #6  0x0000555555589439 in destroy_context (context=0x5555557b1bb0) at unit/test-gatt.c:284
>>> #7  context_quit (user_data=0x5555557b1bb0) at unit/test-gatt.c:312
>>> #8  0x000055555558d59b in handle_rsp (pdu_len=<optimized out>, pdu=0x5555557c6571 "\001\002\003\001)",
>>>     opcode=11 '\v', att=0x5555557b2e90) at src/shared/att.c:640
>>> #9  can_read_data (io=<optimized out>, user_data=0x5555557b2e90) at src/shared/att.c:813
>>> #10 0x0000555555596ec5 in watch_callback (channel=<optimized out>, cond=<optimized out>,
>>>     user_data=<optimized out>) at src/shared/io-glib.c:170
>>> #11 0x00007ffff7b198e5 in g_main_context_dispatch () from /usr/lib64/libglib-2.0.so.0
>>> #12 0x00007ffff7b19c48 in ?? () from /usr/lib64/libglib-2.0.so.0
>>> #13 0x00007ffff7b19f0a in g_main_loop_run () from /usr/lib64/libglib-2.0.so.0
>>> #14 0x000055555558c031 in tester_run () at src/shared/tester.c:830
>>> #15 0x0000555555587e19 in main (argc=1, argv=0x7fffffffdc58) at unit/test-gatt.c:3182
>>>
>>> Valgrind also complains loudly:
>>> $> valgrind unit/test-gatt > /dev/null
>>> ==20817== Memcheck, a memory error detector
>>> ==20817== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
>>> ==20817== Using Valgrind-3.10.0 and LibVEX; rerun with -h for copyright info
>>> ==20817== Command: unit/test-gatt
>>> ==20817==
>>> ==20817== Syscall param socketcall.bind(my_addr.sa_data) points to uninitialised byte(s)
>>> ==20817==    at 0x522A737: bind (in /lib64/libc-2.21.so)
>>> ==20817==    by 0x14BBC2: ecb_aes_setup (crypto.c:110)
>>> ==20817==    by 0x14BBC2: bt_crypto_new (crypto.c:148)
>>> ==20817==    by 0x140788: bt_att_new (att.c:937)
>>> ==20817==    by 0x13EA4B: create_context.constprop.24 (test-gatt.c:592)
>>> ==20817==    by 0x13F2E2: run_callback (tester.c:412)
>>> ==20817==    by 0x4E808E4: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.4200.1)
>>> ==20817==    by 0x4E80C47: ??? (in /usr/lib64/libglib-2.0.so.0.4200.1)
>>> ==20817==    by 0x4E80F09: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.4200.1)
>>> ==20817==    by 0x140030: tester_run (tester.c:830)
>>> ==20817==    by 0x13BE18: main (test-gatt.c:3182)
>>> ==20817==  Address 0xffeff6aa8 is on thread 1's stack
>>> ==20817==  in frame #1, created by bt_crypto_new (crypto.c:141)
>>> ==20817==
>> This one looks like a Valgrind bug. It probably does not take into account, that sockaddr passed to the bind is not a 'struct sockaddr' but actually a 'struct sockaddr_alg' of different size. The code, as such, does proper initialization.
> 
> Valgrind probably is expecting some other size for the sockaddr, we do
> memset to 0 so it is probably just a false positive.
> 
>>> ==20817== Syscall param socketcall.bind(my_addr.sa_data) points to uninitialised byte(s)
>>> ==20817==    at 0x522A737: bind (in /lib64/libc-2.21.so)
>>> ==20817==    by 0x14BC4B: cmac_aes_setup (crypto.c:132)
>>> ==20817==    by 0x14BC4B: bt_crypto_new (crypto.c:161)
>>> ==20817==    by 0x140788: bt_att_new (att.c:937)
>>> ==20817==    by 0x13EA4B: create_context.constprop.24 (test-gatt.c:592)
>>> ==20817==    by 0x13F2E2: run_callback (tester.c:412)
>>> ==20817==    by 0x4E808E4: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.4200.1)
>>> ==20817==    by 0x4E80C47: ??? (in /usr/lib64/libglib-2.0.so.0.4200.1)
>>> ==20817==    by 0x4E80F09: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.4200.1)
>>> ==20817==    by 0x140030: tester_run (tester.c:830)
>>> ==20817==    by 0x13BE18: main (test-gatt.c:3182)
>>> ==20817==  Address 0xffeff6aa8 is on thread 1's stack
>>> ==20817==  in frame #1, created by bt_crypto_new (crypto.c:141)
>>> ==20817==
>>> ==20817== Invalid read of size 1
>>> ==20817==    at 0x145076: cancel_request (gatt-client.c:1854)
>>> ==20817==    by 0x14B902: queue_remove_all (queue.c:387)
>>> ==20817==    by 0x1457CC: bt_gatt_client_cancel_all (gatt-client.c:1866)
>>> ==20817==    by 0x145838: bt_gatt_client_free (gatt-client.c:1569)
>>> ==20817==    by 0x13D438: destroy_context (test-gatt.c:284)
>>> ==20817==    by 0x13D438: context_quit (test-gatt.c:312)
>>> ==20817==    by 0x14159A: handle_rsp (att.c:640)
>>> ==20817==    by 0x14159A: can_read_data (att.c:813)
>>> ==20817==    by 0x14AEC4: watch_callback (io-glib.c:170)
>>> ==20817==    by 0x4E808E4: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.4200.1)
>>> ==20817==    by 0x4E80C47: ??? (in /usr/lib64/libglib-2.0.so.0.4200.1)
>>> ==20817==    by 0x4E80F09: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.4200.1)
>>> ==20817==    by 0x140030: tester_run (tester.c:830)
>>> ==20817==    by 0x13BE18: main (test-gatt.c:3182)
>>> ==20817==  Address 0x5a13908 is 8 bytes inside a block of size 40 free'd
>>> ==20817==    at 0x4C2A37C: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
>>> ==20817==    by 0x140F1E: cancel_att_send_op (att.c:222)
>>> ==20817==    by 0x140F1E: bt_att_cancel (att.c:1200)
>>> ==20817==    by 0x145075: cancel_request (gatt-client.c:1852)
>>> ==20817==    by 0x14B902: queue_remove_all (queue.c:387)
>>> ==20817==    by 0x1457CC: bt_gatt_client_cancel_all (gatt-client.c:1866)
>>> ==20817==    by 0x145838: bt_gatt_client_free (gatt-client.c:1569)
>>> ==20817==    by 0x13D438: destroy_context (test-gatt.c:284)
>>> ==20817==    by 0x13D438: context_quit (test-gatt.c:312)
>>> ==20817==    by 0x14159A: handle_rsp (att.c:640)
>>> ==20817==    by 0x14159A: can_read_data (att.c:813)
>>> ==20817==    by 0x14AEC4: watch_callback (io-glib.c:170)
>>> ==20817==    by 0x4E808E4: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.4200.1)
>>> ==20817==    by 0x4E80C47: ??? (in /usr/lib64/libglib-2.0.so.0.4200.1)
>>> ==20817==    by 0x4E80F09: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.4200.1)
>>> ==20817==
>>> ==20817== Invalid read of size 1
>>> ==20817==    at 0x14507C: cancel_request (gatt-client.c:1857)
>>> ==20817==    by 0x14B902: queue_remove_all (queue.c:387)
>>> ==20817==    by 0x1457CC: bt_gatt_client_cancel_all (gatt-client.c:1866)
>>> ==20817==    by 0x145838: bt_gatt_client_free (gatt-client.c:1569)
>>> ==20817==    by 0x13D438: destroy_context (test-gatt.c:284)
>>> ==20817==    by 0x13D438: context_quit (test-gatt.c:312)
>>> ==20817==    by 0x14159A: handle_rsp (att.c:640)
>>> ==20817==    by 0x14159A: can_read_data (att.c:813)
>>> ==20817==    by 0x14AEC4: watch_callback (io-glib.c:170)
>>> ==20817==    by 0x4E808E4: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.4200.1)
>>> ==20817==    by 0x4E80C47: ??? (in /usr/lib64/libglib-2.0.so.0.4200.1)
>>> ==20817==    by 0x4E80F09: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.4200.1)
>>> ==20817==    by 0x140030: tester_run (tester.c:830)
>>> ==20817==    by 0x13BE18: main (test-gatt.c:3182)
>>> ==20817==  Address 0x5a13909 is 9 bytes inside a block of size 40 free'd
>>> ==20817==    at 0x4C2A37C: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
>>> ==20817==    by 0x140F1E: cancel_att_send_op (att.c:222)
>>> ==20817==    by 0x140F1E: bt_att_cancel (att.c:1200)
>>> ==20817==    by 0x145075: cancel_request (gatt-client.c:1852)
>>> ==20817==    by 0x14B902: queue_remove_all (queue.c:387)
>>> ==20817==    by 0x1457CC: bt_gatt_client_cancel_all (gatt-client.c:1866)
>>> ==20817==    by 0x145838: bt_gatt_client_free (gatt-client.c:1569)
>>> ==20817==    by 0x13D438: destroy_context (test-gatt.c:284)
>>> ==20817==    by 0x13D438: context_quit (test-gatt.c:312)
>>> ==20817==    by 0x14159A: handle_rsp (att.c:640)
>>> ==20817==    by 0x14159A: can_read_data (att.c:813)
>>> ==20817==    by 0x14AEC4: watch_callback (io-glib.c:170)
>>> ==20817==    by 0x4E808E4: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.4200.1)
>>> ==20817==    by 0x4E80C47: ??? (in /usr/lib64/libglib-2.0.so.0.4200.1)
>>> ==20817==    by 0x4E80F09: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.4200.1)
>>> ==20817==
>>> ==20817==
>>> ==20817== HEAP SUMMARY:
>>> ==20817==     in use at exit: 29,640 bytes in 618 blocks
>>> ==20817==   total heap usage: 36,545 allocs, 35,927 frees, 1,585,464 bytes allocated
>>> ==20817==
>>> ==20817== LEAK SUMMARY:
>>> ==20817==    definitely lost: 0 bytes in 0 blocks
>>> ==20817==    indirectly lost: 0 bytes in 0 blocks
>>> ==20817==      possibly lost: 0 bytes in 0 blocks
>>> ==20817==    still reachable: 29,640 bytes in 618 blocks
>>> ==20817==         suppressed: 0 bytes in 0 blocks
>>> ==20817== Rerun with --leak-check=full to see details of leaked memory
>>> ==20817==
>>> ==20817== For counts of detected and suppressed errors, rerun with: -v
>>> ==20817== Use --track-origins=yes to see where uninitialised values come from
>>> ==20817== ERROR SUMMARY: 358 errors from 4 contexts (suppressed: 0 from 0)
>>>
>>> Unfortunately, my understanding of the code did not allow me
>>> to fis this :-(
>>>
>>> Best regards,
>>>
>>>       Stefan
>>>
> 
> 
> 

Re: [PATCH] core/gatt-database: Fix memory corruption

[Luiz Augusto von Dentz]

Hi Andrejs,

On Thu, Mar 12, 2015 at 10:54 AM, Andrejs Hanins
<andrejs.hanins@ubnt.com> wrote:
> Hi Luiz,
>
> On 2015.03.12. 10:39, Luiz Augusto von Dentz wrote:
>> Hi Andrejs,
>>
>> On Thu, Mar 12, 2015 at 10:24 AM, Andrejs Hanins
>> <andrejs.hanins@ubnt.com> wrote:
>>> Hi Lukasz,
>>>
>>> On 2015.03.11. 23:19, Stefan Seyfried wrote:
>>>> Am 11.03.2015 um 15:35 schrieb Andrejs Hanins:
>>>>> Pointer to on-stack variable was returned from pending_write_new.
>>>>
>>>> I still get a crash in the tests when running with memory debugging
>>>> enabled (which is default in openSUSE Build Service):
>>>>
>>>> $> MALLOC_CHECK_=3 MALLOC_PERTURB_=69 unit/test-gatt
>>>>
>>>> /TP/GAC/CL/BV-01-C - init
>>>> /TP/GAC/CL/BV-01-C - setup
>>>> [...]
>>>> /TP/GAR/CL/BV-01-C - setup complete
>>>> /TP/GAR/CL/BV-01-C - run
>>>> /TP/GAR/CL/BV-01-C - test passed
>>>> Segmentation fault
>>>>
>>>> Program received signal SIGSEGV, Segmentation fault.
>>>> 0x000055555558cb70 in bt_att_send (att=0x4000000f300432d, opcode=opcode@entry=24 '\030',
>>>>     pdu=pdu@entry=0x7fffffff4a0f, length=length@entry=1,
>>>>     callback=callback@entry=0x55555558fc30 <cancel_long_write_cb>, user_data=0x5555557b1ca0,
>>>>     destroy=destroy@entry=0x0) at src/shared/att.c:1135
>>>> 1135            if (!att || !att->io)
>>>> (gdb) bt
>>>> #0  0x000055555558cb70 in bt_att_send (att=0x4000000f300432d, opcode=opcode@entry=24 '\030',
>>>>     pdu=pdu@entry=0x7fffffff4a0f, length=length@entry=1,
>>>>     callback=callback@entry=0x55555558fc30 <cancel_long_write_cb>, user_data=0x5555557b1ca0,
>>>>     destroy=destroy@entry=0x0) at src/shared/att.c:1135
>>>> #1  0x0000555555591039 in cancel_long_write_req (client=<optimized out>, req=<optimized out>)
>>>>     at src/shared/gatt-client.c:1791
>>> Lukasz, I think there is some "missed-ref" problem related to the code you have recently added to the gatt-client/cancel_request() to cancel long_write and prep_write. Namely, bt_att_cancel can actually free the request which is later on accessed as req->long_write and req->prep_write thus reading free'd memory. Valgrind shows it happens this way:
>>>
>>>   Address 0x5a06fa8 is 8 bytes inside a block of size 40 free'd
>>>     at 0x4C2B200: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
>>>     by 0x415BC0: request_unref (gatt-client.c:160)   <<------ this one frees the request which is accessed later in cancel_request()
>>>     by 0x410BB3: cancel_att_send_op (att.c:222)
>>>     by 0x412700: bt_att_cancel (att.c:1194)
>>>     by 0x418EC0: cancel_request (gatt-client.c:1852)
>>>     by 0x421C91: queue_remove_all (queue.c:387)
>>>     by 0x418F53: bt_gatt_client_cancel_all (gatt-client.c:1866)
>>>     by 0x418601: bt_gatt_client_free (gatt-client.c:1569)
>>>     by 0x418A65: bt_gatt_client_unref (gatt-client.c:1692)
>>>     by 0x4021FB: destroy_context (test-gatt.c:284)
>>>     by 0x40230C: context_quit (test-gatt.c:312)
>>>     by 0x4031AC: test_read_cb (test-gatt.c:677)
>>>
>>> I can't instantly figure out the proper fix (add_ref to the request in the beginning of cancel_request() to avoid preliminary free?), hopefully it will be easier for you, as you are the author of the original code. Probably there are also other similar issues elsewhere.
>>
>> Yep, this seems a regression we introduced with prepare write set, but
>> I did not managed to reproduce it with our unit tests how are getting
>> this trace?
>
> valgrind --log-file=valgrind.txt  ./unit/test-gatt
>
> But if you do "MALLOC_CHECK_=3 MALLOC_PERTURB_=69 unit/test-gatt" as suggested by Stefan, then core dump also happens to me.

Ive just sent a patch for it, it is not crashing even with the following:

MALLOC_CHECK_=3 MALLOC_PERTURB_=69 valgrind --trace-children=yes
--track-origins=yes --show-possibly-lost=no --leak-check=full
unit/test-gatt

==30730==
==30730== HEAP SUMMARY:
==30730==     in use at exit: 29,640 bytes in 618 blocks
==30730==   total heap usage: 36,559 allocs, 35,941 frees, 1,592,908
bytes allocated
==30730==
==30730== LEAK SUMMARY:
==30730==    definitely lost: 0 bytes in 0 blocks
==30730==    indirectly lost: 0 bytes in 0 blocks
==30730==      possibly lost: 0 bytes in 0 blocks
==30730==    still reachable: 29,640 bytes in 618 blocks
==30730==         suppressed: 0 bytes in 0 blocks

>
>>
>>>
>>>> #2  0x00005555555910ab in cancel_request (data=0x5555557b1e80) at src/shared/gatt-client.c:1855
>>>> #3  0x0000555555597903 in queue_remove_all (queue=<optimized out>, function=function@entry=0x0,
>>>>     user_data=user_data@entry=0x0, destroy=destroy@entry=0x555555591060 <cancel_request>)
>>>>     at src/shared/queue.c:387
>>>> #4  0x00005555555917cd in bt_gatt_client_cancel_all (client=client@entry=0x5555557b36f0)
>>>>     at src/shared/gatt-client.c:1866
>>>> #5  0x0000555555591839 in bt_gatt_client_free (client=0x5555557b36f0) at src/shared/gatt-client.c:1569
>>>> #6  0x0000555555589439 in destroy_context (context=0x5555557b1bb0) at unit/test-gatt.c:284
>>>> #7  context_quit (user_data=0x5555557b1bb0) at unit/test-gatt.c:312
>>>> #8  0x000055555558d59b in handle_rsp (pdu_len=<optimized out>, pdu=0x5555557c6571 "\001\002\003\001)",
>>>>     opcode=11 '\v', att=0x5555557b2e90) at src/shared/att.c:640
>>>> #9  can_read_data (io=<optimized out>, user_data=0x5555557b2e90) at src/shared/att.c:813
>>>> #10 0x0000555555596ec5 in watch_callback (channel=<optimized out>, cond=<optimized out>,
>>>>     user_data=<optimized out>) at src/shared/io-glib.c:170
>>>> #11 0x00007ffff7b198e5 in g_main_context_dispatch () from /usr/lib64/libglib-2.0.so.0
>>>> #12 0x00007ffff7b19c48 in ?? () from /usr/lib64/libglib-2.0.so.0
>>>> #13 0x00007ffff7b19f0a in g_main_loop_run () from /usr/lib64/libglib-2.0.so.0
>>>> #14 0x000055555558c031 in tester_run () at src/shared/tester.c:830
>>>> #15 0x0000555555587e19 in main (argc=1, argv=0x7fffffffdc58) at unit/test-gatt.c:3182
>>>>
>>>> Valgrind also complains loudly:
>>>> $> valgrind unit/test-gatt > /dev/null
>>>> ==20817== Memcheck, a memory error detector
>>>> ==20817== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
>>>> ==20817== Using Valgrind-3.10.0 and LibVEX; rerun with -h for copyright info
>>>> ==20817== Command: unit/test-gatt
>>>> ==20817==
>>>> ==20817== Syscall param socketcall.bind(my_addr.sa_data) points to uninitialised byte(s)
>>>> ==20817==    at 0x522A737: bind (in /lib64/libc-2.21.so)
>>>> ==20817==    by 0x14BBC2: ecb_aes_setup (crypto.c:110)
>>>> ==20817==    by 0x14BBC2: bt_crypto_new (crypto.c:148)
>>>> ==20817==    by 0x140788: bt_att_new (att.c:937)
>>>> ==20817==    by 0x13EA4B: create_context.constprop.24 (test-gatt.c:592)
>>>> ==20817==    by 0x13F2E2: run_callback (tester.c:412)
>>>> ==20817==    by 0x4E808E4: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.4200.1)
>>>> ==20817==    by 0x4E80C47: ??? (in /usr/lib64/libglib-2.0.so.0.4200.1)
>>>> ==20817==    by 0x4E80F09: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.4200.1)
>>>> ==20817==    by 0x140030: tester_run (tester.c:830)
>>>> ==20817==    by 0x13BE18: main (test-gatt.c:3182)
>>>> ==20817==  Address 0xffeff6aa8 is on thread 1's stack
>>>> ==20817==  in frame #1, created by bt_crypto_new (crypto.c:141)
>>>> ==20817==
>>> This one looks like a Valgrind bug. It probably does not take into account, that sockaddr passed to the bind is not a 'struct sockaddr' but actually a 'struct sockaddr_alg' of different size. The code, as such, does proper initialization.
>>
>> Valgrind probably is expecting some other size for the sockaddr, we do
>> memset to 0 so it is probably just a false positive.
>>
>>>> ==20817== Syscall param socketcall.bind(my_addr.sa_data) points to uninitialised byte(s)
>>>> ==20817==    at 0x522A737: bind (in /lib64/libc-2.21.so)
>>>> ==20817==    by 0x14BC4B: cmac_aes_setup (crypto.c:132)
>>>> ==20817==    by 0x14BC4B: bt_crypto_new (crypto.c:161)
>>>> ==20817==    by 0x140788: bt_att_new (att.c:937)
>>>> ==20817==    by 0x13EA4B: create_context.constprop.24 (test-gatt.c:592)
>>>> ==20817==    by 0x13F2E2: run_callback (tester.c:412)
>>>> ==20817==    by 0x4E808E4: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.4200.1)
>>>> ==20817==    by 0x4E80C47: ??? (in /usr/lib64/libglib-2.0.so.0.4200.1)
>>>> ==20817==    by 0x4E80F09: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.4200.1)
>>>> ==20817==    by 0x140030: tester_run (tester.c:830)
>>>> ==20817==    by 0x13BE18: main (test-gatt.c:3182)
>>>> ==20817==  Address 0xffeff6aa8 is on thread 1's stack
>>>> ==20817==  in frame #1, created by bt_crypto_new (crypto.c:141)
>>>> ==20817==
>>>> ==20817== Invalid read of size 1
>>>> ==20817==    at 0x145076: cancel_request (gatt-client.c:1854)
>>>> ==20817==    by 0x14B902: queue_remove_all (queue.c:387)
>>>> ==20817==    by 0x1457CC: bt_gatt_client_cancel_all (gatt-client.c:1866)
>>>> ==20817==    by 0x145838: bt_gatt_client_free (gatt-client.c:1569)
>>>> ==20817==    by 0x13D438: destroy_context (test-gatt.c:284)
>>>> ==20817==    by 0x13D438: context_quit (test-gatt.c:312)
>>>> ==20817==    by 0x14159A: handle_rsp (att.c:640)
>>>> ==20817==    by 0x14159A: can_read_data (att.c:813)
>>>> ==20817==    by 0x14AEC4: watch_callback (io-glib.c:170)
>>>> ==20817==    by 0x4E808E4: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.4200.1)
>>>> ==20817==    by 0x4E80C47: ??? (in /usr/lib64/libglib-2.0.so.0.4200.1)
>>>> ==20817==    by 0x4E80F09: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.4200.1)
>>>> ==20817==    by 0x140030: tester_run (tester.c:830)
>>>> ==20817==    by 0x13BE18: main (test-gatt.c:3182)
>>>> ==20817==  Address 0x5a13908 is 8 bytes inside a block of size 40 free'd
>>>> ==20817==    at 0x4C2A37C: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
>>>> ==20817==    by 0x140F1E: cancel_att_send_op (att.c:222)
>>>> ==20817==    by 0x140F1E: bt_att_cancel (att.c:1200)
>>>> ==20817==    by 0x145075: cancel_request (gatt-client.c:1852)
>>>> ==20817==    by 0x14B902: queue_remove_all (queue.c:387)
>>>> ==20817==    by 0x1457CC: bt_gatt_client_cancel_all (gatt-client.c:1866)
>>>> ==20817==    by 0x145838: bt_gatt_client_free (gatt-client.c:1569)
>>>> ==20817==    by 0x13D438: destroy_context (test-gatt.c:284)
>>>> ==20817==    by 0x13D438: context_quit (test-gatt.c:312)
>>>> ==20817==    by 0x14159A: handle_rsp (att.c:640)
>>>> ==20817==    by 0x14159A: can_read_data (att.c:813)
>>>> ==20817==    by 0x14AEC4: watch_callback (io-glib.c:170)
>>>> ==20817==    by 0x4E808E4: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.4200.1)
>>>> ==20817==    by 0x4E80C47: ??? (in /usr/lib64/libglib-2.0.so.0.4200.1)
>>>> ==20817==    by 0x4E80F09: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.4200.1)
>>>> ==20817==
>>>> ==20817== Invalid read of size 1
>>>> ==20817==    at 0x14507C: cancel_request (gatt-client.c:1857)
>>>> ==20817==    by 0x14B902: queue_remove_all (queue.c:387)
>>>> ==20817==    by 0x1457CC: bt_gatt_client_cancel_all (gatt-client.c:1866)
>>>> ==20817==    by 0x145838: bt_gatt_client_free (gatt-client.c:1569)
>>>> ==20817==    by 0x13D438: destroy_context (test-gatt.c:284)
>>>> ==20817==    by 0x13D438: context_quit (test-gatt.c:312)
>>>> ==20817==    by 0x14159A: handle_rsp (att.c:640)
>>>> ==20817==    by 0x14159A: can_read_data (att.c:813)
>>>> ==20817==    by 0x14AEC4: watch_callback (io-glib.c:170)
>>>> ==20817==    by 0x4E808E4: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.4200.1)
>>>> ==20817==    by 0x4E80C47: ??? (in /usr/lib64/libglib-2.0.so.0.4200.1)
>>>> ==20817==    by 0x4E80F09: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.4200.1)
>>>> ==20817==    by 0x140030: tester_run (tester.c:830)
>>>> ==20817==    by 0x13BE18: main (test-gatt.c:3182)
>>>> ==20817==  Address 0x5a13909 is 9 bytes inside a block of size 40 free'd
>>>> ==20817==    at 0x4C2A37C: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
>>>> ==20817==    by 0x140F1E: cancel_att_send_op (att.c:222)
>>>> ==20817==    by 0x140F1E: bt_att_cancel (att.c:1200)
>>>> ==20817==    by 0x145075: cancel_request (gatt-client.c:1852)
>>>> ==20817==    by 0x14B902: queue_remove_all (queue.c:387)
>>>> ==20817==    by 0x1457CC: bt_gatt_client_cancel_all (gatt-client.c:1866)
>>>> ==20817==    by 0x145838: bt_gatt_client_free (gatt-client.c:1569)
>>>> ==20817==    by 0x13D438: destroy_context (test-gatt.c:284)
>>>> ==20817==    by 0x13D438: context_quit (test-gatt.c:312)
>>>> ==20817==    by 0x14159A: handle_rsp (att.c:640)
>>>> ==20817==    by 0x14159A: can_read_data (att.c:813)
>>>> ==20817==    by 0x14AEC4: watch_callback (io-glib.c:170)
>>>> ==20817==    by 0x4E808E4: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.4200.1)
>>>> ==20817==    by 0x4E80C47: ??? (in /usr/lib64/libglib-2.0.so.0.4200.1)
>>>> ==20817==    by 0x4E80F09: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.4200.1)
>>>> ==20817==
>>>> ==20817==
>>>> ==20817== HEAP SUMMARY:
>>>> ==20817==     in use at exit: 29,640 bytes in 618 blocks
>>>> ==20817==   total heap usage: 36,545 allocs, 35,927 frees, 1,585,464 bytes allocated
>>>> ==20817==
>>>> ==20817== LEAK SUMMARY:
>>>> ==20817==    definitely lost: 0 bytes in 0 blocks
>>>> ==20817==    indirectly lost: 0 bytes in 0 blocks
>>>> ==20817==      possibly lost: 0 bytes in 0 blocks
>>>> ==20817==    still reachable: 29,640 bytes in 618 blocks
>>>> ==20817==         suppressed: 0 bytes in 0 blocks
>>>> ==20817== Rerun with --leak-check=full to see details of leaked memory
>>>> ==20817==
>>>> ==20817== For counts of detected and suppressed errors, rerun with: -v
>>>> ==20817== Use --track-origins=yes to see where uninitialised values come from
>>>> ==20817== ERROR SUMMARY: 358 errors from 4 contexts (suppressed: 0 from 0)
>>>>
>>>> Unfortunately, my understanding of the code did not allow me
>>>> to fis this :-(
>>>>
>>>> Best regards,
>>>>
>>>>       Stefan
>>>>
>>
>>
>>



-- 
Luiz Augusto von Dentz

Re: [PATCH] core/gatt-database: Fix memory corruption

[Andrejs Hanins]

Luiz,

On 2015.03.12. 11:14, Luiz Augusto von Dentz wrote:
> Hi Andrejs,
> 
> On Thu, Mar 12, 2015 at 10:54 AM, Andrejs Hanins
> <andrejs.hanins@ubnt.com> wrote:
>> Hi Luiz,
>>
>> On 2015.03.12. 10:39, Luiz Augusto von Dentz wrote:
>>> Hi Andrejs,
>>>
>>> On Thu, Mar 12, 2015 at 10:24 AM, Andrejs Hanins
>>> <andrejs.hanins@ubnt.com> wrote:
>>>> Hi Lukasz,
>>>>
>>>> On 2015.03.11. 23:19, Stefan Seyfried wrote:
>>>>> Am 11.03.2015 um 15:35 schrieb Andrejs Hanins:
>>>>>> Pointer to on-stack variable was returned from pending_write_new.
>>>>>
>>>>> I still get a crash in the tests when running with memory debugging
>>>>> enabled (which is default in openSUSE Build Service):
>>>>>
>>>>> $> MALLOC_CHECK_=3 MALLOC_PERTURB_=69 unit/test-gatt
>>>>>
>>>>> /TP/GAC/CL/BV-01-C - init
>>>>> /TP/GAC/CL/BV-01-C - setup
>>>>> [...]
>>>>> /TP/GAR/CL/BV-01-C - setup complete
>>>>> /TP/GAR/CL/BV-01-C - run
>>>>> /TP/GAR/CL/BV-01-C - test passed
>>>>> Segmentation fault
>>>>>
>>>>> Program received signal SIGSEGV, Segmentation fault.
>>>>> 0x000055555558cb70 in bt_att_send (att=0x4000000f300432d, opcode=opcode@entry=24 '\030',
>>>>>     pdu=pdu@entry=0x7fffffff4a0f, length=length@entry=1,
>>>>>     callback=callback@entry=0x55555558fc30 <cancel_long_write_cb>, user_data=0x5555557b1ca0,
>>>>>     destroy=destroy@entry=0x0) at src/shared/att.c:1135
>>>>> 1135            if (!att || !att->io)
>>>>> (gdb) bt
>>>>> #0  0x000055555558cb70 in bt_att_send (att=0x4000000f300432d, opcode=opcode@entry=24 '\030',
>>>>>     pdu=pdu@entry=0x7fffffff4a0f, length=length@entry=1,
>>>>>     callback=callback@entry=0x55555558fc30 <cancel_long_write_cb>, user_data=0x5555557b1ca0,
>>>>>     destroy=destroy@entry=0x0) at src/shared/att.c:1135
>>>>> #1  0x0000555555591039 in cancel_long_write_req (client=<optimized out>, req=<optimized out>)
>>>>>     at src/shared/gatt-client.c:1791
>>>> Lukasz, I think there is some "missed-ref" problem related to the code you have recently added to the gatt-client/cancel_request() to cancel long_write and prep_write. Namely, bt_att_cancel can actually free the request which is later on accessed as req->long_write and req->prep_write thus reading free'd memory. Valgrind shows it happens this way:
>>>>
>>>>   Address 0x5a06fa8 is 8 bytes inside a block of size 40 free'd
>>>>     at 0x4C2B200: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
>>>>     by 0x415BC0: request_unref (gatt-client.c:160)   <<------ this one frees the request which is accessed later in cancel_request()
>>>>     by 0x410BB3: cancel_att_send_op (att.c:222)
>>>>     by 0x412700: bt_att_cancel (att.c:1194)
>>>>     by 0x418EC0: cancel_request (gatt-client.c:1852)
>>>>     by 0x421C91: queue_remove_all (queue.c:387)
>>>>     by 0x418F53: bt_gatt_client_cancel_all (gatt-client.c:1866)
>>>>     by 0x418601: bt_gatt_client_free (gatt-client.c:1569)
>>>>     by 0x418A65: bt_gatt_client_unref (gatt-client.c:1692)
>>>>     by 0x4021FB: destroy_context (test-gatt.c:284)
>>>>     by 0x40230C: context_quit (test-gatt.c:312)
>>>>     by 0x4031AC: test_read_cb (test-gatt.c:677)
>>>>
>>>> I can't instantly figure out the proper fix (add_ref to the request in the beginning of cancel_request() to avoid preliminary free?), hopefully it will be easier for you, as you are the author of the original code. Probably there are also other similar issues elsewhere.
>>>
>>> Yep, this seems a regression we introduced with prepare write set, but
>>> I did not managed to reproduce it with our unit tests how are getting
>>> this trace?
>>
>> valgrind --log-file=valgrind.txt  ./unit/test-gatt
>>
>> But if you do "MALLOC_CHECK_=3 MALLOC_PERTURB_=69 unit/test-gatt" as suggested by Stefan, then core dump also happens to me.
> 
> Ive just sent a patch for it, it is not crashing even with the following:
> 
> MALLOC_CHECK_=3 MALLOC_PERTURB_=69 valgrind --trace-children=yes
> --track-origins=yes --show-possibly-lost=no --leak-check=full
> unit/test-gatt
Thanks, works fine also for me now. Am I right, that even though this is a quite nasty bug IMO, there will be no new BlueZ release soon, as 5.29 was born just yesterday?

> 
> ==30730==
> ==30730== HEAP SUMMARY:
> ==30730==     in use at exit: 29,640 bytes in 618 blocks
> ==30730==   total heap usage: 36,559 allocs, 35,941 frees, 1,592,908
> bytes allocated
> ==30730==
> ==30730== LEAK SUMMARY:
> ==30730==    definitely lost: 0 bytes in 0 blocks
> ==30730==    indirectly lost: 0 bytes in 0 blocks
> ==30730==      possibly lost: 0 bytes in 0 blocks
> ==30730==    still reachable: 29,640 bytes in 618 blocks
> ==30730==         suppressed: 0 bytes in 0 blocks
> 
>>
>>>
>>>>
>>>>> #2  0x00005555555910ab in cancel_request (data=0x5555557b1e80) at src/shared/gatt-client.c:1855
>>>>> #3  0x0000555555597903 in queue_remove_all (queue=<optimized out>, function=function@entry=0x0,
>>>>>     user_data=user_data@entry=0x0, destroy=destroy@entry=0x555555591060 <cancel_request>)
>>>>>     at src/shared/queue.c:387
>>>>> #4  0x00005555555917cd in bt_gatt_client_cancel_all (client=client@entry=0x5555557b36f0)
>>>>>     at src/shared/gatt-client.c:1866
>>>>> #5  0x0000555555591839 in bt_gatt_client_free (client=0x5555557b36f0) at src/shared/gatt-client.c:1569
>>>>> #6  0x0000555555589439 in destroy_context (context=0x5555557b1bb0) at unit/test-gatt.c:284
>>>>> #7  context_quit (user_data=0x5555557b1bb0) at unit/test-gatt.c:312
>>>>> #8  0x000055555558d59b in handle_rsp (pdu_len=<optimized out>, pdu=0x5555557c6571 "\001\002\003\001)",
>>>>>     opcode=11 '\v', att=0x5555557b2e90) at src/shared/att.c:640
>>>>> #9  can_read_data (io=<optimized out>, user_data=0x5555557b2e90) at src/shared/att.c:813
>>>>> #10 0x0000555555596ec5 in watch_callback (channel=<optimized out>, cond=<optimized out>,
>>>>>     user_data=<optimized out>) at src/shared/io-glib.c:170
>>>>> #11 0x00007ffff7b198e5 in g_main_context_dispatch () from /usr/lib64/libglib-2.0.so.0
>>>>> #12 0x00007ffff7b19c48 in ?? () from /usr/lib64/libglib-2.0.so.0
>>>>> #13 0x00007ffff7b19f0a in g_main_loop_run () from /usr/lib64/libglib-2.0.so.0
>>>>> #14 0x000055555558c031 in tester_run () at src/shared/tester.c:830
>>>>> #15 0x0000555555587e19 in main (argc=1, argv=0x7fffffffdc58) at unit/test-gatt.c:3182
>>>>>
>>>>> Valgrind also complains loudly:
>>>>> $> valgrind unit/test-gatt > /dev/null
>>>>> ==20817== Memcheck, a memory error detector
>>>>> ==20817== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
>>>>> ==20817== Using Valgrind-3.10.0 and LibVEX; rerun with -h for copyright info
>>>>> ==20817== Command: unit/test-gatt
>>>>> ==20817==
>>>>> ==20817== Syscall param socketcall.bind(my_addr.sa_data) points to uninitialised byte(s)
>>>>> ==20817==    at 0x522A737: bind (in /lib64/libc-2.21.so)
>>>>> ==20817==    by 0x14BBC2: ecb_aes_setup (crypto.c:110)
>>>>> ==20817==    by 0x14BBC2: bt_crypto_new (crypto.c:148)
>>>>> ==20817==    by 0x140788: bt_att_new (att.c:937)
>>>>> ==20817==    by 0x13EA4B: create_context.constprop.24 (test-gatt.c:592)
>>>>> ==20817==    by 0x13F2E2: run_callback (tester.c:412)
>>>>> ==20817==    by 0x4E808E4: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.4200.1)
>>>>> ==20817==    by 0x4E80C47: ??? (in /usr/lib64/libglib-2.0.so.0.4200.1)
>>>>> ==20817==    by 0x4E80F09: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.4200.1)
>>>>> ==20817==    by 0x140030: tester_run (tester.c:830)
>>>>> ==20817==    by 0x13BE18: main (test-gatt.c:3182)
>>>>> ==20817==  Address 0xffeff6aa8 is on thread 1's stack
>>>>> ==20817==  in frame #1, created by bt_crypto_new (crypto.c:141)
>>>>> ==20817==
>>>> This one looks like a Valgrind bug. It probably does not take into account, that sockaddr passed to the bind is not a 'struct sockaddr' but actually a 'struct sockaddr_alg' of different size. The code, as such, does proper initialization.
>>>
>>> Valgrind probably is expecting some other size for the sockaddr, we do
>>> memset to 0 so it is probably just a false positive.
>>>
>>>>> ==20817== Syscall param socketcall.bind(my_addr.sa_data) points to uninitialised byte(s)
>>>>> ==20817==    at 0x522A737: bind (in /lib64/libc-2.21.so)
>>>>> ==20817==    by 0x14BC4B: cmac_aes_setup (crypto.c:132)
>>>>> ==20817==    by 0x14BC4B: bt_crypto_new (crypto.c:161)
>>>>> ==20817==    by 0x140788: bt_att_new (att.c:937)
>>>>> ==20817==    by 0x13EA4B: create_context.constprop.24 (test-gatt.c:592)
>>>>> ==20817==    by 0x13F2E2: run_callback (tester.c:412)
>>>>> ==20817==    by 0x4E808E4: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.4200.1)
>>>>> ==20817==    by 0x4E80C47: ??? (in /usr/lib64/libglib-2.0.so.0.4200.1)
>>>>> ==20817==    by 0x4E80F09: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.4200.1)
>>>>> ==20817==    by 0x140030: tester_run (tester.c:830)
>>>>> ==20817==    by 0x13BE18: main (test-gatt.c:3182)
>>>>> ==20817==  Address 0xffeff6aa8 is on thread 1's stack
>>>>> ==20817==  in frame #1, created by bt_crypto_new (crypto.c:141)
>>>>> ==20817==
>>>>> ==20817== Invalid read of size 1
>>>>> ==20817==    at 0x145076: cancel_request (gatt-client.c:1854)
>>>>> ==20817==    by 0x14B902: queue_remove_all (queue.c:387)
>>>>> ==20817==    by 0x1457CC: bt_gatt_client_cancel_all (gatt-client.c:1866)
>>>>> ==20817==    by 0x145838: bt_gatt_client_free (gatt-client.c:1569)
>>>>> ==20817==    by 0x13D438: destroy_context (test-gatt.c:284)
>>>>> ==20817==    by 0x13D438: context_quit (test-gatt.c:312)
>>>>> ==20817==    by 0x14159A: handle_rsp (att.c:640)
>>>>> ==20817==    by 0x14159A: can_read_data (att.c:813)
>>>>> ==20817==    by 0x14AEC4: watch_callback (io-glib.c:170)
>>>>> ==20817==    by 0x4E808E4: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.4200.1)
>>>>> ==20817==    by 0x4E80C47: ??? (in /usr/lib64/libglib-2.0.so.0.4200.1)
>>>>> ==20817==    by 0x4E80F09: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.4200.1)
>>>>> ==20817==    by 0x140030: tester_run (tester.c:830)
>>>>> ==20817==    by 0x13BE18: main (test-gatt.c:3182)
>>>>> ==20817==  Address 0x5a13908 is 8 bytes inside a block of size 40 free'd
>>>>> ==20817==    at 0x4C2A37C: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
>>>>> ==20817==    by 0x140F1E: cancel_att_send_op (att.c:222)
>>>>> ==20817==    by 0x140F1E: bt_att_cancel (att.c:1200)
>>>>> ==20817==    by 0x145075: cancel_request (gatt-client.c:1852)
>>>>> ==20817==    by 0x14B902: queue_remove_all (queue.c:387)
>>>>> ==20817==    by 0x1457CC: bt_gatt_client_cancel_all (gatt-client.c:1866)
>>>>> ==20817==    by 0x145838: bt_gatt_client_free (gatt-client.c:1569)
>>>>> ==20817==    by 0x13D438: destroy_context (test-gatt.c:284)
>>>>> ==20817==    by 0x13D438: context_quit (test-gatt.c:312)
>>>>> ==20817==    by 0x14159A: handle_rsp (att.c:640)
>>>>> ==20817==    by 0x14159A: can_read_data (att.c:813)
>>>>> ==20817==    by 0x14AEC4: watch_callback (io-glib.c:170)
>>>>> ==20817==    by 0x4E808E4: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.4200.1)
>>>>> ==20817==    by 0x4E80C47: ??? (in /usr/lib64/libglib-2.0.so.0.4200.1)
>>>>> ==20817==    by 0x4E80F09: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.4200.1)
>>>>> ==20817==
>>>>> ==20817== Invalid read of size 1
>>>>> ==20817==    at 0x14507C: cancel_request (gatt-client.c:1857)
>>>>> ==20817==    by 0x14B902: queue_remove_all (queue.c:387)
>>>>> ==20817==    by 0x1457CC: bt_gatt_client_cancel_all (gatt-client.c:1866)
>>>>> ==20817==    by 0x145838: bt_gatt_client_free (gatt-client.c:1569)
>>>>> ==20817==    by 0x13D438: destroy_context (test-gatt.c:284)
>>>>> ==20817==    by 0x13D438: context_quit (test-gatt.c:312)
>>>>> ==20817==    by 0x14159A: handle_rsp (att.c:640)
>>>>> ==20817==    by 0x14159A: can_read_data (att.c:813)
>>>>> ==20817==    by 0x14AEC4: watch_callback (io-glib.c:170)
>>>>> ==20817==    by 0x4E808E4: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.4200.1)
>>>>> ==20817==    by 0x4E80C47: ??? (in /usr/lib64/libglib-2.0.so.0.4200.1)
>>>>> ==20817==    by 0x4E80F09: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.4200.1)
>>>>> ==20817==    by 0x140030: tester_run (tester.c:830)
>>>>> ==20817==    by 0x13BE18: main (test-gatt.c:3182)
>>>>> ==20817==  Address 0x5a13909 is 9 bytes inside a block of size 40 free'd
>>>>> ==20817==    at 0x4C2A37C: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
>>>>> ==20817==    by 0x140F1E: cancel_att_send_op (att.c:222)
>>>>> ==20817==    by 0x140F1E: bt_att_cancel (att.c:1200)
>>>>> ==20817==    by 0x145075: cancel_request (gatt-client.c:1852)
>>>>> ==20817==    by 0x14B902: queue_remove_all (queue.c:387)
>>>>> ==20817==    by 0x1457CC: bt_gatt_client_cancel_all (gatt-client.c:1866)
>>>>> ==20817==    by 0x145838: bt_gatt_client_free (gatt-client.c:1569)
>>>>> ==20817==    by 0x13D438: destroy_context (test-gatt.c:284)
>>>>> ==20817==    by 0x13D438: context_quit (test-gatt.c:312)
>>>>> ==20817==    by 0x14159A: handle_rsp (att.c:640)
>>>>> ==20817==    by 0x14159A: can_read_data (att.c:813)
>>>>> ==20817==    by 0x14AEC4: watch_callback (io-glib.c:170)
>>>>> ==20817==    by 0x4E808E4: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.4200.1)
>>>>> ==20817==    by 0x4E80C47: ??? (in /usr/lib64/libglib-2.0.so.0.4200.1)
>>>>> ==20817==    by 0x4E80F09: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.4200.1)
>>>>> ==20817==
>>>>> ==20817==
>>>>> ==20817== HEAP SUMMARY:
>>>>> ==20817==     in use at exit: 29,640 bytes in 618 blocks
>>>>> ==20817==   total heap usage: 36,545 allocs, 35,927 frees, 1,585,464 bytes allocated
>>>>> ==20817==
>>>>> ==20817== LEAK SUMMARY:
>>>>> ==20817==    definitely lost: 0 bytes in 0 blocks
>>>>> ==20817==    indirectly lost: 0 bytes in 0 blocks
>>>>> ==20817==      possibly lost: 0 bytes in 0 blocks
>>>>> ==20817==    still reachable: 29,640 bytes in 618 blocks
>>>>> ==20817==         suppressed: 0 bytes in 0 blocks
>>>>> ==20817== Rerun with --leak-check=full to see details of leaked memory
>>>>> ==20817==
>>>>> ==20817== For counts of detected and suppressed errors, rerun with: -v
>>>>> ==20817== Use --track-origins=yes to see where uninitialised values come from
>>>>> ==20817== ERROR SUMMARY: 358 errors from 4 contexts (suppressed: 0 from 0)
>>>>>
>>>>> Unfortunately, my understanding of the code did not allow me
>>>>> to fis this :-(
>>>>>
>>>>> Best regards,
>>>>>
>>>>>       Stefan
>>>>>
>>>
>>>
>>>
> 
> 
> 

Re: [PATCH] core/gatt-database: Fix memory corruption

[Johan Hedberg]

Hi Andrejs,

On Thu, Mar 12, 2015, Andrejs Hanins wrote:
> Thanks, works fine also for me now. Am I right, that even though this
> is a quite nasty bug IMO, there will be no new BlueZ release soon, as
> 5.29 was born just yesterday?

We'll likely release 5.30 earlier than planned, maybe end of next week
or something like that. There are several other features that we should
be able to get into it (like the GattProfile1 interface support), so it
wont have to be a pure bug-fix release.

Johan