[PATCH] Fix double free in tui_source_element

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] Fix double free in tui_source_element

Bogdan Harjoc
To reproduce, cycle a few times between these layouts: no tui, tui
one-window, tui two-windows (including some layout that shows
disassembly).

tui_set_source_content() expands win_info->content, and has to move
tui_source_element items to the new vector storage, destroying the
items in the old storage, and ~tui_source_element() calls xfree on
'line'. Due to a missing copy ctor, items in the new storage have the
old 'line' pointer which eventually gets freed again. Patch is
attached, I added DISABLE_COPY_AND_ASSIGN() in a few more tui classes
to check for more similar issues.

Valgrind output:

==27971== Invalid free() / delete / delete[] / realloc()
void xfree<char>(char*) (common-utils.h:60)
tui_set_source_content(...) (tui-source.c:198)
tui_update_source_window_as_is(...) (tui-winsource.c:95)
tui_show_symtab_source(...) (tui-source.c:221)
tui_update_source_windows_with_addr(...) (tui-winsource.c:152)
tui_set_layout(tui_layout_type) (tui-layout.c:198)

==27971==  Address 0x... is 0 bytes inside a block of size 63 free'd
void xfree<char> (common-utils.h:60)
tui_source_element::~tui_source_element() (tui-data.h:184)
void std::_Destroy<tui_source_element,...) (stl_construct.h:206)
std::vector<tui_source_element,...>::_M_default_append(...)
std::vector<tui_source_element, ...>::resize(...) (stl_vector.h:692)
tui_alloc_source_buffer(...) (tui-winsource.c:693)
tui_set_source_content(...) (tui-source.c:138)

gdb-tui-source-element-move-ctor.patch (3K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fix double free in tui_source_element

Tom Tromey-2
>>>>> "Bogdan" == Bogdan Harjoc <[hidden email]> writes:

Bogdan> To reproduce, cycle a few times between these layouts: no tui, tui
Bogdan> one-window, tui two-windows (including some layout that shows
Bogdan> disassembly).

Bogdan> tui_set_source_content() expands win_info->content, and has to move
Bogdan> tui_source_element items to the new vector storage, destroying the
Bogdan> items in the old storage, and ~tui_source_element() calls xfree on
Bogdan> 'line'. Due to a missing copy ctor, items in the new storage have the
Bogdan> old 'line' pointer which eventually gets freed again. Patch is
Bogdan> attached, I added DISABLE_COPY_AND_ASSIGN() in a few more tui classes
Bogdan> to check for more similar issues.

Thanks.  I already have something like this on my big TUI refactoring
branch, but your patch is better.

It needs a ChangeLog entry.
Also, do you have a copyright assignment in place?

Tom