Invalid Read in Valgrind

Question

I keep seeing these errors in Valgrind. My program also segfaults, but I'm trying to work out the errors in Valgrind first.

81 char *ptr = NULL;
82      if (addition_is_safe(sz, TRAILER_SZ))
83          ptr = malloc(sz + TRAILER_SZ);
84 syslog(LOG_NOTICE, "Calling set trailer value, ptr=%p", ptr);

prints this to the log:

Sep 17 00:05:50 appliance m61[18301]: Calling set trailer value, ptr=0x96af158

Valgrind:

==18069== 2 errors in context 1 of 4:
==18069== Invalid read of size 1
==18069==    at 0x402BCCA: memcpy (mc_replace_strmem.c:882)
==18069==    by 0x8048819: main (test025.c:13)
==18069==  Address 0x4213709 is 137 bytes inside a block of size 368 free'd
==18069==    at 0x4028F0F: free (vg_replace_malloc.c:446)
==18069==    by 0x40AA00B: fclose@@GLIBC_2.1 (iofclose.c:88)
==18069==    by 0x4136003: __vsyslog_chk (syslog.c:228)
==18069==    by 0x4136446: syslog (syslog.c:119)
==18069==    by 0x8049823: attempt_create_allocation (m61_allocation_core.c:84)
==18069==    by 0x8048914: m61_malloc (m61.c:47)
==18069==    by 0x80487B4: main (test025.c:9)
==18069== 

EDIT: The test code I ran was actually meant to produce an invalid free to be caught in Valgrind and my memory debugging code. However, it wasn't supposed to segfault, which is what was happening. Once I turned off syslog, there was no longer a seg fault and the Valgrind report is as expected for the test code. I don't know why syslog caused an issue above.

Test Code:

1 #include "m61.h"
2 #include <stdio.h>
3 #include <assert.h>
4 #include <string.h>


7 int main() {
8    char *a = (char *) malloc(200);
9    char *b = (char *) malloc(50);
10   char *c = (char *) malloc(200);
11   char *p = (char *) malloc(3000);
12   (void) a, (void) c;
13   memcpy(p, b - 200, 450);
14   free(p + 200);

New Valgrind Report After turning off Valgrind:

==5688== 18 errors in context 1 of 3:
==5688== Invalid read of size 4
==5688==    at 0x402BD00: memcpy (mc_replace_strmem.c:882)
==5688==    by 0x8048849: main (test025.c:13)
==5688==  Address 0x41f92c0 is 16 bytes before a block of size 208 alloc'd
==5688==    at 0x4029F6F: malloc (vg_replace_malloc.c:270)
==5688==    by 0x80497AD: attempt_create_allocation (m61_allocation_core.c:77)
==5688==    by 0x8048950: m61_malloc (m61.c:48)
==5688==    by 0x8048804: main (test025.c:10)

Answer 1

Initial attempt

The read error you're getting means you allocated some space that was then freed, but some part of your program managed to keep a pointer to the middle of the block of memory, and then tried to read a byte from inside that block with memcpy() in main() at line 13.

What is not so clear is where the memory was allocated. It looks as if the free was done inside a call to syslog(), which is rather curious.

You will need to look at your code in test025.c rather carefully. If m61.c is your code, you should look at that too (and presumably m61_allocation_core.c too). The code in vg_replace_malloc.c is definitively valgrind code; I'm not sure about the parts in between.

Revised code — revised answer

The memcpy() in your revised code is clearly 100% erroneous. Equally, the free() is 100% erroneous. To see why, you have to remember that malloc() typically stores some control information before the pointer that is allocated. You also need to know that trying to free() a pointer that was not returned by malloc(), calloc() or realloc() is a serious error.

Your code is:

 7 int main() {
 8     char *a = (char *) malloc(200);
 9     char *b = (char *) malloc(50);
10     char *c = (char *) malloc(200);
11     char *p = (char *) malloc(3000);
12     (void) a, (void) c;
13     memcpy(p, b - 200, 450);
14     free(p + 200);

Apart from error checking (or the absence thereof), lines 7-11 are unexceptionable. There are those who will excoriate you for the casts; I'm not of that school of thought. You have my permission to ignore them — as long as you promise you compile with -Wmissing-prototypes -Wstrict-prototypes (and -Wall -Wextra) and you fix the problems reported before running the code. If you don't, you need to heed me (about using the stringent compiler options and fixing the problems identified by the compiler warnings) or them (about not casting the result of malloc()).

Line 12 avoids 'unused variable' warnings; it is otherwise a no-op.

Line 13, the memcpy(), tries to copy from 200 bytes before the start of b to 200 bytes beyond the end of the space allocated for b. Of those 450 bytes, accessing 400 of them leads to undefined behaviour. To the extent that you are only reading memory, and the destination — p — is big enough, you are 'OK'. But reading through the bytes either side of b is not valid and valgrind is fully correct to complain.

Line 14, the free(), tries to free a pointer that was not returned by malloc(). As such, you are invoking undefined behaviour. There is nothing that you can expect to happen except disaster. The chances are that the free() itself won't fail — though a debugging malloc() would notice that you are freeing from the middle of allocated space. But any subsequent memory allocation or release is very problematic. The trouble is that free() will look at the control information before (typically) the space pointed at, and that information won't be valid.