kellabyte/Haywire — pull request #28 by jbcrail (2013-06-03 02:25:33+00:00)
Title: Fix for bad reference to freed memory

Given the following test setup and the resulting output, I found a bug when deleting a key from the radix trie:

valgrind -v --tool=memcheck --leak-check=full --show-reachable=yes --track-origins=yes ./builds/unix/debug/hello_world

ab -n 10 -c 1 -k http://0.0.0.0:8000/

```text
==10566== Memcheck, a memory error detector
==10566== Copyright (C) 2002-2012, and GNU GPL'd, by Julian Seward et al.
==10566== Using Valgrind-3.8.1 and LibVEX; rerun with -h for copyright info
==10566== Command: ./builds/unix/debug/hello_world
==10566==
--10566-- Valgrind options:
--10566-- --suppressions=/usr/lib/valgrind/debian-libc6-dbg.supp
--10566-- -v
--10566-- --tool=memcheck
--10566-- --leak-check=full
--10566-- --show-reachable=yes
--10566-- --track-origins=yes
...
Added route /
--10566-- REDIR: 0x52dc9b0 (free) redirected to 0x4c2ba00 (free)
--10566-- REDIR: 0x52dcaa0 (realloc) redirected to 0x4c2cf10 (realloc)
--10566-- REDIR: 0x52e2c40 (__GI_strlen) redirected to 0x4c2d7c0 (__GI_strlen)
Listening on 0.0.0.0:8000
--10566-- REDIR: 0x52e2e00 (strncmp) redirected to 0x4a25710 (_vgnU_ifunc_wrapper)
--10566-- REDIR: 0x5392980 (__strncmp_sse42) redirected to 0x4c2dc20 (strncmp)
==10566== Invalid read of size 8
==10566== at 0x404516: delete_internal (radix.c:369)
==10566== by 0x4045E2: rxt_delete (radix.c:391)
==10566== by 0x408964: http_request_on_header_field (http_request.c:71)
==10566== by 0x406485: http_parser_execute (http_parser.c:1334)
==10566== by 0x4037BC: http_stream_on_read (http_server.c:113)
==10566== by 0x411C80: uv__read (stream.c:1023)
==10566== by 0x4120B7: uv__stream_io (stream.c:1118)
==10566== by 0x417A1E: uv__io_poll (linux-core.c:211)
==10566== by 0x40A59B: uv_run (core.c:317)
==10566== by 0x403643: hw_http_open (http_server.c:71)
==10566== by 0x4034C8: main (program.c:26)
==10566== Address 0x5631c98 is 8 bytes inside a block of size 192 free'd
==10566== at 0x4C2BA6C: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==10566== by 0x404511: delete_internal (radix.c:366)
==10566== by 0x4045E2: rxt_delete (radix.c:391)
==10566== by 0x408964: http_request_on_header_field (http_request.c:71)
==10566== by 0x406485: http_parser_execute (http_parser.c:1334)
==10566== by 0x4037BC: http_stream_on_read (http_server.c:113)
==10566== by 0x411C80: uv__read (stream.c:1023)
==10566== by 0x4120B7: uv__stream_io (stream.c:1118)
==10566== by 0x417A1E: uv__io_poll (linux-core.c:211)
==10566== by 0x40A59B: uv_run (core.c:317)
==10566== by 0x403643: hw_http_open (http_server.c:71)
==10566== by 0x4034C8: main (program.c:26)
==10566==
==10566== Invalid read of size 8
==10566== at 0x404516: delete_internal (radix.c:369)
==10566== by 0x4045E2: rxt_delete (radix.c:391)
==10566== by 0x408AC4: http_request_on_headers_complete (http_request.c:106)
==10566== by 0x406F31: http_parser_execute (http_parser.c:1580)
==10566== by 0x4037BC: http_stream_on_read (http_server.c:113)
==10566== by 0x411C80: uv__read (stream.c:1023)
==10566== by 0x4120B7: uv__stream_io (stream.c:1118)
==10566== by 0x417A1E: uv__io_poll (linux-core.c:211)
==10566== by 0x40A59B: uv_run (core.c:317)
==10566== by 0x403643: hw_http_open (http_server.c:71)
==10566== by 0x4034C8: main (program.c:26)
==10566== Address 0x5632388 is 8 bytes inside a block of size 192 free'd
==10566== at 0x4C2BA6C: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==10566== by 0x404511: delete_internal (radix.c:366)
==10566== by 0x4045E2: rxt_delete (radix.c:391)
==10566== by 0x408AC4: http_request_on_headers_complete (http_request.c:106)
==10566== by 0x406F31: http_parser_execute (http_parser.c:1580)
==10566== by 0x4037BC: http_stream_on_read (http_server.c:113)
==10566== by 0x411C80: uv__read (stream.c:1023)
==10566== by 0x4120B7: uv__stream_io (stream.c:1118)
==10566== by 0x417A1E: uv__io_poll (linux-core.c:211)
==10566== by 0x40A59B: uv_run (core.c:317)
==10566== by 0x403643: hw_http_open (http_server.c:71)
==10566== by 0x4034C8: main (program.c:26)
```

After freeing the `sibling` pointer in `delete_internal()`, the `reset_key` function references the freed pointer. I moved the call to `reset_key()` inside the if statement and before `free()` is called. If you had added `sibling = NULL;` after `free()`, you would have triggered a segmentation fault.