From 851ab98455599b713658ee92c7b672a2b1bcae52 Mon Sep 17 00:00:00 2001 From: NOT XVilka Date: Tue, 30 Jun 2026 19:04:28 +0800 Subject: [PATCH] Fix memory leaks across core, bin, and PDB parsing (#6579) Co-authored-by: Anton Kochkov --- librz/arch/cc.c | 2 +- librz/arch/types.c | 19 +++++++-- librz/bin/bin.c | 9 +++++ librz/bin/pdb/modi.c | 18 +++++++-- librz/bin/pdb/modi.h | 1 + librz/bin/pdb/pdb.c | 4 +- librz/bin/pdb/symbol.c | 1 + librz/core/canalysis.c | 8 +--- librz/core/cio.c | 3 +- librz/core/cmd/cmd_print.c | 1 + librz/core/cpdb.c | 3 ++ librz/core/ctypes.c | 1 + librz/core/devirtualize_cxx.c | 72 +++++++++++++++++++---------------- librz/debug/debug.c | 2 +- librz/flag/flag.c | 1 + 15 files changed, 93 insertions(+), 52 deletions(-) diff --git a/librz/arch/cc.c b/librz/arch/cc.c index 88a9e81522..cda4cdde22 100644 --- a/librz/arch/cc.c +++ b/librz/arch/cc.c @@ -271,7 +271,7 @@ static bool filter_cc(void *user, const SdbKv *kv) { } RZ_API RzList /**/ *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) { diff --git a/librz/arch/types.c b/librz/arch/types.c index c9ccf9ebe5..181048a143 100644 --- a/librz/arch/types.c +++ b/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 /**/ *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 /**/ *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 /**/ *var_global_type_paths(RzAnalysis * tpl->root = rz_type_clone(gv->type); rz_list_append(matches, tpl); } + rz_list_free(tlist); return matches; } diff --git a/librz/bin/bin.c b/librz/bin/bin.c index 6f7737288d..c56093e9b9 100644 --- a/librz/bin/bin.c +++ b/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." 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; } diff --git a/librz/bin/pdb/modi.c b/librz/bin/pdb/modi.c index d87dc4c8d6..a0b7a07db2 100644 --- a/librz/bin/pdb/modi.c +++ b/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); } diff --git a/librz/bin/pdb/modi.h b/librz/bin/pdb/modi.h index ddebf41123..71d6a71c1b 100644 --- a/librz/bin/pdb/modi.h +++ b/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 diff --git a/librz/bin/pdb/pdb.c b/librz/bin/pdb/pdb.c index 25ea3a09c7..24e8f6b5e4 100644 --- a/librz/bin/pdb/pdb.c +++ b/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); diff --git a/librz/bin/pdb/symbol.c b/librz/bin/pdb/symbol.c index ee8c6f54fb..ee802f01ce 100644 --- a/librz/bin/pdb/symbol.c +++ b/librz/bin/pdb/symbol.c @@ -224,6 +224,7 @@ RZ_IPI bool PDBSymbolIter_collect(PDBSymbolIter *iter, RzPVector /* return false; } if (!PDBSymbolIter_next(iter, symbol)) { + free(symbol); break; } rz_pvector_push(symbols, symbol); diff --git a/librz/core/canalysis.c b/librz/core/canalysis.c index c0660cd12b..82822b9a1a 100644 --- a/librz/core/canalysis.c +++ b/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++; diff --git a/librz/core/cio.c b/librz/core/cio.c index b5b94deaaa..7cd1368904 100644 --- a/librz/core/cio.c +++ b/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; } diff --git a/librz/core/cmd/cmd_print.c b/librz/core/cmd/cmd_print.c index 729f8d279b..4775dd36bc 100644 --- a/librz/core/cmd/cmd_print.c +++ b/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; } diff --git a/librz/core/cpdb.c b/librz/core/cpdb.c index 6d8ad07cea..f92d77fe1d 100644 --- a/librz/core/cpdb.c +++ b/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) { diff --git a/librz/core/ctypes.c b/librz/core/ctypes.c index ee9eb23e77..4cbd9bed6b 100644 --- a/librz/core/ctypes.c +++ b/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) { diff --git a/librz/core/devirtualize_cxx.c b/librz/core/devirtualize_cxx.c index b5d3e731ec..a2448ecf80 100644 --- a/librz/core/devirtualize_cxx.c +++ b/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; } +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(ctx->virtual_xref, vfunc, &found); + if (!found) { + set = rz_set_u_new(); + 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); - RzPVector *pvect = rz_set_s_to_vector(vfunc_set); - void **it; - rz_pvector_foreach (pvect, it) { - const char *vfunc = *it; - bool found = false; - RzSetU *set = ht_sp_find(virtual_xref, vfunc, &found); - if (!found) { - set = rz_set_u_new(); - ht_sp_insert(virtual_xref, vfunc, set); - } - rz_set_u_add(set, key); - } + VirtualXrefCtx ctx = { virtual_xref, key }; + ht_sp_foreach((HtSP *)vfunc_set, collect_virtual_xref, &ctx); return true; } diff --git a/librz/debug/debug.c b/librz/debug/debug.c index 2cafdd86fa..f97004cfdc 100644 --- a/librz/debug/debug.c +++ b/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) { diff --git a/librz/flag/flag.c b/librz/flag/flag.c index d728ef513d..2e8ad392f8 100644 --- a/librz/flag/flag.c +++ b/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");