Fix memory leaks across core, bin, and PDB parsing (#6579)

Co-authored-by: Anton Kochkov <anton.kochkov@gmail.com>

librz/arch/cc.c

@@ -271,7 +271,7 @@ static bool filter_cc(void *user, const SdbKv *kv) {
 }
 
 RZ_API RzList /*<char *>*/ *rz_analysis_calling_conventions(RzAnalysis *analysis) {
-	RzList *ccl = rz_list_new();
+	RzList *ccl = rz_list_newf(free);
 	void **iter;
 	RzPVector *items = sdb_get_items_filter(analysis->sdb_cc, filter_cc, NULL, true);
 	rz_pvector_foreach (items, iter) {

librz/arch/types.c

@@ -3,6 +3,16 @@
 
 #include "analysis_private.h"
 
+static void type_path_tuple_free(void *e) {
+	RzTypePathTuple *tpl = (RzTypePathTuple *)e;
+	if (!tpl) {
+		return;
+	}
+	rz_type_path_free(tpl->path);
+	rz_type_free(tpl->root);
+	free(tpl);
+}
+
 static RZ_OWN RzList /*<RzTypePathTuple *>*/ *var_global_type_paths(RzAnalysis *analysis, RzAnalysisVarGlobal *gv, ut64 addr, unsigned int depth) {
 	rz_return_val_if_fail(gv, false);
 
@@ -20,10 +30,12 @@ static RZ_OWN RzList /*<RzTypePathTuple *>*/ *var_global_type_paths(RzAnalysis *
 	if (!tlist) {
 		return NULL;
 	}
-	RzListIter *iter;
 	RzTypePath *path;
-	RzList *matches = rz_list_new();
-	rz_list_foreach (tlist, iter, path) {
+	RzList *matches = rz_list_newf(type_path_tuple_free);
+	// Take ownership of each path out of `tlist` as it is processed: items are
+	// either freed here or moved into a tuple in `matches`. `tlist` is left
+	// empty, so it can be released normally without touching its elements.
+	while ((path = rz_list_pop_head(tlist))) {
 		if (!path->path) {
 			rz_type_path_free(path);
 			continue;
@@ -44,6 +56,7 @@ static RZ_OWN RzList /*<RzTypePathTuple *>*/ *var_global_type_paths(RzAnalysis *
 		tpl->root = rz_type_clone(gv->type);
 		rz_list_append(matches, tpl);
 	}
+	rz_list_free(tlist);
 	return matches;
 }
 

librz/bin/bin.c

@@ -252,6 +252,15 @@ RZ_API RzBinFile *rz_bin_reload(RzBin *bin, RzBinFile *bf, ut64 baseaddr) {
 	opt.filename = bf->file;
 	rz_buf_seek(bf->buf, 0, RZ_BUF_SET);
 	RzBinFile *nbf = rz_bin_open_buf(bin, bf->buf, &opt);
+	// On reload the new file reuses the same fd, so opening it overwrites the
+	// old file's "cur" and "fd.<fd>" entries in bin->sdb. That releases fewer
+	// references to the old sdb than the regular teardown path does, leaving
+	// the extra reference rz_bin_object_new took on bf->sdb dangling, so the
+	// old sdb would leak once the old file is deleted below. Drop it here:
+	// sdb_free is reference-counted, so this only decrements the counter and
+	// the sdb is actually released when rz_bin_file_delete() drops the last
+	// reference.
+	sdb_free(bf->sdb);
 	rz_bin_file_delete(bin, bf);
 	return nbf;
 }

librz/bin/pdb/modi.c

@@ -45,8 +45,18 @@ RZ_IPI bool PDBModuleInfo_parse(const RzPdb *pdb, const PDB_DBIModule *m, PDBMod
 	if (!PDBModuleInfo_symbols(modi, &iter)) {
 		return false;
 	}
-	if (!PDBSymbolIter_collect(&iter, &modi->symbols)) {
-		return false;
-	}
-	return true;
+	bool collected = PDBSymbolIter_collect(&iter, &modi->symbols);
+	rz_buf_free(iter.b);
+	return collected;
+}
+
+RZ_IPI void PDBModuleInfo_free(void *x) {
+	PDBModuleInfo *modi = (PDBModuleInfo *)x;
+	if (!modi) {
+		return;
+	}
+	// modi->stream is borrowed from the module's raw MSF stream, so only the
+	// collected symbols and the module info itself are owned here.
+	rz_pvector_free(modi->symbols);
+	free(modi);
 }

librz/bin/pdb/modi.h

@@ -15,5 +15,6 @@
 #define CV_SIGNATURE_RESERVED 5L // All signatures from 5 to 64K are reserved
 
 RZ_IPI bool PDBModuleInfo_parse(const RzPdb *pdb, const PDB_DBIModule *m, PDBModuleInfo *modi);
+RZ_IPI void PDBModuleInfo_free(void *x);
 
 #endif // MODI_H

librz/bin/pdb/pdb.c

@@ -58,7 +58,7 @@ static bool parse_streams(RzPdb *pdb) {
 		return false;
 	}
 	if (pdb->s_dbi->modules) {
-		pdb->module_infos = rz_pvector_new(NULL);
+		pdb->module_infos = rz_pvector_new(PDBModuleInfo_free);
 		void **modit;
 		rz_pvector_foreach (pdb->s_dbi->modules, modit) {
 			const PDB_DBIModule *m = *modit;
@@ -67,7 +67,7 @@ static bool parse_streams(RzPdb *pdb) {
 				return false;
 			}
 			if (!PDBModuleInfo_parse(pdb, m, modi)) {
-				free(modi);
+				PDBModuleInfo_free(modi);
 				return false;
 			}
 			rz_pvector_push(pdb->module_infos, modi);

librz/bin/pdb/symbol.c

@@ -224,6 +224,7 @@ RZ_IPI bool PDBSymbolIter_collect(PDBSymbolIter *iter, RzPVector /*<PDBSymbol *>
 			return false;
 		}
 		if (!PDBSymbolIter_next(iter, symbol)) {
+			free(symbol);
 			break;
 		}
 		rz_pvector_push(symbols, symbol);

librz/core/canalysis.c

@@ -1882,13 +1882,7 @@ RZ_API int rz_core_analysis_search(RzCore *core, ut64 from, ut64 to, ut64 ref, i
 						count++;
 					}
 					break;
-				default: {
-					rz_analysis_op_init(&op);
-					if (rz_analysis_op(core->analysis, &op, at + i, buf + i, core->blocksize - i, RZ_ANALYSIS_OP_MASK_BASIC) < 1) {
-						rz_analysis_op_fini(&op);
-						continue;
-					}
-				}
+				default:
 					if (op.ptr != UT64_MAX &&
 						core_analysis_followptr(core, RZ_ANALYSIS_XREF_TYPE_DATA, at + i, op.ptr, ref, false, ptrdepth)) {
 						count++;

librz/core/cio.c

@@ -29,7 +29,7 @@ RZ_API int rz_core_setup_debugger(RzCore *r, const char *debugbackend, bool atta
 		rz_debug_select(r->dbg, r->dbg->pid, r->dbg->tid);
 	}
 	rz_config_set_bool(r->config, "dbg.swstep", (r->dbg->cur && !r->dbg->cur->canstep));
-	rz_io_system(r->io, rz_strf(buf, "pid %d", r->dbg->pid));
+	free(rz_io_system(r->io, rz_strf(buf, "pid %d", r->dbg->pid)));
 
 	// this makes to attach twice showing warnings in the output
 	// we get "resource busy" so it seems isn't an issue
@@ -1047,6 +1047,7 @@ RZ_API bool rz_core_write_block_op_at(RzCore *core, ut64 addr, RzCoreWriteOp op,
 		return false;
 	}
 
+	free(buf);
 	return true;
 }
 

librz/core/cmd/cmd_print.c

@@ -6731,6 +6731,7 @@ static RzCmdStatus print_8bit_hexpair(RzCore *core, ut64 addr, size_t len) {
 	}
 	rz_io_read_at_mapped(core->io, addr, buf, len);
 	rz_print_bytes(core->print, buf, len, "%02x");
+	free(buf);
 	return RZ_CMD_STATUS_OK;
 }
 

librz/core/cpdb.c

@@ -238,6 +238,9 @@ static bool symbol_load(RzPdb *pdb, const PDBSymbol *symbol, void *u) {
 
 		ut64 addr = rz_bin_pdb_to_rva(pdb, &public->offset);
 		if (addr == UT64_MAX) {
+			free(filtered_name);
+			free(fname);
+			free(name);
 			return true;
 		}
 		if (ctx->baddr != UT64_MAX) {

librz/core/ctypes.c

@@ -978,6 +978,7 @@ RZ_IPI void rz_types_define(RzCore *core, const char *type) {
 		RZ_LOG_ERROR("core: %s\n", error_msg);
 		free(error_msg);
 	}
+	free(tmp);
 }
 
 RZ_IPI bool rz_types_open_file(RzCore *core, const char *path) {

librz/core/devirtualize_cxx.c

@@ -807,33 +807,32 @@ static void devirtualize_variable_vtable(RzCore *core, RzCppVariableBook *var_bo
 	rz_analysis_op_free(op);
 }
 
+typedef struct {
+	RzStrBuf *comment;
+	bool first;
+} VirtualCallCommentCtx;
+
+static bool build_virtual_call_comment(void *user, const char *vfunc_name, RZ_UNUSED const void *v) {
+	VirtualCallCommentCtx *ctx = (VirtualCallCommentCtx *)user;
+	if (ctx->first) {
+		rz_strbuf_setf(ctx->comment, "Virtual Call : %s", vfunc_name);
+		ctx->first = false;
+	} else {
+		rz_strbuf_appendf(ctx->comment, " / %s", vfunc_name);
+	}
+	return true;
+}
+
 static bool add_comment(void *user, const ut64 key, const void *v) {
 	RzCore *core = (RzCore *)user;
 	RzSetS *set = (RzSetS *)v;
 
-	RzPVector *vect = rz_set_s_to_vector(set);
-	void **it;
-
 	RzStrBuf *comment = rz_strbuf_new(NULL);
-	bool first = true;
-	rz_pvector_foreach (vect, it) {
-		const char *vfunc_name = *it;
-		if (first) {
-			rz_strbuf_setf(comment, "Virtual Call : %s", vfunc_name);
-			first = false;
-			continue;
-		}
-		rz_strbuf_appendf(comment, " / %s", vfunc_name);
-	}
-	const char *str_comment = rz_strbuf_drain(comment);
+	VirtualCallCommentCtx ctx = { comment, true };
+	ht_sp_foreach((HtSP *)set, build_virtual_call_comment, &ctx);
+	char *str_comment = rz_strbuf_drain(comment);
 	rz_core_meta_comment_add(core, str_comment, key);
-
-	rz_pvector_fini(vect);
-
-	rz_pvector_foreach (vect, it) {
-		free(*it);
-	}
-	rz_pvector_free(vect);
+	free(str_comment);
 
 	return true;
 }
@@ -844,20 +843,27 @@ static bool free_virt_calls(void *user, const ut64 key, const void *v) {
 	return true;
 }
 
-static bool add_virtual_xref(RzAnalysis *analysis, const ut64 key, RzSetS *vfunc_set) {
-	HtSP *virtual_xref = rz_analysis_get_virtual_xrefs(analysis);
-	RzPVector *pvect = rz_set_s_to_vector(vfunc_set);
-	void **it;
-	rz_pvector_foreach (pvect, it) {
-		const char *vfunc = *it;
+typedef struct {
+	HtSP *virtual_xref;
+	ut64 key;
+} VirtualXrefCtx;
+
+static bool collect_virtual_xref(void *user, const char *vfunc, RZ_UNUSED const void *v) {
+	VirtualXrefCtx *ctx = (VirtualXrefCtx *)user;
 	bool found = false;
-		RzSetU *set = ht_sp_find(virtual_xref, vfunc, &found);
+	RzSetU *set = ht_sp_find(ctx->virtual_xref, vfunc, &found);
 	if (!found) {
 		set = rz_set_u_new();
-			ht_sp_insert(virtual_xref, vfunc, set);
-		}
-		rz_set_u_add(set, key);
+		ht_sp_insert(ctx->virtual_xref, vfunc, set);
 	}
+	rz_set_u_add(set, ctx->key);
+	return true;
+}
+
+static bool add_virtual_xref(RzAnalysis *analysis, const ut64 key, RzSetS *vfunc_set) {
+	HtSP *virtual_xref = rz_analysis_get_virtual_xrefs(analysis);
+	VirtualXrefCtx ctx = { virtual_xref, key };
+	ht_sp_foreach((HtSP *)vfunc_set, collect_virtual_xref, &ctx);
 	return true;
 }
 

librz/debug/debug.c

@@ -625,7 +625,7 @@ RZ_API bool rz_debug_select(RzDebug *dbg, int pid, int tid) {
 		dbg->tid = tid;
 	}
 
-	rz_io_system(dbg->iob.io, rz_strf(tmpbuf, "pid %d", dbg->tid));
+	free(rz_io_system(dbg->iob.io, rz_strf(tmpbuf, "pid %d", dbg->tid)));
 
 	// Synchronize with the current thread's data
 	if (dbg->corebind.core) {

librz/flag/flag.c

@@ -337,6 +337,7 @@ RZ_API bool rz_flag_reset_obj_flags(RZ_NONNULL RZ_BORROW RzFlag *flags, RZ_NULLA
 		}
 		rz_serialize_flag_save(sdb, flags);
 		backup_succeeded = sdb_text_save(sdb, backup_filename, false);
+		sdb_free(sdb);
 	}
 	if (!backup_succeeded) {
 		RZ_LOG_WARN("Could not backup RzFlag before resetting flag space. Abort flag space reset.\n");