How serious is the next memory leak shown by Valgrind

Question

After writing thousands of lines of code, I use valgrind and am horrified to see the number of errors. I used to use GDB. Most of my errors are related to string functions. I am sending a piece. I understand that an error occurs because strlen does not account for the final NULL, while strcpy adds it. How serious is this? Do I really need to fix them? I can fix this, but am worried if this can lead to more errors, since my code did not remember it when I was in it.

Will strcpy copy trailing NULL even if no space has been reserved for it?

t.write_length = (strlen("NA\n");/*Line number 116*/ t.data = malloc(strlen("NA\n");/*117*/ strcpy(t.data,"NA\n");/*118*/ 

Valgrind:

 ==3287== Invalid write of size 1 ==3287== at 0x400764E: memcpy (mc_replace_strmem.c:497) ==3287== by 0x804A714: log_txn_commit (Log_manager.c:118) ==3287== by 0x8049D3C: on_txn_commit (TxFS_manager.c:85) ==3287== by 0x804939E: handler (Reader.c:139) ==3287== by 0xBF5F18: start_thread (in /lib/libpthread-2.12.90.so) ==3287== by 0xB37A2D: clone (in /lib/libc-2.12.90.so) ==3287== Address 0x403282b is 0 bytes after a block of size 3 alloc'd ==3287== at 0x4005BDC: malloc (vg_replace_malloc.c:195) ==3287== by 0x804A6F5: log_txn_commit (Log_manager.c:117) ==3287== by 0x8049D3C: on_txn_commit (TxFS_manager.c:85) ==3287== by 0x804939E: handler (Reader.c:139) ==3287== by 0xBF5F18: start_thread (in /lib/libpthread-2.12.90.so) ==3287== by 0xB37A2D: clone (in /lib/libc-2.12.90.so) 

Answer 1

This is a serious memory overwrite issue. Your code should be

 t.write_length = strlen("NA\n");/*Line number 116*/ t.data = malloc(t.write_length + 1); strcpy(t.data,"NA\n"); 

and be sure to install. strcpy() will add the terminator '\0' for which there is no space.

To avoid overflow, the size of the array pointed to by the receiver should be long enough to contain the same C string as the source (including the terminating null character) and should not overlap in memory using the source.

Always take Valgrind recommendations seriously!

Answer 2

You always want to fix the errors reported by Valgrind. Invalid entries lead to unexpected behavior, which by definition is not what your program should do. Depending on how your program is laid out in memory, you may overwrite other important variables or not completely write down what you expect.

If fixing this leads to more errors in your code, it means that other parts of your code are wrong, not the Valgrind report. You must fix this error, and if this leads to further error messages, you will also fix them. Ignore incorrect read / write errors at your own risk.

Answer 3

Yes, you always need malloc(strlen(str) + 1) bytes for the string (for this annoying null terminator). Or it is easier to use strdup .

Answer 4

Potentially pretty serious, and you have to fix it. Consider using either strdup() if you need trailing null or memcpy() if you don't need trailing null.