Skip to content

Conversation

@Grond66
Copy link

@Grond66 Grond66 commented Jan 10, 2026

Your checklist for this pull request

  • [*] I've documented or updated the documentation of every API function and struct this PR changes.
  • [*] I've added tests that prove my fix is effective or that my feature works (if possible)

Detailed description

Capstone's source has a few places where memory is allocated, but there are no checks for out-of-memory (OOM) conditions. This can cause problems in memory-constrained environments; instead of having an operation error out with CS_ERR_MEM, we get a null-pointer dereference. This is fairly simple to fix, but it does touch quite a few files. Please let me know if you want any improvements or further explanation of the fixes.

Test plan

The standard test suite should do fine.

Closing issues

None, but I can open a tracking issue if that's helpful.

Samuel Arnold added 2 commits January 5, 2026 13:44
@Grond66 Grond66 marked this pull request as ready for review January 10, 2026 01:20

if (*cache == NULL)
*cache = make_id2insn(insns, max);
if (!(*cache = make_id2insn(insns, max)))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are only two places where handle->insn_id is actually called. Wouldn't it be way simple to just initialize the cache in cs_disasm and cs_disasm_iter?
This would save us all the other changes in the other functions.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can move the checked allocation into a new routine which is common between cs_disasm and cs_disasm_iter...
That way the arch-specific ID-translation code can just assume that any needed cache has already been allocated.
But this does mean that we'll need to provide a way for the architecture-init code to signal how much cache it's associated translation table needs.
That will basically entail a new handle->required_insn_cache_size field (which will be zero for archetectures that don't need it).
Does that work for you?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this makes sense.
To give some context. The reason I'd like to have as few allocations in the modules as possible is for removing allocations completely in the (far) future.

This makes it potentially easier to refactor then, if we want to introduce a stack only disassembly logic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants