From dcd5edcc34fc185db03e25bc1b441acbdd5c3f8d Mon Sep 17 00:00:00 2001 From: renoseven Date: Fri, 25 Apr 2025 10:12:09 +0800 Subject: [PATCH 1/6] upatch-diff: impl section type checking Signed-off-by: renoseven --- upatch-diff/elf-common.h | 78 +++++++++++++++++++++++++++++----------- 1 file changed, 57 insertions(+), 21 deletions(-) diff --git a/upatch-diff/elf-common.h b/upatch-diff/elf-common.h index bc2c1acf..5e63b098 100644 --- a/upatch-diff/elf-common.h +++ b/upatch-diff/elf-common.h @@ -49,31 +49,40 @@ } \ } while (0) -static inline bool is_rela_section(struct section *sec) +static inline bool is_text_section(struct section *sec) { - /* - * An architecture usually only accepts one type. - * And, X86_64 only uses RELA - */ - return (sec->sh.sh_type == SHT_RELA); + return (sec != NULL) && + (sec->sh.sh_type == SHT_PROGBITS) && + (sec->sh.sh_flags & SHF_EXECINSTR); } +static inline bool is_data_section(struct section *sec) +{ + return (sec != NULL) && + (sec->sh.sh_type == SHT_PROGBITS) && + (sec->sh.sh_flags & SHF_WRITE); +} -static inline bool is_text_section(struct section *sec) +static inline bool is_rodata_section(struct section *sec) { - return (sec->sh.sh_type == SHT_PROGBITS && - (sec->sh.sh_flags & SHF_EXECINSTR)); + return (sec != NULL) && + (sec->sh.sh_type == SHT_PROGBITS) && + ((sec->sh.sh_flags & SHF_WRITE) == 0); } -static inline bool is_string_section(struct section *sec) +static inline bool is_symtab_section(struct section *sec) { - return sec->sh.sh_flags & SHF_STRINGS; + return (sec != NULL) && (sec->sh.sh_type == SHT_SYMTAB); } -/* used for c++ exception handle */ -static inline bool is_except_section(struct section *sec) +static inline bool is_strtab_section(struct section *sec) { - return !strncmp(sec->name, ".gcc_except_table", 17); + return (sec != NULL) && (sec->sh.sh_type == SHT_STRTAB); +} + +static inline bool is_rela_section(struct section *sec) +{ + return (sec != NULL) && (sec->sh.sh_type == SHT_RELA); } static inline bool is_note_section(struct section *sec) @@ -81,7 +90,40 @@ static inline bool is_note_section(struct section *sec) if (is_rela_section(sec)) { sec = sec->base; } - return sec->sh.sh_type == SHT_NOTE; + return (sec->sh.sh_type == SHT_NOTE); +} + +static inline bool is_bss_section(struct section *sec) +{ + return (sec != NULL) && (sec->sh.sh_type == SHT_NOBITS); +} + +static inline bool is_group_section(struct section *sec) +{ + return (sec != NULL) && (sec->sh.sh_type == SHT_GROUP); +} + +static inline bool is_read_only_section(struct section *sec) +{ + return (sec != NULL) && ((sec->sh.sh_flags & SHF_WRITE) == 0); +} + +static inline bool is_string_section(struct section *sec) +{ + return (sec != NULL) && (sec->sh.sh_flags & SHF_STRINGS); +} + +static inline bool is_string_literal_section(struct section *sec) +{ + return (sec != NULL) && + (sec->sh.sh_flags & SHF_STRINGS) && + ((sec->sh.sh_flags & SHF_WRITE) == 0); +} + +/* used for c++ exception handle */ +static inline bool is_except_section(struct section *sec) +{ + return !strncmp(sec->name, ".gcc_except_table", 17); } static inline bool is_eh_frame(struct section *sec) @@ -160,12 +202,6 @@ static inline struct section *find_section_by_name(struct list_head *list, const return NULL; } -// section like .rodata.str1. and .rodata.__func__. -static inline bool is_string_literal_section(struct section *sec) -{ - return !strncmp(sec->name, ".rodata.", 8) && (strstr(sec->name, ".str") || strstr(sec->name, "__func__")); -} - static inline bool has_digit_tail(char *tail) { if (*tail != '.') { -- Gitee From 51e542fd98c508350f6681ac571c0ecbfc754610 Mon Sep 17 00:00:00 2001 From: renoseven Date: Fri, 25 Apr 2025 15:45:38 +0800 Subject: [PATCH 2/6] upatch-diff: allow string literal section being included Signed-off-by: renoseven --- upatch-diff/create-diff-object.c | 53 +++++++++++++++++--------------- 1 file changed, 29 insertions(+), 24 deletions(-) diff --git a/upatch-diff/create-diff-object.c b/upatch-diff/create-diff-object.c index 54de38ef..8a4089dc 100644 --- a/upatch-diff/create-diff-object.c +++ b/upatch-diff/create-diff-object.c @@ -910,34 +910,39 @@ static bool has_tls_included(struct upatch_elf *uelf) static void verify_patchability(struct upatch_elf *uelf) { - struct section *sec; + struct section *sec = NULL; + struct rela *rela = NULL; int errs = 0; list_for_each_entry(sec, &uelf->sections, list) { - if (sec->status == CHANGED && !sec->include && !is_rela_section(sec)) { - log_normal("Section '%s' changed, but not included\n", sec->name); - errs++; - } - - if (sec->status != SAME && sec->grouped) { - log_normal("Section '%s' is changed, but it is in section group\n", - sec->name); - errs++; - } - - if (sec->sh.sh_type == SHT_GROUP && sec->status == NEW) { - log_normal("Section '%s' is new, but it is not supported\n", - sec->name); - errs++; + if (sec->status != SAME) { // NEW or CHANGED + if ((sec->include == 0) && !is_rela_section(sec)) { + log_warn("Section '%s' is changed, but it is not included\n", + sec->name); + errs++; + } + if ((sec->grouped != 0) || is_group_section(sec)) { + log_warn("Section '%s' is changed, but it is in a group\n", + sec->name); + errs++; + } } - - if ((sec->include && sec->status != NEW) && - (!strncmp(sec->name, ".data", 5) || - !strncmp(sec->name, ".bss", 4)) && - (strcmp(sec->name, ".data.unlikely") && - strcmp(sec->name, ".data.once"))) { - log_normal("Data section '%s' is included", sec->name); - errs++; + if (sec->status != NEW) { // SAME or CHANGED + if (sec->rela == NULL) { + continue; + } + if (!is_data_section(sec) && !is_bss_section(sec)) { + continue; + } + list_for_each_entry(rela, &sec->rela->relas, list) { + if ((rela->sym == NULL) || (rela->sym->status == NEW)) { + continue; + } + if (!is_string_literal_section(rela->sym->sec)) { + log_warn("Data section '%s' is changed\n", sec->name); + errs++; + } + } } } -- Gitee From 245ece82a753c71c26d755d06279812328f92f77 Mon Sep 17 00:00:00 2001 From: renoseven Date: Fri, 25 Apr 2025 18:06:32 +0800 Subject: [PATCH 3/6] upatch-diff: fix cannot detect some changes issue Signed-off-by: renoseven --- upatch-diff/create-diff-object.c | 53 ++++++++++++++++---------------- 1 file changed, 26 insertions(+), 27 deletions(-) diff --git a/upatch-diff/create-diff-object.c b/upatch-diff/create-diff-object.c index 8a4089dc..731bbdb1 100644 --- a/upatch-diff/create-diff-object.c +++ b/upatch-diff/create-diff-object.c @@ -763,24 +763,24 @@ static void include_symbol(struct symbol *sym) static void include_section(struct section *sec) { - struct rela *rela; + struct rela *rela = NULL; - if (sec->include) { + if (sec->include != 0) { return; } sec->include = 1; - if (sec->secsym) { - sec->secsym->include = 1; - } - if (!sec->rela) { - return; - } - - sec->rela->include = 1; - list_for_each_entry(rela, &sec->rela->relas, list) { - include_symbol(rela->sym); + if (!is_rela_section(sec)) { + if (sec->secsym != NULL) { + sec->secsym->include = 1; + } + if (sec->rela != NULL) { + sec->rela->include = 1; + list_for_each_entry(rela, &sec->rela->relas, list) { + include_symbol(rela->sym); + } + } } } @@ -808,27 +808,26 @@ static void include_standard_elements(struct upatch_elf *uelf) list_entry(uelf->symbols.next, struct symbol, list)->include = 1; } -static int include_changed_functions(struct upatch_elf *uelf) +static int include_changes(struct upatch_elf *uelf) { struct symbol *sym; int changed_nr = 0; list_for_each_entry(sym, &uelf->symbols, list) { - if (sym->status == CHANGED && - sym->type == STT_FUNC) { - changed_nr++; - include_symbol(sym); + if (sym->status != CHANGED) { + continue; } - /* exception handler is a special function */ - if (sym->status == CHANGED && - sym->type == STT_SECTION && - sym->sec && is_except_section(sym->sec)) { - log_warn("Exception section '%s' is changed\n", sym->sec->name); + + if (sym->type == STT_FUNC) { changed_nr++; include_symbol(sym); - } - if (sym->type == STT_FILE) { - sym->include = 1; + } else if (sym->type == STT_SECTION) { + if (is_string_literal_section(sym->sec) || is_except_section(sym->sec)) { + changed_nr++; + include_symbol(sym); + } + } else if (sym->type == STT_FILE) { + include_symbol(sym); } } @@ -935,7 +934,7 @@ static void verify_patchability(struct upatch_elf *uelf) continue; } list_for_each_entry(rela, &sec->rela->relas, list) { - if ((rela->sym == NULL) || (rela->sym->status == NEW)) { + if ((rela->sym == NULL) || (rela->sym->status != CHANGED)) { continue; } if (!is_string_literal_section(rela->sym->sec)) { @@ -1064,7 +1063,7 @@ int main(int argc, char*argv[]) include_standard_elements(&uelf_patched); - num_changed = include_changed_functions(&uelf_patched); + num_changed = include_changes(&uelf_patched); new_globals_exist = include_new_globals(&uelf_patched); if (!num_changed && !new_globals_exist) { log_normal("No functional changes\n"); -- Gitee From 4bc094610b542b51875baf517c6ebf1860f616d3 Mon Sep 17 00:00:00 2001 From: renoseven Date: Fri, 25 Apr 2025 10:25:11 +0800 Subject: [PATCH 4/6] upatch-diff: fix .text section change does not include symbol issue Signed-off-by: renoseven --- upatch-diff/elf-compare.c | 37 +++++++++++++++++++++++-------------- 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/upatch-diff/elf-compare.c b/upatch-diff/elf-compare.c index fe975283..64435443 100644 --- a/upatch-diff/elf-compare.c +++ b/upatch-diff/elf-compare.c @@ -29,24 +29,33 @@ #include "elf-compare.h" #include "elf-insn.h" -static void compare_correlated_symbol(struct symbol *sym, - struct symbol *symtwin) +static int compare_correlated_section(struct section *sec, struct section *twin); + +static void compare_correlated_symbol(struct symbol *sym, struct symbol *twin) { - // compare bind and type info - if (sym->sym.st_info != symtwin->sym.st_info || - (sym->sec && !symtwin->sec) || - (symtwin->sec && !sym->sec)) { - ERROR("symbol info mismatch: %s", sym->name); + // symbol type & binding cannot be changed + if (sym->type != twin->type) { + ERROR("symbol '%s' type mismatched", sym->name); } - // check if correlated symbols have correlated sections - if (sym->sec && symtwin->sec && sym->sec->twin != symtwin->sec) { - ERROR("symbol changed sections: %s", sym->name); + if (sym->sym.st_info != twin->sym.st_info) { + ERROR("symbol '%s' binding mismatched", sym->name); } - // data object can't change size - if (sym->type == STT_OBJECT && sym->sym.st_size != symtwin->sym.st_size) { - ERROR("object size mismatch: %s", sym->name); + // object symbol size cannot be changed + if ((sym->type == STT_OBJECT) && (sym->sym.st_size != twin->sym.st_size)) { + ERROR("symbol '%s' object size mismatched", sym->name); } - if (sym->sym.st_shndx == SHN_UNDEF || sym->sym.st_shndx == SHN_ABS) { + + // compare symbol sections + if ((sym->sec != NULL) && (twin->sec != NULL)) { + // symbol section correlation cannot be changed + if (sym->sec->twin != twin->sec) { + ERROR("symbol '%s' section mismatched", sym->name); + } + compare_correlated_section(sym->sec, twin->sec); + sym->status = sym->sec->status; + } + + if ((sym->sym.st_shndx == SHN_UNDEF) || (sym->sym.st_shndx == SHN_ABS)) { sym->status = SAME; } /* -- Gitee From b9f12ab64643e92ec202a3f0713739c8c1b392a9 Mon Sep 17 00:00:00 2001 From: renoseven Date: Fri, 25 Apr 2025 18:13:55 +0800 Subject: [PATCH 5/6] upatch-diff: fix .data.rel section is changed issue Signed-off-by: renoseven --- upatch-diff/create-diff-object.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/upatch-diff/create-diff-object.c b/upatch-diff/create-diff-object.c index 731bbdb1..a5bc1ab0 100644 --- a/upatch-diff/create-diff-object.c +++ b/upatch-diff/create-diff-object.c @@ -937,10 +937,12 @@ static void verify_patchability(struct upatch_elf *uelf) if ((rela->sym == NULL) || (rela->sym->status != CHANGED)) { continue; } - if (!is_string_literal_section(rela->sym->sec)) { - log_warn("Data section '%s' is changed\n", sec->name); - errs++; + if (is_read_only_section(sec) || + is_string_literal_section(rela->sym->sec)) { + continue; } + log_warn("Data section '%s' is changed\n", sec->name); + errs++; } } } -- Gitee From 6b896bfe887d18bac53c2f1d3c3c57d170a6e451 Mon Sep 17 00:00:00 2001 From: renoseven Date: Fri, 25 Apr 2025 10:14:54 +0800 Subject: [PATCH 6/6] upatch-diff: rewrite patch verification logic Signed-off-by: renoseven --- upatch-diff/create-diff-object.c | 102 +++++++++++++++++++------------ 1 file changed, 62 insertions(+), 40 deletions(-) diff --git a/upatch-diff/create-diff-object.c b/upatch-diff/create-diff-object.c index a5bc1ab0..980c1111 100644 --- a/upatch-diff/create-diff-object.c +++ b/upatch-diff/create-diff-object.c @@ -894,64 +894,86 @@ static void include_debug_sections(struct upatch_elf *uelf) /* currently, there si no special section need to be handled */ static void process_special_sections(void) {} -static bool has_tls_included(struct upatch_elf *uelf) +static int verify_symbol_patchability(struct upatch_elf *uelf) { - struct symbol *sym; + int err_count = 0; + struct symbol *sym = NULL; list_for_each_entry(sym, &uelf->symbols, list) { - if (sym->include == 1 && sym->type == STT_TLS) { - log_normal("TLS symbol '%s' is not supported", sym->name); - return true; + if (sym->include == 0) { + continue; + } + if (sym->type == STT_TLS) { + log_warn("Symbol '%s' is included, but TLS is not supported\n", + sym->name); + err_count++; + } + if (sym->type == STT_GNU_IFUNC) { + log_warn("Symbol '%s' is included, but IFUNC is not supported\n", + sym->name); + err_count++; } } - return false; + + return err_count; } -static void verify_patchability(struct upatch_elf *uelf) +static int verify_section_patchability(struct upatch_elf *uelf) { - struct section *sec = NULL; - struct rela *rela = NULL; - int errs = 0; + int err_count = 0; + struct section *sec = NULL; list_for_each_entry(sec, &uelf->sections, list) { - if (sec->status != SAME) { // NEW or CHANGED - if ((sec->include == 0) && !is_rela_section(sec)) { - log_warn("Section '%s' is changed, but it is not included\n", - sec->name); - errs++; - } - if ((sec->grouped != 0) || is_group_section(sec)) { - log_warn("Section '%s' is changed, but it is in a group\n", - sec->name); - errs++; - } - } - if (sec->status != NEW) { // SAME or CHANGED - if (sec->rela == NULL) { + if ((sec->status == NEW) && (sec->include == 0)) { + // new sections should be included + log_warn("Section '%s' is %s, but it is not included\n", + sec->name, status_str(sec->status)); + err_count++; + } else if ((sec->status == CHANGED) && (sec->include == 0)) { + // changed sections should be included, except rela & debug section + if (is_rela_section(sec) || is_debug_section(sec)) { continue; } - if (!is_data_section(sec) && !is_bss_section(sec)) { - continue; + log_warn("Section '%s' is %s, but it is not included\n", + sec->name, status_str(sec->status)); + err_count++; + } else if ((sec->status == CHANGED) && (sec->include != 0)) { + // changed group section cannot be included + if (is_group_section(sec) || (sec->grouped != 0)) { + log_warn("Section '%s' is %s, but it is not supported\n", + sec->name, status_str(sec->status)); + err_count++; } - list_for_each_entry(rela, &sec->rela->relas, list) { - if ((rela->sym == NULL) || (rela->sym->status != CHANGED)) { - continue; - } - if (is_read_only_section(sec) || - is_string_literal_section(rela->sym->sec)) { - continue; + // changed .data & .bss section cannot be included + if (is_data_section(sec) || is_bss_section(sec)) { + struct rela *rela = NULL; + list_for_each_entry(rela, &sec->rela->relas, list) { + if ((rela->sym == NULL) || (rela->sym->status != CHANGED)) { + continue; + } + if (is_read_only_section(rela->sym->sec) || + is_string_literal_section(rela->sym->sec)) { + continue; + } + log_warn("Section '%s' is %s, but it is not supported\n", + sec->name, status_str(sec->status)); + err_count++; } - log_warn("Data section '%s' is changed\n", sec->name); - errs++; } } } - if (errs) { - ERROR("%d, Unsupported section changes", errs); - } - if (has_tls_included(uelf)) { - ERROR("Unsupported symbol included"); + return err_count; +} + +static void verify_patchability(struct upatch_elf *uelf) +{ + int err_count = 0; + + err_count += verify_symbol_patchability(uelf); + err_count += verify_section_patchability(uelf); + if (err_count != 0) { + ERROR("Found %d unexpected changes", err_count); } } -- Gitee