Fix some memory leaks in the BAP plugin - #46
Merged
Rot127 merged 1 commit intoJun 28, 2026
Conversation
Rot127
approved these changes
Jun 27, 2026
Rot127
merged commit Jun 28, 2026
272bb15
into
BinaryAnalysisPlatform:trace-10.0
1 of 2 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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_registersis documented as returning caller-ownedGArray *, 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 useg_autoptrmacro 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 theexeclogexample plugin.2- Similarly, in
add_post_reg_stateandadd_pre_reg_state, aGByteArraywas used as a scratch buffer to read register state, it's local to the function and never stored anywhere. Thus, also freed withg_autoptr.3- More interestingly, inside
log_insn_mem_access, the functionadd_mem_ophad two leaks:3-a)
uint8_t *buffer is allocated viag_mallocand given toframe_buffer_append_mem_infowhich takes it by a const pointer, that buffer is ultimately copied inside the latter function byframe_init_mem_operand_info, and the original is left without freeing.3-b) The
OperandInfostruct allocated byframe_init_mem_operand_infois returned toframe_buffer_append_mem_info, which callsappend_op_info, which can fail if the frame is uninitialized. On failure, theOperandInfostruct is never used or stored anywhere so it should be freed, but neitherframe_buffer_append_mem_infonoradd_mem_opfree it on failure. Both return or log an error and leave the orphan memory.This was fixed by:
frame_buffer_append_mem_infoandframe_init_mem_operand_infowere renamed to have the suffixtake, 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 theOperandInfostruct and freed when it's freed.On failure,
frame_buffer_append_mem_info_takefrees the orphaned struct.An inefficient alternative to
frame_buffer_append_mem_info_takemight have been to keep the copy semantics and free the temporary buffer inadd_mem_op, but this is unnecessary anyway since it's never used elsewhere in the function, andframe_buffer_append_mem_infohas 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.