Fix leaks of RzILMem buffers and clarify ownership of buffer. #5739
Conversation
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
Rot127
requested review from
kazarmy,
ret2libc,
thestr4ng3r and
wargio
as code owners
notxvilka
reviewed
Jan 7, 2026
Contributor
notxvilka
left a comment
notxvilka
left a comment
There was a problem hiding this comment.
LGTM with one unclear thing
…rship. The previous assumption of RzILMem was that the buffer is not owned by it. This made sense, because in the beginning it only held an RzIO buffer. But the Xtensa and Sparc RzAnalysisILInitCallback definition initializes an additional RzILMem object. This additional RzILMem has a sparse buffer in both cases. The Xtensa and Sparc plugin pass the sparse buffer to RzILMem with the assumption that it takes ownership. They have to, because there is no fini() version of RzAnalysisILInitCallback in which they could free the buffer. There are also no API functions to free the buffer in RzILMem. So the plugins don't have a nice way to clean it up in a theoretical fini() callback. This commit fixes the leak by splitting rz_il_mem_new() into two version. One to taking the ownership of the buffer, one borrowing it. This seems the most natural solution, because buffers can abstract from all kind of backends (RzIO, a file, some memory etc.). Each time the ownership of the buffer is different. So the creator of rz_il_mem_new_*() can decide what buffer with what ownership they pass.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Your checklist for this pull request
RZ_APIfunction and struct this PR changes.RZ_API).Detailed description
The previous assumption of
RzILMemwas that the buffer is not owned by it. This made sense, because in the beginning it only held anRzIObuffer.But the Xtensa and Sparc
RzAnalysisILInitCallbackdefinition initializes an additionalRzILMemobject. This additionalRzILMemhas a sparse buffer in both cases. The Xtensa and Sparc plugin pass the sparse buffer toRzILMemwith the assumption that it is owned. They have to, because there is nofini()version ofRzAnalysisILInitCallbackin which they could free the buffer. There are also no API functions to free the buffer inRzILMem. So the plugins don't have a nice way to clean it up in a theoreticalfini()callback.This commit fixes the leak by adding a flag to
rz_il_mem_new()to mark the ownership of the given buffer.This seems the most natural solution, because buffers can abstract from all kind of backends (
RzIO, a file, some memory etc.). Each time the ownership of the buffer is different. So the creator ofrz_il_mem_new()can decide what buffer with what ownership they pass.Test plan
All green no crashes.
Closing issues
...