malloc memory corruption, fopen

3
votes

I've tried using GDB and Valgrind, but I can't seem to pinpoint the problem. Interestingly, the program crashes during normal execution and GDB, but not Valgrid.

To help you follow along with the code, heres the basic point of the program: Communicate with a server via sockets and UDP to transfer a file, and handle some basic packet loss.

I won't share the server's code, because I know the issue isn't there. The point that might confuse some, is that I'm implementing packet loss myself, with a number generator. Right now it doesn't do anything really, besides make the program use another recvfrom.

To guide you throught the programs output, the client tells the server what file it wants, the server tells the client how big the file is it's going to send, and then sends it in chunks (of 10 characters at a time).

The output shows what chunk is sent, how many characters were received, and what the concatenated string is.

The file transfer succeeds from what i can tell, its just the fopen call that I use to write the received file that is giving me trouble. Not sure if it's to do with my malloc call or not.

Here is the source code:

pastebin.com/Z79hvw6L

Here are the outputs from CLI execution, and Valgrind (GDB doesn't seem to give any more info):

Notice the CLI gives a malloc memory corruption error, and Valgrind doesn't.

CLI: http://pastebin.com/qdTKMCD2

VALGRIND: http://pastebin.com/8inRygnU

Thanks for any help!

Added the GDB Backtrace results

======= Backtrace: =========
/lib/i386-linux-gnu/libc.so.6(+0x6b961)[0x19a961]
/lib/i386-linux-gnu/libc.so.6(+0x6e15d)[0x19d15d]
/lib/i386-linux-gnu/libc.so.6(__libc_malloc+0x63)[0x19ef53]
/lib/i386-linux-gnu/libc.so.6(+0x5c2b8)[0x18b2b8]
/lib/i386-linux-gnu/libc.so.6(fopen+0x2c)[0x18b38c]
/home/---/client[0x8048dc2]
/lib/i386-linux-gnu/libc.so.6(__libc_start_main+0xe7)[0x145e37]
/home/---/client[0x8048871]

Maybe this could give someone an insight as to what part of the program the error is in?

3
csocketsudpmallocfopen
In general, if you can't post a relevant source subset within the context of your post, you won't get many responses. – Joe
Why don't you post the backtrace of where it crashes in valgrind? I bet that if you get that backtrace, then set a memory watchpoint on the corrupted memory (the point of the crash) it will lead you to your problem. – dbeer
What do you mean Joe? The area I think the problem is in? – user974703
@dbeer, unless I'm misinterpreting the valgrind output, it's not crashing at all in valgrind? – user974703
added backtrace. I've been watching my string that hold all of the chunks, and it seems to turn out fine. It's right at the fopen that it crashes. – user974703

3 Answers

3
votes
char chunk[10];
chunk[10] = '\0';

is wrong, chunk[10] is one past the array.

And in general, be careful with doing this

char filename[25];
scanf("%s",filename);

If you enter a long filename, you'll trash memory. using fgets() would be better. You also would at least want to check if scanf succeeds, else the following strlen() on filename isn't valid.

line 93, buf[strlen(buf)-1]='\0'; is dangerous, you can't use strlen if the buffer isn't already nul terminated, and you trash memory if buf is an empty string, as you index buf[-1].

Edit. Your other problem is strcat(fullstring,chunk); , you've no control in your loop that stops appending to this string if you happen to receive more data than it can hold. The size is also likely off by one, as you need room for the last nul terminator. Make it at least char * fullstring = malloc(sizeof(char)*filesize + 1 ); But your loop really needs to check that's it is not writing past the end of that buffer.

As for adding a nul terminator to buf , the recv call returns how many bytes you've read, so if you've checked recv for errors, do buf[numbytes] = 0 , but this will be off by one as well, as you've allocated 10 bytes for buf and you try to read 10 bytes into it as well - but in C, a string needs room for a nul terminator too. Make buf 11 bytes big. Or recv() only 9 bytes.

In fact, you're off by one many places, so start counting how many bytes you need, and were you put stuff into them. Remember that in C, arrays starts with index zero, and an array of 10 can only be indexed by index 0 to 9.

2
votes

This (line 93) is suspect:

buf[strlen(buf)-1]='\0';

UPDATE This (line 99,100) is also wrong:

char chunk[10];
chunk[10] = '\0';

UPDATE2: The buffer is too small

char * fullstring = malloc(sizeof(char)*filesize); // line 103
...
strcat(fullstring,chunk); // line 124

UPDATE3: UDP is unreliable. Transmission of a packet may fail (packets may be dropped anywhere between sender and receiver) , and the packets may be received in a different order than in which you sent them.

1
votes

Well, it shouldn't be a problem on modern OS:es but you don't check the returned value from malloc() for NULL. On what line does it crash and with what signal?