Remove dead code

[stoeckmann]

HAVE_REALLOC is never set, so even if the system has realloc support
it was never used.

Removing the dead code is the easiest solution. Otherwise keep in
mind that simply enabling the realloc version leads to a possible
use after free bug (linebuf.buf could still point to the previous
location since it is not updated after free, and calling free anyway
is not a good idea because it is assumed that linebuf's buffers are
never NULL).

[stoeckmann]

See https://marc.info/?l=openbsd-tech&m=164096045901110&w=2 for more information about a possible realloc-fix (OpenBSD less).

[gwsw]

I'm not seeing the problem you mentioned in the openbsd thread. If the new_buf alloc succeeds but the new_attr alloc fails, then both new_buf and new_attr will be freed (the new_attr free does nothing of course) and both global variables linebuf and attr remain pointing to the previous buffers, neither of which is freed. How does the use-after-free happen?

[gwsw]

And I don't understand "linebuf.buf could still point to the previous location since it is not updated after free". I must be missing something, because it seems to me that after the free() in line 192, linebuf.buf is immediately updated in line 194. You must be referring to something else?

[stoeckmann]

If the new_buf realloc succeeds, then linebuf still points to the old location. The old location might have been freed by realloc, as stated in its manual page:

If the area pointed to was moved, a free(ptr) is done.

This effectively means that right after a successful return of realloc, linebuf has to be considered invalid and must not be referenced anymore. Leaving the function with return 1 -- regardless of freeing new_buf -- leads eventually to a dereference of linebuf which points to an already freed area.

Regarding your comment about update of linebuf in 194 after reaching line 192: This is correct and the code works as expected. Keep in mind that 192 is reached if HAVE_REALLOC is not set. My report covers the theoretical case in which HAVE_REALLOC would be set.

And last but not least, the use after free bug only happens if the first realloc succeeds and the second does not, i.e. we have to reach the error handling if-body.

[gwsw]

Thanks, you're correct that I didn't properly handle the case where realloc returns the original buffer.