search

davidmoreno / onion

C library to create simple HTTP servers and Web Applications.
http://www.coralbits.com/libonion/
Other
2.02k stars 250 forks source link

Fix double free in examples/ofileserver #253

Closed ghost closed 4 years ago

ghost commented 5 years ago

Running examples/ofileserver with valgrind revealed double free, which is caused by calling free_onion (custom function that calls onion_free) on SIGINT and then calling onion_free. So this pull request just removes that second onion_free.

davidmoreno commented 5 years ago

Hi,

I could not reproduce the double free. If you can reproduce, tell me please the steps.

It should not do double check as if it gets to the free_onion on a SIGINT, it does an exit. So nocalling on the other onion_free.

I can not see any condition on which it will call both.

Anyway the second one is not reachable currently (there is no code to stop the onion_listen in this example), but it is a good practice to free all data after use.

If the double free is indeed happening anywhere I would add something like

if (o){
  onion_free(o)
 o = NULL;
}

both in free_onion and at the end of the main function.

I close the PR, but feel free to reopen it with more information.

ghost commented 5 years ago

Hm, didn't happen to me as well this time, so I ran it again, made a request, quickly stopped it with Ctrl + C and it triggered double free... I'm attaching screenshot of terminal (in case someone didn't believe me) and output from terminal copied into text file. It leaked 952 bytes both times, so that should be something to look into as well.

log.txt prtsc

ghost commented 5 years ago

Also, I cannot reopen this PR...

davidmoreno commented 5 years ago

Can you try the code I suggested to check if it still gives the same problem?

if (o){
  onion_free(o)
 o = NULL;
}
ghost commented 5 years ago

Seems to have fixed it.