Fix unchecked allocations by Grond66 · Pull Request #2844 · capstone-engine/capstone

Grond66 · 2026-01-10T01:18:21Z

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.

Rot127 · 2026-01-11T00:37:26Z


 	if (*cache == NULL)
-		*cache = make_id2insn(insns, max);
+		if (!(*cache = make_id2insn(insns, max)))


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.

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?

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.

OK, bc5f6b2 should address this.
Lemmie know if that's sufficient.

Rot127 · 2026-01-28T20:50:54Z

Looked briefly and seems good. But can you please fix the build?

Grond66 · 2026-01-30T02:45:48Z

A missing semicolon?
Well, that's embarrassing.
I'm not sure how that happened, but after updating make clean && make runs fine on my end.
Let me know if any further changes are needed.

Rot127 · 2026-01-30T16:34:39Z

No problem

I'm not sure how that happened, but after updating make clean && make runs fine on my end.

Makefile is deprecated though (we still run and fix the build, but don't extend it for developer tasks). I'd recommend to switch to cmake.

Rot127 · 2026-02-01T11:57:45Z

@Grond66 There are memory issues. Please rebase though before. I just merged a big RISC-V update.

[0178](https://github.com/capstone-engine/capstone/actions/runs/21502490444/job/62019210331?pr=2844#step:12:40179)
[       OK ] /extr_u.s.yaml - TC #1: TestInput { name: (null), arch: CS_ARCH_TRICORE, options: [CS_MODE_TRICORE_131], addr: 0x0, bytes: 0x17, 0x01, 0x60, 0x02 }
[ RUN      ] /insn-riscv32.s.yaml - TC #0: TestInput { name: (null), arch: CS_ARCH_RISCV, options: [CS_MODE_RISCV32], addr: 0x0, bytes: 0x37, 0x34, 0x00, 0x00 }
=================================================================
==6045==ERROR: AddressSanitizer: global-buffer-overflow on address 0x5608fba12c44 at pc 0x5608fa3115f1 bp 0x7ffeff267bf0 sp 0x7ffeff267be0
READ of size 2 at 0x5608fba12c44 thread T0
    #0 0x5608fa3115f0 in populate_insn_map_cache /home/runner/work/capstone/capstone/Mapping.c:37
    #1 0x5608fa30069c in cs_open /home/runner/work/capstone/capstone/cs.c:814
    #2 0x5608fa2fc605 in open_cs_handle /home/runner/work/capstone/capstone/suite/cstest/src/test_run.c:160
    #3 0x5608fa2fd063 in cstest_unit_test_setup /home/runner/work/capstone/capstone/suite/cstest/src/test_run.c:204
    #4 0x5608fa7aa51b in cmocka_run_one_test_or_fixture /home/runner/work/capstone/capstone/build/suite/cstest/extern/src/cmocka_ext/src/cmocka.c:2914
    #5 0x5608fa7aa735 in cmocka_run_one_tests /home/runner/work/capstone/capstone/build/suite/cstest/extern/src/cmocka_ext/src/cmocka.c:2999
    #6 0x5608fa7aad3c in _cmocka_run_group_tests /home/runner/work/capstone/capstone/build/suite/cstest/extern/src/cmocka_ext/src/cmocka.c:3148
    #7 0x5608fa2fef46 in eval_test_cases /home/runner/work/capstone/capstone/suite/cstest/src/test_run.c:295
    #8 0x5608fa2ff8fc in cstest_run_tests /home/runner/work/capstone/capstone/suite/cstest/src/test_run.c:323
    #9 0x5608fa25ff2d in main /home/runner/work/capstone/capstone/suite/cstest/src/cstest.c:110
    #10 0x7f59c962a1c9  (/lib/x86_64-linux-gnu/libc.so.6+0x2a1c9) (BuildId: 274eec488d230825a136fa9c4d85370fed7a0a5e)
    #11 0x7f59c962a28a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2a28a) (BuildId: 274eec488d230825a136fa9c4d85370fed7a0a5e)
    #12 0x5608fa25ece4 in _start (/home/runner/work/capstone/capstone/build/suite/cstest/cstest+0xd2bce4) (BuildId: d0d9670b95f2bf27cbc65e8d99407cc8b66b1aad)

0x5608fba12c44 is located 0 bytes after global variable 'insns' defined in '/home/runner/work/capstone/capstone/arch/RISCV/RISCVMapping.c:84:23' (0x5608fba07d60) of size 44772
0x5608fba12c44 is located 60 bytes before global variable 'RISCV_insn_count' defined in '/home/runner/work/capstone/capstone/arch/RISCV/RISCVMapping.c:101:20' (0x5608fba12c80) of size 4
SUMMARY: AddressSanitizer: global-buffer-overflow /home/runner/work/capstone/capstone/Mapping.c:37 in populate_insn_map_cache
Shadow bytes around the buggy address:
  0x5608fba12980: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x5608fba12a00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x5608fba12a80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x5608fba12b00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x5608fba12b80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x5608fba12c00: 00 00 00 00 00 00 00 00[04]f9 f9 f9 f9 f9 f9 f9
  0x5608fba12c80: 04 f9 f9 f9 f9 f9 f9 f9 00 00 00 00 00 00 00 00
  0x5608fba12d00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x5608fba12d80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x5608fba12e00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x5608fba12e80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==6045==ABORTING


0% tests passed, 1 tests failed out of 1

Total Test time (real) =   7.56 sec

The following tests FAILED:
	  1 - MCTests (Subprocess aborted)

Grond66 · 2026-02-05T02:51:29Z

OK, looks like the memory safety issue is now fixed.
It was a fairly banal bug where the wrong limit variable was used when populating the instruction cache.
I think all tests should pass now, but I wasn't fully able to replicate the testing environment locally.
Let me know if anything else is needed @Rot127.

Rot127 · 2026-02-05T14:33:26Z

I think all tests should pass now, but I wasn't fully able to replicate the testing environment locally.

Can you tell me what the issue was? Did the build fail, or were docs missing?

Rot127

lgtm!
Thanks a lot for refactoring it.

Just have those nitpicks.
After those it is good to merge.

Rot127 · 2026-02-13T17:22:34Z

@Grond66 I want to make a release over the next week. Having your addition in there would be really nice. Do you think you will find time to finish it? It is not much left.

Grond66 · 2026-02-13T23:51:32Z

I think all tests should pass now, but I wasn't fully able to replicate the testing environment locally.

Can you tell me what the issue was? Did the build fail, or were docs missing?

No, one of the python tests failed. I think it was because it involved doing sudo cmake install, or something like that.

Co-authored-by: Rot127 <45763064+Rot127@users.noreply.github.com>

Grond66 · 2026-02-14T00:17:18Z

@Grond66 I want to make a release over the next week. Having your addition in there would be really nice. Do you think you will find time to finish it? It is not much left.

@Rot127
Yeah, everything you've asked for should now be done.
Let me know if it's not.

Grond66 marked this pull request as ready for review January 10, 2026 01:20

Rot127 requested changes Jan 11, 2026

View reviewed changes

Grond66 force-pushed the fix-unchecked-allocations branch from 6fa69eb to bc5f6b2 Compare January 26, 2026 23:59

github-actions Bot removed BPF Arch EVM Arch M680X Arch M68K Arch labels Jan 26, 2026

github-actions Bot removed SH Arch WASM Arch HPPA Arch labels Jan 26, 2026

Grond66 requested a review from Rot127 January 27, 2026 22:27

Grond66 force-pushed the fix-unchecked-allocations branch from bc5f6b2 to 2a419b6 Compare January 30, 2026 02:42

Samuel Arnold added 2 commits February 4, 2026 18:40

Fix trivial unchecked allocations

04c97c2

Move instruction lookup caching allocation into the core

b2ab68d

Grond66 force-pushed the fix-unchecked-allocations branch from 2a419b6 to b2ab68d Compare February 5, 2026 02:48

Rot127 requested changes Feb 5, 2026

View reviewed changes

Comment thread cs.c Outdated

Comment thread cs.c

Comment thread Mapping.c

Comment thread cs_priv.h Outdated

Comment thread Mapping.h

Comment thread Mapping.c Outdated

Comment thread cs_priv.h Outdated

Comment thread Mapping.h Outdated

Grond66 and others added 4 commits February 13, 2026 15:57

Apply formatting nitpicks

ef89bd2

Co-authored-by: Rot127 <45763064+Rot127@users.noreply.github.com>

Assert to avoid an integer overflow in lookup_insn_map()

82d0415

Remove unused member from cs_struct

4a85bd2

Document what happens when lookup_insn_map() fails

69c1609

Rot127 approved these changes Feb 14, 2026

View reviewed changes

Rot127 merged commit a739923 into capstone-engine:next Feb 14, 2026
29 checks passed

Conversation

Grond66 commented Jan 10, 2026

Uh oh!

Rot127 Jan 11, 2026

Choose a reason for hiding this comment

Uh oh!

Grond66 Jan 13, 2026

Choose a reason for hiding this comment

Uh oh!

Rot127 Jan 13, 2026

Choose a reason for hiding this comment

Uh oh!

Grond66 Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Rot127 commented Jan 28, 2026

Uh oh!

Grond66 commented Jan 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Rot127 commented Jan 30, 2026

Uh oh!

Rot127 commented Feb 1, 2026

Uh oh!

Grond66 commented Feb 5, 2026

Uh oh!

Rot127 commented Feb 5, 2026

Uh oh!

Rot127 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Rot127 commented Feb 13, 2026

Uh oh!

Grond66 commented Feb 13, 2026

Uh oh!

Grond66 commented Feb 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Grond66 commented Jan 30, 2026 •

edited

Loading

Grond66 commented Feb 14, 2026 •

edited

Loading