Skip to content

Fix some memory leaks in the BAP plugin - #46

Merged
Rot127 merged 1 commit into
BinaryAnalysisPlatform:trace-10.0from
moste00:fix/plugin_mem_leaks
Jun 28, 2026
Merged

Fix some memory leaks in the BAP plugin#46
Rot127 merged 1 commit into
BinaryAnalysisPlatform:trace-10.0from
moste00:fix/plugin_mem_leaks

Conversation

@moste00

@moste00 moste00 commented Jun 27, 2026

Copy link
Copy Markdown

The plugin had several memory leaks which prevented it from running a relatively large, realistic workload. For example, a single run of the TCC compiler binary on a int main() { return 0; } made QEMU freeze the machine (8 GB of memory and 4 GB of swap) after about 10 minutes of runtime.

In one successful run of a smaller binary, up to 800 MB of memory can be wasted in total by program exit, according to Valgrind's MemCheck.

3 types of problems are fixed in this PR:

1- Most straightforwardly, the QEMU API qemu_plugin_get_registers is documented as returning caller-owned GArray *, there were 3 calls to it in the plugin with no free on the happy path, and the pointer wasn't stored anywhere. This was changed to use g_autoptr macro that uses a destructor-like (or defer-like) cleanup function on any return path from the function. This is also how it's used in the execlog example plugin.

2- Similarly, in add_post_reg_state and add_pre_reg_state, a GByteArray was used as a scratch buffer to read register state, it's local to the function and never stored anywhere. Thus, also freed with g_autoptr.

3- More interestingly, inside log_insn_mem_access, the function add_mem_op had two leaks:

3-a) uint8_t * buffer is allocated via g_malloc and given to frame_buffer_append_mem_info which takes it by a const pointer, that buffer is ultimately copied inside the latter function by frame_init_mem_operand_info, and the original is left without freeing.

3-b) The OperandInfo struct allocated by frame_init_mem_operand_info is returned to frame_buffer_append_mem_info, which calls append_op_info, which can fail if the frame is uninitialized. On failure, the OperandInfo struct is never used or stored anywhere so it should be freed, but neither frame_buffer_append_mem_info nor add_mem_op free it on failure. Both return or log an error and leave the orphan memory.

This was fixed by:

  • frame_buffer_append_mem_info and frame_init_mem_operand_info were renamed to have the suffix take, meaning they take ownership of the buffer they're passed directly. There is no need to free the original buffer in the function since its shallow copied to the OperandInfo struct and freed when it's freed.

  • On failure, frame_buffer_append_mem_info_take frees the orphaned struct.

An inefficient alternative to frame_buffer_append_mem_info_take might have been to keep the copy semantics and free the temporary buffer in add_mem_op , but this is unnecessary anyway since it's never used elsewhere in the function, and frame_buffer_append_mem_info has no other callers that would need updating.

After those changes, the TCC run was successful, producing 25 GB output file. Only 2-3 MB of memory was leaked on the smaller run according to valgrind, most of the leaks either coming from QEMU proper (main.c, perhaps some commandline arguments or global lifetime data), and some plugin data is leaked by being still reachable at program exit, probably due to the plugin exit callback being commented out and not being called to flush the final frames.

@Rot127
Rot127 merged commit 272bb15 into BinaryAnalysisPlatform:trace-10.0 Jun 28, 2026
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants