Valgrind reported this error to me recently:
==258178== Invalid read of size 8
==258178== at 0x3579AB: MQTTClient_waitForCompletion (in /home/nhorman/git/privafy/test/microedge-c/test)
==258178== by 0x1724BF: publish_msg (comms_connect.c:625)
==258178== by 0x16F55A: _send_message (comms_register.c:294)
==258178== by 0x16F605: send_message_sync (comms_register.c:311)
==258178== by 0x178AEA: send_obj_write_resp_mqtt_message (day0_mqtt_state.c:49)
==258178== by 0x178FE5: day0_obj_write_req_mqtt_handler (day0_mqtt_state.c:170)
==258178== by 0x1777A5: service_state_machine_run (service_state_machine_handler.c:670)
==258178== by 0x49EE12C: start_thread (pthread_create.c:442)
==258178== by 0x4A6ED73: clone (clone.S:100)
==258178== Address 0x5075e38 is 24 bytes inside a block of size 152 free'd
==258178== at 0x48460E4: free (vg_replace_malloc.c:884)
==258178== by 0x3673FD: ListUnlink (in /home/nhorman/git/privafy/test/microedge-c/test)
==258178== by 0x3674C0: ListRemove (in /home/nhorman/git/privafy/test/microedge-c/test)
==258178== by 0x35206D: MQTTClient_destroy (in /home/nhorman/git/privafy/test/microedge-c/test)
==258178== by 0x171BBD: comms_run (comms_connect.c:445)
==258178== by 0x49EE12C: start_thread (pthread_create.c:442)
==258178== by 0x4A6ED73: clone (clone.S:100)
==258178== Block was alloc'd at
==258178== at 0x484386F: malloc (vg_replace_malloc.c:393)
==258178== by 0x351A96: MQTTClient_createWithOptions (in /home/nhorman/git/privafy/test/microedge-c/test)
==258178== by 0x171987: comms_run (comms_connect.c:409)
==258178== by 0x49EE12C: start_thread (pthread_create.c:442)
==258178== by 0x4A6ED73: clone (clone.S:100)
Its occuring because, in MQTTClient_waitForCompletion, the
mqttclient_mutex is repeatedly dropped and re-aquired. While thats fine
to do, it creates the possibility for another thread to run between the
time MQTClient_waitForCompletion calls MQTTClient_yield and the time it
reaquires the lock. Valgrind notes that during that period, if another
thread calls MQTTCilent_destroy, the value of m->c in
MQTTClient_waitForCompletion will change, triggering the above invalid
read warning.
Fix is pretty easy, just move the if (m == NULL || m->c == NULL)
conditional to inside the while loop, so that the value of m->c is
revalidated after each lock re-aquisition.
Signed-off-by: Neil Horman <nhorman@gmail.com>
checkEyecatchers() used vsnprintf() with a format specifier "%d" to print the wrong eyecatcher content, but an eyecatcher was defined as 'double'.
Since "%d" expects an 'int' on the stack (which is usually 32 bit in size) but gets a 'double' instead (which is usually 64 bit), the following 'file' argument will be retrieved as wrong pointer value from stack, resulting in a crash in Log() -> vsnprintf() -> strlen().
This was fixed by defining 'eyecatcher' as 'uint64_t' (instead of 'double') and printing an eyecatcher using 'PRIx64' (instead of "d").
Signed-off-by: Axel Sommerfeldt <axel.sommerfeldt@fastmail.de>
Encountered this valgrind error while testing some code the other day:
==2568== Memcheck, a memory error detector
==2568== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==2568== Using Valgrind-3.16.1 and LibVEX; rerun with -h for copyright info
==2568== Command: ./test
==2568== Parent PID: 2560
==2568==
==2568== Thread 7:
==2568== Invalid read of size 8
==2568== at 0x4CDA970: connectionLost_call (in /usr/lib/libpaho-mqtt3cs.so.1.3.9)
==2568== by 0x500AEA6: start_thread (pthread_create.c:477)
==2568== by 0x5121A2E: clone (clone.S:95)
==2568== Address 0x91e8238 is 40 bytes inside a block of size 176 free'd
==2568== at 0x48399AB: free (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==2568== by 0x4CF8B0F: myfree (in /usr/lib/libpaho-mqtt3cs.so.1.3.9)
==2568== by 0x4CF3947: ListUnlink (in /usr/lib/libpaho-mqtt3cs.so.1.3.9)
==2568== by 0x4CF3A16: ListRemove (in /usr/lib/libpaho-mqtt3cs.so.1.3.9)
==2568== by 0x4CDA612: MQTTClient_destroy (in /usr/lib/libpaho-mqtt3cs.so.1.3.9)
==2568== by 0x142BC0: comms_run (comms_connect.c:436)
==2568== by 0x500AEA6: start_thread (pthread_create.c:477)
==2568== by 0x5121A2E: clone (clone.S:95)
==2568== Block was alloc'd at
==2568== at 0x483877F: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==2568== by 0x4CF8725: mymalloc (in /usr/lib/libpaho-mqtt3cs.so.1.3.9)
==2568== by 0x4CD9F58: MQTTClient_createWithOptions (in /usr/lib/libpaho-mqtt3cs.so.1.3.9)
==2568== by 0x1428FB: comms_run (comms_connect.c:404)
==2568== by 0x500AEA6: start_thread (pthread_create.c:477)
==2568== by 0x5121A2E: clone (clone.S:95)
==2568==
It indicates a use after free of the MQTTClient data structure in the
connectionLost_call.
A bit of digging revealed that the problem was that my code Called
MQTTClient_disconnect and MQTTClient_destroy in rapid succession.
Because MQTTClient_disconnect does this:
Thread_start(connectionLost_call, m);
Its entirely possible for connectionLost_call to be executed after the
return from MQTTClient_disconnect completes, at which time the Client
may be destroyed/freed, leading to the above valgrind error.
fix is pretty straightforward. We could use a mutex and condition
variable to serialize the two threads, but Windows doesn't have support
for condition variables currently, so just use one time semaphore to
ensure serialization
Signed-off-by: Neil Horman <nhorman@gmail.com>
convert to use of semaphore
CIPop