Fix unchecked allocations (#2844)

* Fix trivial unchecked allocations

* Move instruction lookup caching allocation into the core

* Apply formatting nitpicks

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

* Assert to avoid an integer overflow in lookup_insn_map()

* Remove unused member from cs_struct

* Document what happens when lookup_insn_map() fails

---------

Co-authored-by: Samuel Arnold <samuel.arnold@crowdstrike.com>
Co-authored-by: Rot127 <45763064+Rot127@users.noreply.github.com>

Author: 3 people

Mapping.c

@@ -7,34 +7,53 @@
 #include "cs_priv.h"
 #include "utils.h"
 
-// create a cache for fast id lookup
-static unsigned short *make_id2insn(const insn_map *insns, unsigned int size)
+// Create a cache to map LLVM instruction IDs to capstone instruction IDs, if
+// the architecture needs this.
+cs_err populate_insn_map_cache(cs_struct *handle)
 {
-	// NOTE: assume that the max id is always put at the end of insns array
-	unsigned short max_id = insns[size - 1].id;
 	unsigned int i;
 
-	unsigned short *cache =
-		(unsigned short *)cs_mem_calloc(max_id + 1, sizeof(*cache));
+	// If this architecture doesn't use instruction mapping, do nothing
+	if (!handle->insn_map || handle->insn_map_size <= 0)
+		return CS_ERR_OK;
 
-	for (i = 1; i < size; i++)
-		cache[insns[i].id] = i;
+	// Since the instruction map is assumed to be stored in ascending
+	// order, we can get the maximum LLVM instruction id just by looking at
+	// the last element.
+	unsigned int cache_elements =
+		handle->insn_map[handle->insn_map_size - 1].id + 1;
 
-	return cache;
+	// This should not be initialized yet.
+	CS_ASSERT(!handle->insn_cache);
+
+	unsigned short *cache = cs_mem_calloc(cache_elements, sizeof(*cache));
+	if (!cache) {
+		handle->errnum = CS_ERR_MEM;
+		return CS_ERR_MEM;
+	}
+	handle->insn_cache = cache;
+
+	for (i = 1; i < handle->insn_map_size; ++i)
+		handle->insn_cache[handle->insn_map[i].id] = i;
+
+	return CS_ERR_OK;
 }
 
-// look for @id in @insns, given its size in @max. first time call will update
-// @cache. return 0 if not found
-unsigned short insn_find(const insn_map *insns, unsigned int max,
-			 unsigned int id, unsigned short **cache)
+const insn_map *lookup_insn_map(cs_struct *handle, unsigned short id)
 {
-	if (id > insns[max - 1].id)
-		return 0;
+	// If this is getting called, we need the cache to already be populated
+	// (this should be done when populate_insn_map_cache() gets called).
+	CS_ASSERT(handle->insn_cache);
+	CS_ASSERT(handle->insn_map_size);
+
+	unsigned short highest_id =
+		handle->insn_map[handle->insn_map_size - 1].id;
+	if (id > highest_id)
+		return NULL;
 
-	if (*cache == NULL)
-		*cache = make_id2insn(insns, max);
+	unsigned short i = handle->insn_cache[id];
 
-	return (*cache)[id];
+	return &handle->insn_map[i];
 }
 
 // Gives the id for the given @name if it is saved in @map.

Mapping.h

@@ -15,35 +15,16 @@
 #include <assert.h>
 #include <string.h>
 
-// map instruction to its characteristics
-typedef struct insn_map {
-	unsigned short id; // The LLVM instruction id
-	unsigned short mapid; // The Capstone instruction id
-#ifndef CAPSTONE_DIET
-	uint16_t regs_use[MAX_IMPL_R_REGS]; ///< list of implicit registers used by
-	///< this instruction
-	uint16_t regs_mod[MAX_IMPL_W_REGS]; ///< list of implicit registers modified
-	///< by this instruction
-	unsigned char groups
-		[MAX_NUM_GROUPS]; ///< list of group this instruction belong to
-	bool branch; // branch instruction?
-	bool indirect_branch; // indirect branch instruction?
-	union {
-		ppc_suppl_info ppc;
-		loongarch_suppl_info loongarch;
-		aarch64_suppl_info aarch64;
-		systemz_suppl_info systemz;
-		arm_suppl_info arm;
-		xtensa_suppl_info xtensa;
-		sparc_suppl_info sparc;
-	} suppl_info; // Supplementary information for each instruction.
-#endif
-} insn_map;
-
-// look for @id in @m, given its size in @max. first time call will update
-// @cache. return 0 if not found
-unsigned short insn_find(const insn_map *m, unsigned int max, unsigned int id,
-			 unsigned short **cache);
+// Create a cache to map LLVM instruction IDs to capstone instruction IDs, if
+// the architecture needs this.
+cs_err populate_insn_map_cache(cs_struct *handle);
+
+// Lookup the insn_map instance for an LLVM instruction ID. This method assumes
+// that this architecture uses the instruction mapping functionality available
+// from populate_insn_map_cache().
+// Returns NULL in case id is larger than the maximum mapped LLVM instruction
+// ID.
+const insn_map *lookup_insn_map(cs_struct *handle, unsigned short id);
 
 unsigned int find_cs_id(unsigned MC_Opcode, const insn_map *imap,
 			unsigned imap_size);

arch/AArch64/AArch64Module.c

@@ -13,6 +13,8 @@ cs_err AArch64_global_init(cs_struct *ud)
 {
 	MCRegisterInfo *mri;
 	mri = cs_mem_malloc(sizeof(*mri));
+	if (!mri)
+		return CS_ERR_MEM;
 
 	AArch64_init_mri(mri);
 	ud->printer = AArch64_printer;

arch/ARC/ARCModule.c

@@ -14,6 +14,8 @@ cs_err ARC_global_init(cs_struct *ud)
 {
 	MCRegisterInfo *mri;
 	mri = cs_mem_malloc(sizeof(*mri));
+	if (!mri)
+		return CS_ERR_MEM;
 
 	ARC_init_mri(mri);
 
@@ -49,4 +51,4 @@ cs_err ARC_option(cs_struct *handle, cs_opt_type type, size_t value)
 	return CS_ERR_OK;
 }
 
-#endif
+#endif

arch/ARM/ARMModule.c

@@ -14,6 +14,8 @@ cs_err ARM_global_init(cs_struct *ud)
 {
 	MCRegisterInfo *mri;
 	mri = cs_mem_malloc(sizeof(*mri));
+	if (!mri)
+		return CS_ERR_MEM;
 
 	ARM_init_mri(mri);
 

arch/Alpha/AlphaMapping.c

@@ -23,6 +23,9 @@ static const insn_map insns[] = {
 #include "AlphaGenCSMappingInsn.inc"
 };
 
+const insn_map *Alpha_insns = insns;
+const unsigned int Alpha_insn_count = ARR_SIZE(insns);
+
 static const map_insn_ops insn_operands[] = {
 #include "AlphaGenCSMappingInsnOp.inc"
 };
@@ -81,30 +84,28 @@ void Alpha_set_detail_op_reg(MCInst *MI, unsigned OpNum, alpha_op_type Reg)
 // given internal insn id, return public instruction info
 void Alpha_get_insn_id(cs_struct *h, cs_insn *insn, unsigned int id)
 {
-	unsigned short i;
+	insn_map const *insn_map = NULL;
 
-	i = insn_find(insns, ARR_SIZE(insns), id, &h->insn_cache);
-	if (i == 0) {
+	if (!(insn_map = lookup_insn_map(h, id)))
 		return;
-	}
-	insn->id = insns[i].mapid;
+	insn->id = insn_map->mapid;
 
 	if (insn->detail) {
 #ifndef CAPSTONE_DIET
-		memcpy(insn->detail->regs_read, insns[i].regs_use,
-		       sizeof(insns[i].regs_use));
+		memcpy(insn->detail->regs_read, insn_map->regs_use,
+		       sizeof(insn_map->regs_use));
 		insn->detail->regs_read_count =
-			(uint8_t)count_positive(insns[i].regs_use);
+			(uint8_t)count_positive(insn_map->regs_use);
 
-		memcpy(insn->detail->regs_write, insns[i].regs_mod,
-		       sizeof(insns[i].regs_mod));
+		memcpy(insn->detail->regs_write, insn_map->regs_mod,
+		       sizeof(insn_map->regs_mod));
 		insn->detail->regs_write_count =
-			(uint8_t)count_positive(insns[i].regs_mod);
+			(uint8_t)count_positive(insn_map->regs_mod);
 
-		memcpy(insn->detail->groups, insns[i].groups,
-		       sizeof(insns[i].groups));
+		memcpy(insn->detail->groups, insn_map->groups,
+		       sizeof(insn_map->groups));
 		insn->detail->groups_count =
-			(uint8_t)count_positive8(insns[i].groups);
+			(uint8_t)count_positive8(insn_map->groups);
 #endif
 	}
 }

arch/Alpha/AlphaMapping.h

@@ -11,6 +11,9 @@
 
 // unsigned int Alpha_map_insn_id(cs_struct *h, unsigned int id);
 
+extern const insn_map *Alpha_insns;
+extern const unsigned int Alpha_insn_count;
+
 // given internal insn id, return public instruction info
 void Alpha_get_insn_id(cs_struct *h, cs_insn *insn, unsigned int id);
 

arch/Alpha/AlphaModule.c

@@ -14,6 +14,8 @@ cs_err ALPHA_global_init(cs_struct *ud)
 	MCRegisterInfo *mri;
 
 	mri = cs_mem_malloc(sizeof(*mri));
+	if (!mri)
+		return CS_ERR_MEM;
 
 	Alpha_init(mri);
 	ud->printer = Alpha_printInst;
@@ -29,6 +31,8 @@ cs_err ALPHA_global_init(cs_struct *ud)
 #ifndef CAPSTONE_DIET
 	ud->reg_access = Alpha_reg_access;
 #endif
+	ud->insn_map = Alpha_insns;
+	ud->insn_map_size = Alpha_insn_count;
 
 	return CS_ERR_OK;
 }

arch/LoongArch/LoongArchModule.c

@@ -15,6 +15,8 @@ cs_err LoongArch_global_init(cs_struct *ud)
 {
 	MCRegisterInfo *mri;
 	mri = cs_mem_malloc(sizeof(*mri));
+	if (!mri)
+		return CS_ERR_MEM;
 
 	LoongArch_init_mri(mri);
 

arch/MOS65XX/MOS65XXModule.c

@@ -14,6 +14,9 @@ cs_err MOS65XX_global_init(cs_struct *ud)
 	mos65xx_info *info;
 
 	info = cs_mem_malloc(sizeof(*info));
+	if (!info)
+		return CS_ERR_MEM;
+
 	info->hex_prefix = NULL;
 	info->cpu_type = MOS65XX_CPU_TYPE_6502;
 	info->long_m = 0;