From af2ac16182b6243f17e06ec75e441014159abe5e Mon Sep 17 00:00:00 2001 From: Cyrille Bagard Date: Sun, 15 Jan 2017 15:47:04 +0100 Subject: Improved symbol resolving using fully defined locations. --- ChangeLog | 26 +++++++++++++++++++++ plugins/pychrysa/format/format.c | 9 ++++++++ src/analysis/disass/area.c | 6 +++++ src/analysis/disass/links.c | 19 ++++++++------- src/arch/arm/v7/post.c | 10 ++++---- src/arch/dalvik/link.c | 24 ++++++++++--------- src/arch/link.c | 21 ++++++++++------- src/arch/post.c | 2 +- src/arch/target.c | 37 ++++++++++++++--------------- src/arch/target.h | 4 ++-- src/format/elf/symbols.c | 2 ++ src/format/format.c | 50 ++++++++++++++++++++++++++++++---------- src/gui/menus/edition.c | 27 +++++++++++----------- 13 files changed, 157 insertions(+), 80 deletions(-) diff --git a/ChangeLog b/ChangeLog index 3339d96..6244d1e 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,31 @@ 17-01-15 Cyrille Bagard + * plugins/pychrysa/format/format.c: + * src/analysis/disass/area.c: + Fix memory leaks. + + * src/analysis/disass/links.c: + * src/arch/arm/v7/post.c: + * src/arch/dalvik/link.c: + * src/arch/link.c: + * src/arch/post.c: + Force fully defined locations when dealing with target operands. + + * src/arch/target.c: + * src/arch/target.h: + Update code. + + * src/format/elf/symbols.c: + Fix memory leaks. + + * src/format/format.c: + Improve symbol resolving using fully defined locations. + + * src/gui/menus/edition.c: + Force fully defined locations when dealing with target operands. + +17-01-15 Cyrille Bagard + * src/analysis/disass/area.c: * src/format/format.c: Ensure that symbols always take priority over the disassembled code. diff --git a/plugins/pychrysa/format/format.c b/plugins/pychrysa/format/format.c index 0628697..cc9e819 100644 --- a/plugins/pychrysa/format/format.c +++ b/plugins/pychrysa/format/format.c @@ -356,7 +356,10 @@ static PyObject *py_binary_format_find_symbol_at(PyObject *self, PyObject *args) found = g_binary_format_find_symbol_at(format, get_internal_vmpa(py_vmpa), &symbol); if (found) + { result = pygobject_new(G_OBJECT(symbol)); + g_object_unref(G_OBJECT(symbol)); + } else { result = Py_None; @@ -401,7 +404,10 @@ static PyObject *py_binary_format_find_next_symbol_at(PyObject *self, PyObject * found = g_binary_format_find_next_symbol_at(format, get_internal_vmpa(py_vmpa), &symbol); if (found) + { result = pygobject_new(G_OBJECT(symbol)); + g_object_unref(G_OBJECT(symbol)); + } else { result = Py_None; @@ -452,6 +458,9 @@ static PyObject *py_binary_format_resolve_symbol(PyObject *self, PyObject *args) result = PyTuple_New(2); PyTuple_SetItem(result, 0, pygobject_new(G_OBJECT(symbol))); PyTuple_SetItem(result, 1, PyLong_FromUnsignedLongLong(diff)); + + g_object_unref(G_OBJECT(symbol)); + } else { diff --git a/src/analysis/disass/area.c b/src/analysis/disass/area.c index 7eacc29..389dd75 100644 --- a/src/analysis/disass/area.c +++ b/src/analysis/disass/area.c @@ -484,6 +484,9 @@ static void update_address_as_routine(GBinFormat *format, const vmpa2t *addr) } + if (found) + g_object_unref(G_OBJECT(symbol)); + } @@ -632,7 +635,10 @@ void load_code_from_mem_area(mem_area *area, mem_area *list, size_t count, GProc has_new_sym = g_binary_format_find_symbol_at(format, &sym_addr, &symbol); if (has_new_sym) + { insert_extra_symbol_into_mem_areas(list, count, symbol); + g_object_unref(G_OBJECT(symbol)); + } } diff --git a/src/analysis/disass/links.c b/src/analysis/disass/links.c index 77ec2af..d3434cb 100644 --- a/src/analysis/disass/links.c +++ b/src/analysis/disass/links.c @@ -139,6 +139,7 @@ static void convert_immediate_into_target(GArchInstruction *instr, size_t index, GImmOperand *imm; /* Version native de l'opérande*/ virt_t addr; /* Adresse visée par le saut */ MemoryDataSize msize; /* Taille de l'opérande */ + vmpa2t target; /* Défination finale précise */ GArchOperand *new; /* Instruction de ciblage */ op = g_arch_instruction_get_operand(instr, index); @@ -152,12 +153,16 @@ static void convert_immediate_into_target(GArchInstruction *instr, size_t index, { msize = g_imm_operand_get_size(imm); - new = g_target_operand_new(msize, addr); + if (g_exe_format_translate_address_into_vmpa(G_EXE_FORMAT(format), addr, &target)) + { + new = g_target_operand_new(msize, &target); + + if (!g_target_operand_resolve(G_TARGET_OPERAND(new), format, false)) + g_object_unref(G_OBJECT(new)); + else + g_arch_instruction_replace_operand(instr, new, op); - if (!g_target_operand_resolve(G_TARGET_OPERAND(new), format, false)) - g_object_unref(G_OBJECT(new)); - else - g_arch_instruction_replace_operand(instr, new, op); + } } @@ -184,7 +189,6 @@ void establish_links_for_instruction(GArchInstruction *instr, GBinFormat *format size_t count; /* Nombre d'opérandes présents */ size_t i; /* Boucle de parcours */ GArchOperand *op; /* Opérande numérique en place */ - virt_t virt; /* Adresse liée à une cible */ vmpa2t addr; /* Localisation plus complète */ GArchInstruction *target; /* Instruction visée au final */ @@ -203,8 +207,7 @@ void establish_links_for_instruction(GArchInstruction *instr, GBinFormat *format op = g_arch_instruction_get_operand(instr, i); if (!G_IS_TARGET_OPERAND(op)) continue; - virt = g_target_operand_get_addr(G_TARGET_OPERAND(op)); - init_vmpa(&addr, VMPA_NO_PHYSICAL, virt); + g_target_operand_get_addr(G_TARGET_OPERAND(op), &addr); target = g_arch_processor_find_instr_by_address(proc, &addr); diff --git a/src/arch/arm/v7/post.c b/src/arch/arm/v7/post.c index 4037c05..5ac3c62 100644 --- a/src/arch/arm/v7/post.c +++ b/src/arch/arm/v7/post.c @@ -49,10 +49,10 @@ void post_process_ldr_instructions(GArchInstruction *instr, GArchProcessor *proc uint32_t addr; /* Adresse visée par le saut */ GBinFormat *bfmt; /* Version basique du format */ GArchOperand *new; /* Instruction de ciblage */ - vmpa2t target; + vmpa2t target; /* Défination finale précise */ mrange_t trange; /* Etendue du symbole à créer */ - VMPA_BUFFER(loc); - char name[5 + VMPA_MAX_LEN]; + VMPA_BUFFER(loc); /* Espace pour une conversion */ + char name[5 + VMPA_MAX_LEN]; /* Etiquette à constituer */ GBinRoutine *routine; /* Nouvelle routine trouvée */ GBinSymbol *symbol; /* Nouveau symbole construit */ @@ -67,7 +67,7 @@ void post_process_ldr_instructions(GArchInstruction *instr, GArchProcessor *proc { bfmt = G_BIN_FORMAT(format); - new = g_target_operand_new(MDS_32_BITS_UNSIGNED, addr); + new = g_target_operand_new(MDS_32_BITS_UNSIGNED, &target); if (!g_target_operand_resolve(G_TARGET_OPERAND(new), bfmt, true)) { @@ -88,8 +88,6 @@ void post_process_ldr_instructions(GArchInstruction *instr, GArchProcessor *proc g_binary_symbol_attach_routine(symbol, routine); g_binary_format_add_symbol(bfmt, symbol); - - g_target_operand_resolve(G_TARGET_OPERAND(new), bfmt, true); } diff --git a/src/arch/dalvik/link.c b/src/arch/dalvik/link.c index 3295c76..f92541b 100644 --- a/src/arch/dalvik/link.c +++ b/src/arch/dalvik/link.c @@ -80,8 +80,9 @@ typedef struct _case_comment void handle_dalvik_packed_switch_links(GArchInstruction *instr, GArchProcessor *proc, GProcContext *context, GExeFormat *format) { GArchOperand *op; /* Opérande numérique en place */ - virt_t virt; /* Adresse virtuelle */ + bool defined; /* Adresse définie ? */ vmpa2t addr; /* Adresse de destination */ + virt_t virt; /* Adresse virtuelle */ GArchInstruction *switch_ins; /* Instruction de branchements */ const mrange_t *range; /* Zone d'occupation */ const vmpa2t *start_addr; /* Adresse de référentiel */ @@ -104,24 +105,25 @@ void handle_dalvik_packed_switch_links(GArchInstruction *instr, GArchProcessor * op = g_arch_instruction_get_operand(instr, 1); - virt = VMPA_NO_VIRTUAL; + defined = false; if (G_IS_TARGET_OPERAND(op)) - virt = g_target_operand_get_addr(G_TARGET_OPERAND(op)); + { + g_target_operand_get_addr(G_TARGET_OPERAND(op), &addr); + defined = true; + } else if (G_IS_IMM_OPERAND(op)) { - if (!g_imm_operand_to_virt_t(G_IMM_OPERAND(op), &virt)) - virt = VMPA_NO_VIRTUAL; + if (g_imm_operand_to_virt_t(G_IMM_OPERAND(op), &virt)) + { + init_vmpa(&addr, VMPA_NO_PHYSICAL, virt); + defined = true; + } } - if (virt != VMPA_NO_VIRTUAL) + if (defined) { - /* TODO : utiliser format pour contruire une adresse avec une position physique, - * ce qui accélèrerait les recherches. - */ - init_vmpa(&addr, VMPA_NO_PHYSICAL, virt); - switch_ins = g_arch_processor_find_instr_by_address(proc, &addr); if (G_IS_DALVIK_SWITCH_INSTR(switch_ins)) diff --git a/src/arch/link.c b/src/arch/link.c index 8b02aa8..0e11521 100644 --- a/src/arch/link.c +++ b/src/arch/link.c @@ -95,8 +95,9 @@ void handle_jump_as_link(GArchInstruction *instr, GArchProcessor *proc, GProcCon void handle_branch_as_link(GArchInstruction *instr, GArchProcessor *proc, GProcContext *context, GExeFormat *format, size_t index) { GArchOperand *op; /* Opérande numérique en place */ - virt_t virt; /* Adresse virtuelle */ + bool defined; /* Adresse définie ? */ vmpa2t addr; /* Adresse de destination */ + virt_t virt; /* Adresse virtuelle */ instr_iter_t *iter; /* Parcours d'instructions */ GArchInstruction *target; /* Ligne visée par la référence*/ @@ -104,21 +105,25 @@ void handle_branch_as_link(GArchInstruction *instr, GArchProcessor *proc, GProcC op = g_arch_instruction_get_operand(instr, index); - virt = VMPA_NO_VIRTUAL; + defined = false; if (G_IS_TARGET_OPERAND(op)) - virt = g_target_operand_get_addr(G_TARGET_OPERAND(op)); + { + g_target_operand_get_addr(G_TARGET_OPERAND(op), &addr); + defined = true; + } else if (G_IS_IMM_OPERAND(op)) { - if (!g_imm_operand_to_virt_t(G_IMM_OPERAND(op), &virt)) - virt = VMPA_NO_VIRTUAL; + if (g_imm_operand_to_virt_t(G_IMM_OPERAND(op), &virt)) + { + init_vmpa(&addr, VMPA_NO_PHYSICAL, virt); + defined = true; + } } - if (virt != VMPA_NO_VIRTUAL) + if (defined) { - init_vmpa(&addr, VMPA_NO_PHYSICAL, virt); - iter = g_arch_processor_get_iter_from_address(proc, &addr); if (iter != NULL) diff --git a/src/arch/post.c b/src/arch/post.c index 9a296d4..59ed40e 100644 --- a/src/arch/post.c +++ b/src/arch/post.c @@ -73,7 +73,7 @@ void post_process_target_resolution(GArchInstruction *instr, GArchProcessor *pro ptr_size = g_arch_processor_get_memory_size(proc); - new = g_target_operand_new(ptr_size, addr); + new = g_target_operand_new(ptr_size, &target); if (!g_target_operand_resolve(G_TARGET_OPERAND(new), bfmt, true)) { diff --git a/src/arch/target.c b/src/arch/target.c index 4655351..63ae295 100644 --- a/src/arch/target.c +++ b/src/arch/target.c @@ -43,7 +43,7 @@ struct _GTargetOperand GArchOperand parent; /* Instance parente */ MemoryDataSize size; /* Taille de l'opérande */ - virt_t addr; /* Adresse de l'élément visé */ + vmpa2t addr; /* Adresse de l'élément visé */ GBinSymbol *symbol; /* Eventuel symbole associé */ phys_t diff; /* Position dans le symbole */ @@ -171,7 +171,7 @@ static void g_target_operand_finalize(GTargetOperand *operand) /****************************************************************************** * * * Paramètres : size = taille des adresse mémoire virtuelles. * -* addr = adresse virtuelle d'un élément à retrouver. * +* addr = localisation d'un élément à retrouver. * * * * Description : Crée un opérande réprésentant une valeur numérique. * * * @@ -181,14 +181,14 @@ static void g_target_operand_finalize(GTargetOperand *operand) * * ******************************************************************************/ -GArchOperand *g_target_operand_new(MemoryDataSize size, virt_t addr) +GArchOperand *g_target_operand_new(MemoryDataSize size, const vmpa2t *addr) { GTargetOperand *result; /* Opérande à retourner */ result = g_object_new(G_TYPE_TARGET_OPERAND, NULL); result->size = size; - result->addr = addr; + copy_vmpa(&result->addr, addr); return G_ARCH_OPERAND(result); @@ -240,8 +240,7 @@ static void g_target_operand_print(const GTargetOperand *operand, GBufferLine *l } else { - init_vmpa(&tmp, VMPA_NO_PHYSICAL, operand->addr); - vmpa2_virt_to_string(&tmp, operand->size, value, &len); + vmpa2_virt_to_string(&operand->addr, operand->size, value, &len); g_buffer_line_append_text(line, BLC_MAIN, value, len, RTT_LABEL, G_OBJECT(operand)); @@ -271,19 +270,20 @@ MemoryDataSize g_target_operand_get_size(const GTargetOperand *operand) /****************************************************************************** * * -* Paramètres : operand = structure dont le contenu est à définir. * +* Paramètres : operand = structure dont le contenu est à consulter. * +* addr = localisation à renseigner. [OUT] * * * * Description : Fournit l'adresse en mémoire de l'élément visé. * * * -* Retour : Adresse en mémoire virtuelle associée. * +* Retour : - * * * * Remarques : - * * * ******************************************************************************/ -virt_t g_target_operand_get_addr(const GTargetOperand *operand) +void g_target_operand_get_addr(const GTargetOperand *operand, vmpa2t *addr) { - return operand->addr; + copy_vmpa(addr, &operand->addr); } @@ -305,17 +305,11 @@ virt_t g_target_operand_get_addr(const GTargetOperand *operand) bool g_target_operand_resolve(GTargetOperand *operand, GBinFormat *format, bool strict) { bool result; /* Bilan à retourner */ - vmpa2t addr; /* Adresse de recherche */ if (operand->symbol != NULL) g_object_unref(G_OBJECT(operand->symbol)); - operand->symbol = NULL; - operand->diff = 0; - - init_vmpa(&addr, VMPA_NO_PHYSICAL, operand->addr); - - result = g_binary_format_resolve_symbol(format, &addr, strict, &operand->symbol, &operand->diff); + result = g_binary_format_resolve_symbol(format, &operand->addr, strict, &operand->symbol, &operand->diff); return result; @@ -337,9 +331,16 @@ bool g_target_operand_resolve(GTargetOperand *operand, GBinFormat *format, bool GBinSymbol *g_target_operand_get_symbol(const GTargetOperand *operand, phys_t *diff) { + GBinSymbol *result; /* Symbole associé à retourner */ + if (diff != NULL) *diff = operand->diff; - return operand->symbol; + result = operand->symbol; + + if (result != NULL) + g_object_ref(G_OBJECT(result)); + + return result; } diff --git a/src/arch/target.h b/src/arch/target.h index e1ea173..529dba8 100644 --- a/src/arch/target.h +++ b/src/arch/target.h @@ -55,13 +55,13 @@ typedef struct _GTargetOperandClass GTargetOperandClass; GType g_target_operand_get_type(void); /* Crée un opérande réprésentant une valeur numérique. */ -GArchOperand *g_target_operand_new(MemoryDataSize, virt_t); +GArchOperand *g_target_operand_new(MemoryDataSize, const vmpa2t *); /* Renseigne la taille de la valeur indiquée à la construction. */ MemoryDataSize g_target_operand_get_size(const GTargetOperand *); /* Fournit l'adresse en mémoire de l'élément visé. */ -virt_t g_target_operand_get_addr(const GTargetOperand *); +void g_target_operand_get_addr(const GTargetOperand *, vmpa2t *); /* Tente une résolution de symbole. */ bool g_target_operand_resolve(GTargetOperand *, GBinFormat *, bool); diff --git a/src/format/elf/symbols.c b/src/format/elf/symbols.c index b936a47..8451911 100644 --- a/src/format/elf/symbols.c +++ b/src/format/elf/symbols.c @@ -215,6 +215,8 @@ static void register_elf_entry_point(GElfFormat *format, virt_t vaddr, phys_t le _g_binary_symbol_attach_routine(symbol, routine, STP_ENTRY_POINT); + g_object_unref(G_OBJECT(symbol)); + } else { diff --git a/src/format/format.c b/src/format/format.c index 550e816..327b28e 100644 --- a/src/format/format.c +++ b/src/format/format.c @@ -297,21 +297,29 @@ bool g_binary_format_add_symbol(GBinFormat *format, GBinSymbol *symbol) GBinRoutine *routine; /* Nouvelle routine à insérer */ /** - * Lorsque les fonctions de recherche type g_binary_format_find_symbol_at() - * sont appelées avec une localisation, cette dernière peut reposer soit sur - * une position physique soit une adresse virtuelle uniquement. + * Pour que les fonctions de recherche basées sur _g_binary_format_find_symbol() + * fassent bien leur office, il faut que les symboles soient triés. * - * Pour que les comparaisons de positions puissent se réaliser, il faut donc - * pouvoir satisfaire les deux aspects : physiques et virtuels. + * Cependant, les localisations à satisfaire lors d'une recherche recontrent + * un problème si les positions physiques ne sont pas renseignées. En effet + * les adresses virtuelles en sont potentiellement décorrélées (c'est le cas + * avec le format ELF par exemple, où les zones en mémoire ne suivent pas le + * même ordre que les segments du binaire). * - * On corrige donc le tir si besoin est ici. + * Comme les comparaisons entre localisations se réalisent sur les éléments + * renseignés communs, à commencer par la position physique si c'est possible, + * une localisation s'appuyant uniquement sur une adresse virtuelle va être + * analysée suivant une liste non triée d'adresses virtuelles. + * + * On corrige donc le tir si besoin est en forçant la comparaison via les + * positions physiques. */ #ifndef NDEBUG range = g_binary_symbol_get_range(symbol); addr = get_mrange_addr(range); - assert(!is_invalid_vmpa(addr)); + assert(has_phys_addr(addr)); #endif g_rw_lock_writer_lock(&format->syms_lock); @@ -684,9 +692,12 @@ static bool _g_binary_format_find_symbol(const GBinFormat *format, const vmpa2t bool result; /* Bilan à retourner */ void *found; /* Résultat de recherches */ - result = false; + /** + * Pour ce qui est des justifications quant à la vérification suivante, + * se référer aux commentaires placés dans g_binary_format_add_symbol(). + */ - *symbol = NULL; /* TODO : vérifier les doublons côtés appelants */ + assert(has_phys_addr(addr)); found = bsearch(addr, format->symbols, format->symbols_count, sizeof(GBinSymbol *), fn); @@ -695,13 +706,22 @@ static bool _g_binary_format_find_symbol(const GBinFormat *format, const vmpa2t if (index != NULL) *index = (GBinSymbol **)found - format->symbols; - *symbol = *(GBinSymbol **)found; + if (symbol != NULL) + { + *symbol = *(GBinSymbol **)found; + g_object_ref(G_OBJECT(*symbol)); + } - g_object_ref(G_OBJECT(*symbol)); result = true; } + else + { + *symbol = NULL; + result = false; + } + return result; } @@ -816,7 +836,7 @@ bool g_binary_format_find_next_symbol_at(GBinFormat *format, const vmpa2t *addr, g_rw_lock_reader_lock(&format->syms_lock); - result = _g_binary_format_find_symbol(format, addr, (__compar_fn_t)find_symbol, &index, symbol); + result = _g_binary_format_find_symbol(format, addr, (__compar_fn_t)find_symbol, &index, NULL); if (result && (index + 1) < format->symbols_count) { @@ -826,7 +846,10 @@ bool g_binary_format_find_next_symbol_at(GBinFormat *format, const vmpa2t *addr, } else + { + *symbol = NULL; result = false; + } g_rw_lock_reader_unlock(&format->syms_lock); @@ -870,6 +893,9 @@ bool g_binary_format_resolve_symbol(GBinFormat *format, const vmpa2t *addr, bool } + else + *diff = 0; + return result; } diff --git a/src/gui/menus/edition.c b/src/gui/menus/edition.c index 9c01a6b..31fbc0d 100644 --- a/src/gui/menus/edition.c +++ b/src/gui/menus/edition.c @@ -434,8 +434,9 @@ static void mcb_edition_follow_ref(GtkMenuItem *menuitem, GMenuBar *bar) GtkDisplayPanel *panel; /* Afficheur effectif de code */ GBufferLine *line; /* Ligne de position courante */ GObject *creator; /* Créateur à l'orgine du seg. */ - virt_t virt; /* Adresse virtuelle */ + bool defined; /* Adresse définie ? */ vmpa2t addr; /* Adresse de destination */ + virt_t virt; /* Adresse virtuelle */ panel = g_editor_item_get_current_view(G_EDITOR_ITEM(bar)); @@ -443,27 +444,25 @@ static void mcb_edition_follow_ref(GtkMenuItem *menuitem, GMenuBar *bar) { assert(creator != NULL); - /** - * On fait le pari de reposer uniquement sur des adresses virtuelles ! - * A changer dans un futur ? - */ - - virt = VMPA_NO_VIRTUAL; + defined = false; if (G_IS_TARGET_OPERAND(creator)) - virt = g_target_operand_get_addr(G_TARGET_OPERAND(creator)); + { + g_target_operand_get_addr(G_TARGET_OPERAND(creator), &addr); + defined = true; + } else if (G_IS_IMM_OPERAND(creator)) { - if (!g_imm_operand_to_virt_t(G_IMM_OPERAND(creator), &virt)) - virt = VMPA_NO_VIRTUAL; + if (g_imm_operand_to_virt_t(G_IMM_OPERAND(creator), &virt)) + { + init_vmpa(&addr, VMPA_NO_PHYSICAL, virt); + defined = true; + } } - if (virt != VMPA_NO_VIRTUAL) - { - init_vmpa(&addr, VMPA_NO_PHYSICAL, virt); + if (defined) gtk_display_panel_request_move(panel, &addr); - } g_object_unref(creator); g_object_unref(G_OBJECT(line)); -- cgit v0.11.2-87-g4458