From 884f5ba6a8f3ab830bd9975cf42107a7be88fea7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Guidi?= Date: Fri, 10 Mar 2023 11:22:00 -0500 Subject: [PATCH 01/18] dynamic: Don't patch all functions by default MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Originally, when the user asks to only unpatch functions, uftrace would patch all other functions by default. This is counter-intuitive, especially when using the agent to unpatch at runtime. The user doesn't expect functions to be patched when they only unpatch funtions. This commit removes this behavior, and requires the user to explicitly define functions to patch. Co-authored-by: Gabriel-Andrew Pollo-Guilbert Signed-off-by: Clément Guidi --- libmcount/dynamic.c | 19 +------------------ 1 file changed, 1 insertion(+), 18 deletions(-) diff --git a/libmcount/dynamic.c b/libmcount/dynamic.c index c9c058eb5..c180cc4f5 100644 --- a/libmcount/dynamic.c +++ b/libmcount/dynamic.c @@ -410,7 +410,6 @@ static void parse_pattern_list(char *patch_funcs, char *def_mod, enum uftrace_pa char *name; int j; struct patt_list *pl; - bool all_negative = true; strv_split(&funcs, patch_funcs, ";"); @@ -421,10 +420,8 @@ static void parse_pattern_list(char *patch_funcs, char *def_mod, enum uftrace_pa if (name[0] == '!') name++; - else { + else pl->positive = true; - all_negative = false; - } delim = strchr(name, '@'); if (delim == NULL) { @@ -439,20 +436,6 @@ static void parse_pattern_list(char *patch_funcs, char *def_mod, enum uftrace_pa list_add_tail(&pl->list, &patterns); } - /* prepend match-all pattern, if all patterns are negative */ - if (all_negative) { - pl = xzalloc(sizeof(*pl)); - pl->positive = true; - pl->module = xstrdup(def_mod); - - if (ptype == PATT_REGEX) - init_filter_pattern(ptype, &pl->patt, "."); - else - init_filter_pattern(PATT_GLOB, &pl->patt, "*"); - - list_add(&pl->list, &patterns); - } - strv_free(&funcs); } From fccda9b2af25852298e9dfd989ec55063238b4c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Guidi?= Date: Tue, 21 Mar 2023 10:18:20 -0400 Subject: [PATCH 02/18] dynamic: Don't unpatch unmatched functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Libmcount would try to unpatch by default any function that is not matched by the user patch or unpatch options. We remove this implicit behavior, so the user explicitly choses which function to unpatch. Signed-off-by: Clément Guidi --- libmcount/dynamic.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/libmcount/dynamic.c b/libmcount/dynamic.c index c180cc4f5..aa786bfbd 100644 --- a/libmcount/dynamic.c +++ b/libmcount/dynamic.c @@ -470,11 +470,8 @@ static bool skip_sym(struct uftrace_symbol *sym, struct mcount_dynamic_info *mdi if (sym->type != ST_LOCAL_FUNC && sym->type != ST_GLOBAL_FUNC && sym->type != ST_WEAK_FUNC) return true; - if (!match_pattern_list(map, soname, sym->name)) { - if (mcount_unpatch_func(mdi, sym, &disasm) == 0) - stats.unpatch++; + if (!match_pattern_list(map, soname, sym->name)) return true; - } return false; } From 422713e3b1a2a25bc3e9e5b0e6a863ed352aa536 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Guidi?= Date: Mon, 10 Apr 2023 15:10:21 -0400 Subject: [PATCH 03/18] dynamic: Refactor 'match_pattern_list' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Return whether a symbol is positively or negatively matched against a pattern list, or not matched at all. Co-authored-by: Gabriel-Andrew Pollo-Guilbert Signed-off-by: Clément Guidi --- libmcount/dynamic.c | 45 ++++++++++++++++++++++++++++----------------- 1 file changed, 28 insertions(+), 17 deletions(-) diff --git a/libmcount/dynamic.c b/libmcount/dynamic.c index aa786bfbd..6e7b56dd9 100644 --- a/libmcount/dynamic.c +++ b/libmcount/dynamic.c @@ -384,10 +384,17 @@ static bool match_pattern_module(char *pathname) return ret; } -static bool match_pattern_list(struct uftrace_mmap *map, char *soname, char *sym_name) +/** + * match_pattern_list - match a symbol name against a pattern list + * @map - memory map of the symbol + * @soname - name of the module + * @sym_name - name of the symbol + * @return - -1 if match negative, 1 if match positive, 0 if no match + */ +static int match_pattern_list(struct uftrace_mmap *map, char *soname, char *sym_name) { struct patt_list *pl; - bool ret = false; + int ret = 0; char *libname = basename(map->libname); list_for_each_entry(pl, &patterns, list) { @@ -398,7 +405,7 @@ static bool match_pattern_list(struct uftrace_mmap *map, char *soname, char *sym continue; if (match_filter_pattern(&pl->patt, sym_name)) - ret = pl->positive; + ret = pl->positive ? 1 : -1; } return ret; @@ -470,9 +477,6 @@ static bool skip_sym(struct uftrace_symbol *sym, struct mcount_dynamic_info *mdi if (sym->type != ST_LOCAL_FUNC && sym->type != ST_GLOBAL_FUNC && sym->type != ST_WEAK_FUNC) return true; - if (!match_pattern_list(map, soname, sym->name)) - return true; - return false; } @@ -539,6 +543,7 @@ static void patch_normal_func_matched(struct mcount_dynamic_info *mdi, struct uf unsigned i; struct uftrace_symbol *sym; bool found = false; + int match; char *soname = get_soname(map->libname); symtab = &map->mod->symtab; @@ -548,9 +553,15 @@ static void patch_normal_func_matched(struct mcount_dynamic_info *mdi, struct uf if (skip_sym(sym, mdi, map, soname)) continue; - found = true; - mcount_patch_func_with_stats(mdi, sym); + + match = match_pattern_list(map, soname, sym->name); + if (!match) + continue; + else if (match == 1) + mcount_patch_func_with_stats(mdi, sym); + else + mcount_unpatch_func(mdi, sym, NULL); } if (!found) @@ -826,27 +837,27 @@ TEST_CASE(dynamic_pattern_list) pr_dbg("check simple match with default module\n"); parse_pattern_list("abc;!def", "main", PATT_SIMPLE); - TEST_EQ(match_pattern_list(main_map, NULL, "abc"), true); - TEST_EQ(match_pattern_list(main_map, NULL, "def"), false); - TEST_EQ(match_pattern_list(other_map, NULL, "xyz"), false); + TEST_EQ(match_pattern_list(main_map, NULL, "abc"), 1); + TEST_EQ(match_pattern_list(main_map, NULL, "def"), -1); + TEST_EQ(match_pattern_list(other_map, NULL, "xyz"), 0); release_pattern_list(); pr_dbg("check negative regex match with default module\n"); parse_pattern_list("!^a", "main", PATT_REGEX); - TEST_EQ(match_pattern_list(main_map, NULL, "abc"), false); - TEST_EQ(match_pattern_list(main_map, NULL, "def"), true); - TEST_EQ(match_pattern_list(other_map, NULL, "xyz"), false); + TEST_EQ(match_pattern_list(main_map, NULL, "abc"), -1); + TEST_EQ(match_pattern_list(main_map, NULL, "def"), 0); + TEST_EQ(match_pattern_list(other_map, NULL, "xyz"), 0); release_pattern_list(); pr_dbg("check wildcard match with other module\n"); parse_pattern_list("*@other", "main", PATT_GLOB); - TEST_EQ(match_pattern_list(main_map, NULL, "abc"), false); - TEST_EQ(match_pattern_list(main_map, NULL, "def"), false); - TEST_EQ(match_pattern_list(other_map, NULL, "xyz"), true); + TEST_EQ(match_pattern_list(main_map, NULL, "abc"), 0); + TEST_EQ(match_pattern_list(main_map, NULL, "def"), 0); + TEST_EQ(match_pattern_list(other_map, NULL, "xyz"), 1); release_pattern_list(); From 8b494271bb972197bcf5bb1fe3d110fdd45b817a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Guidi?= Date: Mon, 3 Apr 2023 12:10:55 -0400 Subject: [PATCH 04/18] utils: Implement syscall wrappers for glibc < 2.30 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Glibc < 2.30 doen't provide wrappers for 'gettid()' and 'tgkill()' so we define them. Signed-off-by: Clément Guidi --- utils/utils.c | 26 ++++++++++++++++++++++++++ utils/utils.h | 3 +++ 2 files changed, 29 insertions(+) diff --git a/utils/utils.c b/utils/utils.c index cd49bb8da..c819278d2 100644 --- a/utils/utils.c +++ b/utils/utils.c @@ -6,6 +6,7 @@ #include #include #include +#include #include #ifdef HAVE_LIBUNWIND @@ -1032,6 +1033,31 @@ int copy_file(const char *path_in, const char *path_out) return 0; } +#ifndef gettid +/** + * gettid - gettid syscall wrapper (glibc < 2.30) + * @return - thread id + */ +pid_t gettid() +{ + return syscall(__NR_gettid); +} +#endif + +#ifndef tgkill +/** + * tgkill - tgkill syscall wrapper (glibc < 2.30) + * @tgid - thread group id + * @tid - thread id + * @signal - signal to send + * @return - 0 on success, -1 on error + */ +int tgkill(pid_t tgid, pid_t tid, int signal) +{ + return syscall(SYS_tgkill, tgid, tid, signal); +} +#endif + #ifdef UNIT_TEST TEST_CASE(utils_parse_cmdline) { diff --git a/utils/utils.h b/utils/utils.h index 393def811..08ea94608 100644 --- a/utils/utils.h +++ b/utils/utils.h @@ -431,4 +431,7 @@ void stacktrace(void); int copy_file(const char *path_in, const char *path_out); +pid_t gettid(void); +int tgkill(pid_t tgid, pid_t tid, int signal); + #endif /* UFTRACE_UTILS_H */ From 8a4ddf3158abeb3d9265369c4fb7240247d90fb7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Guidi?= Date: Tue, 4 Apr 2023 09:30:03 -0400 Subject: [PATCH 05/18] utils: Define signal related functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Functions to setup real-time signals and broadcast signals to all threads in an application. Useful for runtime synchronization mechanisms. Co-authored-by: Gabriel-Andrew Pollo-Guilbert Signed-off-by: Clément Guidi --- utils/utils.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++++++ utils/utils.h | 3 ++ 2 files changed, 93 insertions(+) diff --git a/utils/utils.c b/utils/utils.c index c819278d2..4ea00bc81 100644 --- a/utils/utils.c +++ b/utils/utils.c @@ -1058,6 +1058,96 @@ int tgkill(pid_t tgid, pid_t tid, int signal) } #endif +/** + * find_unused_sigrt - find a real-time signal with no associated action + * @return - unused RT signal, or -1 if none found + */ +int find_unused_sigrt() +{ + int sig; + struct sigaction oldact; + + for (sig = SIGRTMIN; sig <= SIGRTMAX; sig++) { + if (sigaction(sig, NULL, &oldact) < 0) { + pr_dbg3("failed to check RT signal handler\n"); + continue; + } + + if (!oldact.sa_handler) + return sig; + } + + pr_dbg2("failed to find unused SIGRT\n"); + return -1; +} + +/** + * thread_broadcast_signal - send a signal to all the other running threads in + * the process + * @sig - signal to send + * @return - number of signals sent, -1 on error + */ +int thread_broadcast_signal(int sig) +{ + char path[32]; + DIR *dir; + struct dirent *dirent; + pid_t pid, tid, task; + int signal_count = 0; + + pid = getpid(); + tid = gettid(); + + snprintf(path, 32, "/proc/%u/task", pid); + dir = opendir(path); + if (!dir) { + pr_dbg("failed to open directory '%s'\n", path); + goto fail_open_dir; + } + + errno = 0; + for (dirent = readdir(dir); dirent != NULL; dirent = readdir(dir)) { + /* skip "." and ".." directories */ + if (dirent->d_name[0] == '.') + continue; + + task = strtol(dirent->d_name, NULL, 10); + if (errno != 0 || task < 0) { + pr_dbg("failed to parse TID '%s'\n", dirent->d_name); + continue; + } + + /* ignore our TID */ + if (task == tid) + continue; + + /* + * By reading /proc//task directory, there is the possibility of + * a race condition where a thread exits before we send the signal. + */ + pr_dbg4("send SIGRT%d to %u\n", sig, task); + if (tgkill(pid, task, sig) < 0) { + if (errno != ESRCH) { + pr_dbg2("cannot signal thread %u: %s\n", task, strerror(errno)); + errno = 0; + } + } + else + signal_count++; + } + + if (errno != 0) + pr_dbg("failed to read directory entry\n"); + + if (closedir(dir) < 0) + pr_dbg2("failed to close directory\n"); + + return signal_count; + +fail_open_dir: + return -1; +} + #ifdef UNIT_TEST TEST_CASE(utils_parse_cmdline) { diff --git a/utils/utils.h b/utils/utils.h index 08ea94608..9f20bfc97 100644 --- a/utils/utils.h +++ b/utils/utils.h @@ -434,4 +434,7 @@ int copy_file(const char *path_in, const char *path_out); pid_t gettid(void); int tgkill(pid_t tgid, pid_t tid, int signal); +int find_unused_sigrt(void); +int thread_broadcast_signal(int sig); + #endif /* UFTRACE_UTILS_H */ From 3eca7890479e0639c63099e3ef52b81886d7b982 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Guidi?= Date: Wed, 8 Mar 2023 14:45:32 -0500 Subject: [PATCH 06/18] dynamic: x86_64: Skip already patched functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Skip the functions where uftrace already injected a call to a trampoline. Signed-off-by: Clément Guidi --- arch/x86_64/mcount-insn.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/arch/x86_64/mcount-insn.c b/arch/x86_64/mcount-insn.c index c8ab6105a..d407717e2 100644 --- a/arch/x86_64/mcount-insn.c +++ b/arch/x86_64/mcount-insn.c @@ -590,8 +590,11 @@ int disasm_check_insns(struct mcount_disasm_engine *disasm, struct mcount_dynami cs_insn *insn = NULL; uint32_t count, i, size; uint8_t endbr64[] = { 0xf3, 0x0f, 0x1e, 0xfa }; + void *trampoline_addr; + uint8_t *operand; struct dynamic_bad_symbol *badsym; unsigned long addr = info->addr; + bool is_call, is_trampoline; badsym = mcount_find_badsym(mdi, info->addr); if (badsym != NULL) { @@ -623,6 +626,20 @@ int disasm_check_insns(struct mcount_disasm_engine *disasm, struct mcount_dynami if (count == 0) return INSTRUMENT_FAILED; + /* Check for pre-existing dynamic instrumentation + 1. check if opcode is call + 2. check operand is trampoline address */ + operand = &((uint8_t *)info->addr)[1]; + trampoline_addr = (void *)mdi->trampoline - (info->addr + CALL_INSN_SIZE); + is_call = ((uint8_t *)info->addr)[0] == 0xe8; + if (is_call) { + is_trampoline = !memcmp(operand, &trampoline_addr, CALL_INSN_SIZE - 1); + if (is_trampoline) { + pr_dbg2("skip dynamically patched func: %s\n", info->sym->name); + return INSTRUMENT_SKIPPED; + } + } + for (i = 0; i < count; i++) { uint8_t insns_byte[32] = { 0, From efc15f63bbe83414e64e8e2227b35fc528a9bcd9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Guidi?= Date: Tue, 4 Apr 2023 09:42:10 -0400 Subject: [PATCH 07/18] dynamic: x86_64: Refactor 'get_target_address' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refactor for more clarity. Signed-off-by: Clément Guidi --- arch/x86_64/mcount-dynamic.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/arch/x86_64/mcount-dynamic.c b/arch/x86_64/mcount-dynamic.c index a14642287..268855247 100644 --- a/arch/x86_64/mcount-dynamic.c +++ b/arch/x86_64/mcount-dynamic.c @@ -247,9 +247,15 @@ void mcount_arch_find_module(struct mcount_dynamic_info *mdi, struct uftrace_sym elf_finish(&elf); } -static unsigned long get_target_addr(struct mcount_dynamic_info *mdi, unsigned long addr) +/** + * get_trampoline_offest - compute the relative address of the trampoline + * @mdi - mcount dynamic info + * @origin - origin address + * @return - distance to the trampoline + */ +static unsigned long get_trampoline_offset(struct mcount_dynamic_info *mdi, unsigned long origin) { - return mdi->trampoline - (addr + CALL_INSN_SIZE); + return mdi->trampoline - (origin + CALL_INSN_SIZE); } static int patch_fentry_code(struct mcount_dynamic_info *mdi, struct uftrace_symbol *sym) @@ -271,7 +277,7 @@ static int patch_fentry_code(struct mcount_dynamic_info *mdi, struct uftrace_sym } /* get the jump offset to the trampoline */ - target_addr = get_target_addr(mdi, (unsigned long)insn); + target_addr = get_trampoline_offset(mdi, (unsigned long)insn); if (target_addr == 0) return INSTRUMENT_SKIPPED; @@ -429,14 +435,14 @@ static void patch_code(struct mcount_dynamic_info *mdi, struct mcount_disasm_inf { void *origin_code_addr; unsigned char call_insn[] = { 0xe8, 0x00, 0x00, 0x00, 0x00 }; - uint32_t target_addr = get_target_addr(mdi, info->addr); + uint32_t target_addr = get_trampoline_offset(mdi, info->addr); /* patch address */ origin_code_addr = (void *)info->addr; if (info->has_intel_cet) { origin_code_addr += ENDBR_INSN_SIZE; - target_addr = get_target_addr(mdi, info->addr + ENDBR_INSN_SIZE); + target_addr = get_trampoline_offset(mdi, info->addr + ENDBR_INSN_SIZE); } /* build the instrumentation instruction */ From 3c86dea0e7e162774dc93bb2ef8b00e5ff520260 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Guidi?= Date: Tue, 4 Apr 2023 09:47:13 -0400 Subject: [PATCH 08/18] dynamic: x86_64: Refactor 'patch_code' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refactor 'patch_code' so it can later be used at runtime. Co-authored-by: Gabriel-Andrew Pollo-Guilbert Signed-off-by: Clément Guidi --- arch/x86_64/mcount-dynamic.c | 119 +++++++++++++---------------------- 1 file changed, 45 insertions(+), 74 deletions(-) diff --git a/arch/x86_64/mcount-dynamic.c b/arch/x86_64/mcount-dynamic.c index 268855247..2047de025 100644 --- a/arch/x86_64/mcount-dynamic.c +++ b/arch/x86_64/mcount-dynamic.c @@ -386,93 +386,66 @@ static int patch_xray_func(struct mcount_dynamic_info *mdi, struct uftrace_symbo return ret; } -/* - * we overwrite instructions over 5bytes from start of function - * to call '__dentry__' that seems similar like '__fentry__'. - * - * while overwriting, After adding the generated instruction which - * returns to the address of the original instruction end, - * save it in the heap. - * - * for example: - * - * 4005f0: 31 ed xor %ebp,%ebp - * 4005f2: 49 89 d1 mov %rdx,%r9 - * 4005f5: 5e pop %rsi - * - * will changed like this : - * - * 4005f0 call qword ptr [rip + 0x200a0a] # 0x601000 - * - * and keeping original instruction : - * - * Original Instructions--------------- - * f1cff0: xor ebp, ebp - * f1cff2: mov r9, rdx - * f1cff5: pop rsi - * Generated Instruction to return----- - * f1cff6: jmp qword ptr [rip] - * f1cffc: QW 0x00000000004005f6 - * - * In the original case, address 0x601000 has a dynamic symbol - * start address. It is also the first element in the GOT array. - * while initializing the mcount library, we will replace it with - * the address of the function '__dentry__'. so, the changed - * instruction will be calling '__dentry__'. - * - * '__dentry__' has a similar function like '__fentry__'. - * the other thing is that it returns to original instructions - * we keeping. it makes it possible to execute the original - * instructions and return to the address at the end of the original - * instructions. Thus, the execution will goes on. - * - */ - -/* - * Patch the instruction to the address as given for arguments. +/** + * patch_code - inject a call to an instrumentation trampoline + * @mdi - dynamic info about the concerned module + * @info - disassembly info (instructions address and size) + * @return - instrumentation status */ -static void patch_code(struct mcount_dynamic_info *mdi, struct mcount_disasm_info *info) +static int patch_code(struct mcount_dynamic_info *mdi, struct mcount_disasm_info *info) { + /* + * Let's assume we have the following instructions. + * + * 0x0: push %rbp <= origin_code_addr + * 0x1: mov %rsp,%rbp + * 0x4: lea 0xeb0(%rip),%rdi + * 0xb: + * + * The goal is to modify the instructions in order to get the following + * ones. + * + * 0x0: call + * 0x5: + * 0xb: + */ + void *origin_code_addr; - unsigned char call_insn[] = { 0xe8, 0x00, 0x00, 0x00, 0x00 }; - uint32_t target_addr = get_trampoline_offset(mdi, info->addr); + void *trampoline_rel_addr; - /* patch address */ origin_code_addr = (void *)info->addr; - - if (info->has_intel_cet) { + if (info->has_intel_cet) origin_code_addr += ENDBR_INSN_SIZE; - target_addr = get_trampoline_offset(mdi, info->addr + ENDBR_INSN_SIZE); - } - /* build the instrumentation instruction */ - memcpy(&call_insn[1], &target_addr, CALL_INSN_SIZE - 1); + trampoline_rel_addr = (void *)get_trampoline_offset(mdi, (unsigned long)origin_code_addr); /* - * we need 5-bytes at least to instrumentation. however, - * if instructions is not fit 5-bytes, we will overwrite the - * 5-bytes and fill the remaining part of the last - * instruction with nop. * - * [example] - * In this example, we overwrite 9-bytes to use 5-bytes. * - * dynamic: 0x19e98b0[01]:push rbp - * dynamic: 0x19e98b1[03]:mov rbp, rsp - * dynamic: 0x19e98b4[05]:mov edi, 0x4005f4 * - * dynamic: 0x40054c[05]:call 0x400ff0 - * dynamic: 0x400551[01]:nop - * dynamic: 0x400552[01]:nop - * dynamic: 0x400553[01]:nop - * dynamic: 0x400554[01]:nop */ - memcpy(origin_code_addr, call_insn, CALL_INSN_SIZE); + /* + * We fill the remaining part of the patching region with nops. + * + * 0x0: int3 + * 0x1: + * 0x5: + * 0xb: + */ + + memcpy(&((uint8_t *)origin_code_addr)[1], &trampoline_rel_addr, CALL_INSN_SIZE - 1); memset(origin_code_addr + CALL_INSN_SIZE, 0x90, /* NOP */ info->orig_size - CALL_INSN_SIZE); - /* flush icache so that cpu can execute the new insn */ - __builtin___clear_cache(origin_code_addr, origin_code_addr + info->orig_size); + /* + * 0x0: call + * 0x5: + * 0xb: + */ + + ((uint8_t *)origin_code_addr)[0] = 0xe8; + + return INSTRUMENT_SUCCESS; } static int patch_normal_func(struct mcount_dynamic_info *mdi, struct uftrace_symbol *sym, @@ -522,9 +495,7 @@ static int patch_normal_func(struct mcount_dynamic_info *mdi, struct uftrace_sym else mcount_save_code(&info, call_offset, jmp_insn, sizeof(jmp_insn)); - patch_code(mdi, &info); - - return INSTRUMENT_SUCCESS; + return patch_code(mdi, &info); } static int unpatch_func(uint8_t *insn, char *name) From a52ccc52731fd03523bd48fa12ec8c301bac3f9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Guidi?= Date: Mon, 10 Apr 2023 12:27:34 -0400 Subject: [PATCH 09/18] dynamic: x86_64: Factor out 'check_endbr64' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Check if instruction at a given address is ENDBR64. Signed-off-by: Clément Guidi --- arch/x86_64/mcount-insn.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/arch/x86_64/mcount-insn.c b/arch/x86_64/mcount-insn.c index d407717e2..a9a829b9a 100644 --- a/arch/x86_64/mcount-insn.c +++ b/arch/x86_64/mcount-insn.c @@ -583,13 +583,24 @@ static bool check_unsupported(struct mcount_disasm_engine *disasm, cs_insn *insn return true; } +/** + * check_endbr64 - check if instruction at @addr is endbr64 + * @addr - address to check for endbr64 + * @return - 1 if found, 0 if not + */ +int check_endbr64(unsigned long addr) +{ + uint8_t endbr64[] = { 0xf3, 0x0f, 0x1e, 0xfa }; + + return !memcmp((void *)addr, endbr64, sizeof(endbr64)); +} + int disasm_check_insns(struct mcount_disasm_engine *disasm, struct mcount_dynamic_info *mdi, struct mcount_disasm_info *info) { int status; cs_insn *insn = NULL; uint32_t count, i, size; - uint8_t endbr64[] = { 0xf3, 0x0f, 0x1e, 0xfa }; void *trampoline_addr; uint8_t *operand; struct dynamic_bad_symbol *badsym; @@ -612,9 +623,9 @@ int disasm_check_insns(struct mcount_disasm_engine *disasm, struct mcount_dynami return INSTRUMENT_SKIPPED; size = info->sym->size; - if (!memcmp((void *)info->addr, endbr64, sizeof(endbr64))) { - addr += sizeof(endbr64); - size -= sizeof(endbr64); + if (check_endbr64(info->addr)) { + addr += ENDBR_INSN_SIZE; + size -= ENDBR_INSN_SIZE; if (size <= CALL_INSN_SIZE) return INSTRUMENT_SKIPPED; From 550af6aed97cb650f32751dd456a30d7a3e28758 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Guidi?= Date: Tue, 4 Apr 2023 09:55:11 -0400 Subject: [PATCH 10/18] dynamic: x86_64: Protect patch zone with trap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When patching a function at runtime, first insert an int3 trap so any incoming thread is diverted from the patching region, to avoid executing partially modified code. The trap handler emulates a call to the trampoline, thus enabling the instrumentation. The trap is eventually removed in subsequent commits. Co-authored-by: Gabriel-Andrew Pollo-Guilbert Signed-off-by: Clément Guidi --- arch/x86_64/mcount-dynamic.c | 136 ++++++++++++++++++++++++++++++++++- 1 file changed, 134 insertions(+), 2 deletions(-) diff --git a/arch/x86_64/mcount-dynamic.c b/arch/x86_64/mcount-dynamic.c index 2047de025..ef10bf743 100644 --- a/arch/x86_64/mcount-dynamic.c +++ b/arch/x86_64/mcount-dynamic.c @@ -9,6 +9,7 @@ #include "libmcount/dynamic.h" #include "libmcount/internal.h" #include "mcount-arch.h" +#include "utils/hashmap.h" #include "utils/symbol.h" #include "utils/utils.h" @@ -19,6 +20,125 @@ static const unsigned char patchable_clang_nop[] = { 0x0f, 0x1f, 0x44, 0x00, 0x0 static const unsigned char endbr64[] = { 0xf3, 0x0f, 0x1e, 0xfa }; +/* + * Hashmap of trampoline locations for every installed int3 trap. The traps are + * used when patching and unpatching functions: they prevent threads from + * entering the critical zone. When a trap is reached, the trap handler fetches + * the trampoline location from the hmap, and emulates a call to the trampoline, + * to mimic regular instrumentation. + * + * (void *int3_location -> void *trampoline) + */ +static struct Hashmap *int3_hmap; + +/** + * register_trap - save trampoline associated to a trap + * @trap - trap address + * @call_loc - address of the symbol to emulate a call to + * @return - -1 on error, 0 on success + */ +static int register_trap(void *trap, void *call_loc) +{ + if (!hashmap_put(int3_hmap, trap, call_loc)) + return -1; + + return 0; +} + +/** + * unregister_trap - remove trap entry from hmap + * @trap - trap address + * @return - -1 on error, 0 on success + */ +static int unregister_trap(void *trap) +{ + if (hashmap_remove(int3_hmap, trap)) + return 0; + + return -1; +} + +/** + * emulate_trampoline_call - SIGTRAP handler, emulates a call to the trampoline + * associated with the trap location + * + * When the trap handler is executed, it changes the program counter to point to + * . When the trap handler exits, the code at will + * execute (which is __dentry__ defined in dynamic.S). + * + * As __dentry__ is expected to be called like a function, it depends on the + * address of the caller to know which tracepoint was executed. The address is + * expected to be found on the stack. Therefore, the trap handler actually needs + * to emulate a call instruction entirely (moving the instruction pointer is not + * enough). + * + * To do so, the trap handler will also push on the stack the next instruction + * pointer that would be used if the executed instruction was a call instead of + * a trap. + * + * @sig - signal caught + * @info - (unused) + * @ucontext - user context, containing registers + */ +static void emulate_trampoline_call(int sig, siginfo_t *info, void *ucontext) +{ + ucontext_t *uctx = ucontext; + mcontext_t *mctx = &uctx->uc_mcontext; + void *int3_addr; + unsigned long trampoline; + unsigned long child_addr; /* probe location for mcount_entry */ + + ASSERT(sig == SIGTRAP); + + __atomic_signal_fence(__ATOMIC_SEQ_CST); + int3_addr = (void *)mctx->gregs[REG_RIP] - 1; /* int3 size = 1 */ + trampoline = (unsigned long)hashmap_get(int3_hmap, int3_addr); + + child_addr = (unsigned long)int3_addr + CALL_INSN_SIZE; + + mctx->gregs[REG_RSP] -= 8; + memcpy((void *)mctx->gregs[REG_RSP], &child_addr, 8); + mctx->gregs[REG_RIP] = trampoline; +} + +/** + * configure_sigtrap_handler - emulate call to trampoline on SIGTRAP + * @return - -1 on failure, 0 on success + */ +static int configure_sigtrap_handler(void) +{ + struct sigaction act; + + act.sa_sigaction = emulate_trampoline_call; + act.sa_flags = SA_SIGINFO; + + if (sigaction(SIGTRAP, &act, NULL) < 0) { + pr_err("failed to configure SIGTRAP handler\n"); + return -1; + } + + pr_dbg2("configured SIGTRAP handler\n"); + return 0; +} + +/** + * mcount_arch_dynamic_init - initialize arch-specific data structures to + * perform runtime dynamic instrumentation + */ +int mcount_arch_dynamic_init(void) +{ + if (!int3_hmap) { + int3_hmap = hashmap_create(4, hashmap_ptr_hash, hashmap_ptr_equals); + if (!int3_hmap) { + pr_dbg("failed to create int3 hashmap\n"); + return -1; + } + } + if (configure_sigtrap_handler() < 0) + return -1; + + return 0; +} int mcount_setup_trampoline(struct mcount_dynamic_info *mdi) { unsigned char trampoline[] = { 0x3e, 0xff, 0x25, 0x01, 0x00, 0x00, 0x00, 0xcc }; @@ -419,11 +539,22 @@ static int patch_code(struct mcount_dynamic_info *mdi, struct mcount_disasm_info trampoline_rel_addr = (void *)get_trampoline_offset(mdi, (unsigned long)origin_code_addr); - /* - * + /* The first step is to insert a 1-byte trap-based probe point (atomic). + * This will prevent threads to enter the critical zone while we patch it, + * so no core will see partial modifications. * + * 0x0: int3 <= origin_code_addr + * 0x1: mov %rsp,%rbp + * 0x4: lea 0xeb0(%rip),%rdi + * 0xb: * + * The trap will emulate a call to the trampoline while in place. */ + + if (register_trap(origin_code_addr, (void *)mdi->trampoline) == -1) + return INSTRUMENT_FAILED; + ((uint8_t *)origin_code_addr)[0] = 0xcc; + /* * We fill the remaining part of the patching region with nops. * @@ -444,6 +575,7 @@ static int patch_code(struct mcount_dynamic_info *mdi, struct mcount_disasm_info */ ((uint8_t *)origin_code_addr)[0] = 0xe8; + unregister_trap(origin_code_addr); return INSTRUMENT_SUCCESS; } From f94c9fe8bb9d6445a77c4acb923e7462169dc8b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Guidi?= Date: Tue, 4 Apr 2023 10:02:53 -0400 Subject: [PATCH 11/18] dynamic: x86_64: Synchronize cores when patching MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When cross-modifying code, the software needs to ensure that all cores will execute valid instructions at any time. When modifications are not atomic, we issue a specific memory barrier (or execute a serializing instruction on Linux < 4.16) to serialize the execution across all cores. This flushes the different caches, especially the processor pipelines that may have partially fetched straddling instructions. Co-authored-by: Gabriel-Andrew Pollo-Guilbert Signed-off-by: Clément Guidi --- arch/x86_64/mcount-dynamic.c | 131 +++++++++++++++++++++++++++++++++++ 1 file changed, 131 insertions(+) diff --git a/arch/x86_64/mcount-dynamic.c b/arch/x86_64/mcount-dynamic.c index ef10bf743..e6c2f0db9 100644 --- a/arch/x86_64/mcount-dynamic.c +++ b/arch/x86_64/mcount-dynamic.c @@ -1,7 +1,15 @@ +#include #include #include #include +#if HAVE_MEMBARRIER +#include +#else +#include +#include +#endif + /* This should be defined before #include "utils.h" */ #define PR_FMT "dynamic" #define PR_DOMAIN DBG_DYNAMIC @@ -121,6 +129,120 @@ static int configure_sigtrap_handler(void) return 0; } +#if HAVE_MEMBARRIER + +/** + * setup_synchronization_mechanism - register intent to use the 'private + * expedited sync core' membarrier to synchronize instruction pipelines and + * caches across cores, for safe cross-modification. + * @return - negative on error, 0 on success + */ +static int setup_synchronization_mechanism(void) +{ + int ret = + syscall(__NR_membarrier, MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED_SYNC_CORE, 0, 0); + if (ret < 0) + pr_dbg2("failed to register membarrier intent: %s\n", strerror(errno)); + return ret; +} + +/** + * synchronize_all_cores - use membarrier to perform cache and pipeline + * synchronization across cores executing cross-modified code + * @return - negative on error, 0 on success + */ +static int synchronize_all_cores(void) +{ + int ret = syscall(__NR_membarrier, MEMBARRIER_CMD_PRIVATE_EXPEDITED_SYNC_CORE, 0, 0); + if (ret < 0) + pr_dbg2("failed to use membarrier: %s\n", strerror(errno)); + return ret; +} + +#else /* HAVE_MEMBARRIER */ + +/* signal used to perform cache and pipeline synchronization across cores */ +static int sig_sync_cores; + +/* counter for the threads that have performed serialization when a signal is + issued */ +static sem_t sem_sync_cores; + +/** + * serialize_instruction_execution - execute core serialize instruction + * + * According to Intel manual, CPUID is a serializing instruction. + */ +static void serialize_instruction_execution(int signum, siginfo_t *info, void *arg) +{ + int _; + __cpuid(_, _, _, _, _); + sem_post(&sem_sync_cores); +} + +/** + * setup_synchronization_mechanism - setup real-time signal and its handler to + * perform core synchronization across all threads + * @return - 0 on success, -1 on failure + */ +static int setup_synchronization_mechanism(void) +{ + struct sigaction act; + + if (sig_sync_cores > 0) + return 0; + + sig_sync_cores = find_unused_sigrt(); + if (sig_sync_cores == -1) + return -1; + + sem_init(&sem_sync_cores, 0, 0); + + act.sa_sigaction = serialize_instruction_execution; + act.sa_flags = 0; + + if (sigaction(sig_sync_cores, &act, NULL) < 0) { + pr_dbg("failed to configure core synchronization signal handler\n"); + return -1; + } + + pr_dbg("configured core synchronization signal (SIGRT%d) handler\n", sig_sync_cores); + return 0; +} + +/** + * serialize_instruction_cache - send RT signals to perform cache and pipeline + * synchronization across cores executing cross-modified code. + * @return - -1 on error, 0 on success + */ +static int synchronize_all_cores(void) +{ + int signal_count; + int sync_count = 0; + struct timespec ts; + + ASSERT(sig_sync_cores >= SIGRTMIN); + + signal_count = thread_broadcast_signal(sig_sync_cores); + + if (clock_gettime(CLOCK_REALTIME, &ts) == -1) + return -1; + ts.tv_sec += 1; + + for (int i = 0; i < signal_count; i++) { + if (sem_timedwait(&sem_sync_cores, &ts) == -1) { + if (errno == EINTR) + i--; + } + else + sync_count++; + } + pr_dbg3("synced core in %d/%d thread(s)\n", sync_count, signal_count); + + return 0; +} + +#endif /* HAVE_MEMBARRIER */ /** * mcount_arch_dynamic_init - initialize arch-specific data structures to * perform runtime dynamic instrumentation @@ -137,8 +259,12 @@ int mcount_arch_dynamic_init(void) if (configure_sigtrap_handler() < 0) return -1; + if (setup_synchronization_mechanism() < 0) + return -1; + return 0; } + int mcount_setup_trampoline(struct mcount_dynamic_info *mdi) { unsigned char trampoline[] = { 0x3e, 0xff, 0x25, 0x01, 0x00, 0x00, 0x00, 0xcc }; @@ -549,12 +675,17 @@ static int patch_code(struct mcount_dynamic_info *mdi, struct mcount_disasm_info * 0xb: * * The trap will emulate a call to the trampoline while in place. + * + * We ensure that every core sees the trap before patching the critical + * zone, by synchronizing the them. */ if (register_trap(origin_code_addr, (void *)mdi->trampoline) == -1) return INSTRUMENT_FAILED; ((uint8_t *)origin_code_addr)[0] = 0xcc; + synchronize_all_cores(); + /* * We fill the remaining part of the patching region with nops. * From c4929b8c56d4c1a553e61b54b59fd4d7cd6afa2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Guidi?= Date: Tue, 4 Apr 2023 10:12:38 -0400 Subject: [PATCH 12/18] dynamic: x86_64: Move threads out of patching zone MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When patching at runtime, no thread can enter the patching region due to the trap that is inserted at the start of it. But threads that entered the region before the trap is installed can still be executing instructions in the region. We broadcast a real-time signal instruction all threads to check their instruction pointer, and execute out-of-line if they are in the patching region. Co-authored-by: Gabriel-Andrew Pollo-Guilbert Signed-off-by: Clément Guidi --- arch/x86_64/mcount-dynamic.c | 180 ++++++++++++++++++++++++++++++++++- libmcount/dynamic.c | 1 + 2 files changed, 180 insertions(+), 1 deletion(-) diff --git a/arch/x86_64/mcount-dynamic.c b/arch/x86_64/mcount-dynamic.c index e6c2f0db9..6e0a6f07a 100644 --- a/arch/x86_64/mcount-dynamic.c +++ b/arch/x86_64/mcount-dynamic.c @@ -1,4 +1,5 @@ #include +#include #include #include #include @@ -7,7 +8,6 @@ #include #else #include -#include #endif /* This should be defined before #include "utils.h" */ @@ -39,6 +39,22 @@ static const unsigned char endbr64[] = { 0xf3, 0x0f, 0x1e, 0xfa }; */ static struct Hashmap *int3_hmap; +/* Hashmap of the addresses of instructions that are in patching zones and need + * to be executed out of line. The addresses are mapped to their out of line + * equivalent. + * + * (void *critical_insn -> void *out_of_line_insn) + */ +static struct Hashmap *patch_region_hmap; + +/* Realtime signal number to instruct running threads to move out of patching + regions */ +static int sig_clear_patch_region; + +/* counter for the threads that are guaranteed to be out of registered patching + regions when a signal is issued */ +static sem_t sem_clear_patch_region; + /** * register_trap - save trampoline associated to a trap * @trap - trap address @@ -233,6 +249,8 @@ static int synchronize_all_cores(void) if (sem_timedwait(&sem_sync_cores, &ts) == -1) { if (errno == EINTR) i--; + else + pr_dbg3("error syncing with signal handler: %s\n", strerror(errno)); } else sync_count++; @@ -243,6 +261,143 @@ static int synchronize_all_cores(void) } #endif /* HAVE_MEMBARRIER */ + +/** + * register_patch_region - mark a memory region as critical by registering the + * addresses that it contains in 'patch_region_hmap' + * @start - address of the patch region + * @len - length of the patch region + * @return - -1 on error, 0 on success + */ +static int register_patch_region(void *start, int len) +{ + void *out_of_line_buffer = mcount_find_code((unsigned long)start + CALL_INSN_SIZE); + if (!out_of_line_buffer) + return -1; + + for (int i = 0; i < len; i++) { + if (!hashmap_put(patch_region_hmap, start + i, out_of_line_buffer + i)) + return -1; + } + + return 0; +} + +/** + * unregister_patch_region - unmark a memory region as critical + * @start - address of the patch region + * @len - length of the patch region + * @return - -1 on error, 0 on success + */ +static int unregister_patch_region(void *start, int len) +{ + void *out_of_line_buffer = mcount_find_code((unsigned long)start); + if (!out_of_line_buffer) + return -1; + + for (int i = 0; i < len; i++) { + if (!hashmap_remove(patch_region_hmap, start + i)) + return -1; + } + + return 0; +} + +/** + * leave_patch_region - signal handler on which a thread executes out of line if + * it happens to be in a registered patching region + * @sig - signal number + * @info - signal info (unused) + * @ucontext - user context + */ +static void leave_patch_region(int sig, siginfo_t *info, void *ucontext) +{ + ucontext_t *uctx = ucontext; + mcontext_t *mctx = &uctx->uc_mcontext; + void *next_insn; + void *out_of_line_insn; + (void)sig; + + next_insn = (void *)mctx->gregs[REG_RIP]; + out_of_line_insn = hashmap_get(patch_region_hmap, next_insn); + if (out_of_line_insn) + mctx->gregs[REG_RIP] = (uint64_t)out_of_line_insn; + + sem_post(&sem_clear_patch_region); +} + +/** + * clear_patch_region - move threads that are in a patching region out of line + * @return - 0 + */ +static int clear_patch_region(void) +{ + int signal_count; + int move_count = 0; + struct timespec ts; + + ASSERT(sig_clear_patch_region >= SIGRTMIN); + + signal_count = thread_broadcast_signal(sig_clear_patch_region); + + if (clock_gettime(CLOCK_REALTIME, &ts) == -1) + return -1; + ts.tv_sec += 1; + + for (int i = 0; i < signal_count; i++) { + if (sem_timedwait(&sem_clear_patch_region, &ts) == -1) { + if (errno == EINTR) + i--; + else + pr_dbg3("error syncing with signal handler: %s\n", strerror(errno)); + } + else + move_count++; + } + pr_dbg3("checked ip of %d/%d thread(s)\n", move_count, signal_count); + + return 0; +} + +/** + * setup_clear_patch_region - initialize data structures and signals used to + * move threads of patching regions + * @return - -1 on error, 0 on success + */ +int setup_clear_patch_region(void) +{ + struct sigaction act; + + if (!patch_region_hmap) { + patch_region_hmap = hashmap_create(4, hashmap_ptr_hash, hashmap_ptr_equals); + if (!patch_region_hmap) { + pr_dbg("failed to create patch region hashmap\n"); + return -1; + } + } + + if (sig_clear_patch_region > 0) + return 0; + + sig_clear_patch_region = find_unused_sigrt(); + if (sig_clear_patch_region == -1) + return -1; + + sem_init(&sem_clear_patch_region, 0, 0); + + act.sa_sigaction = leave_patch_region; + act.sa_flags = 0; + + if (sigaction(sig_clear_patch_region, &act, NULL) < 0) { + pr_dbg("failed to configure clear signal (SIGRT%d) handler\n", + sig_clear_patch_region); + return -1; + } + + pr_dbg("configured clear signal (SIGRT%d) handler\n", sig_clear_patch_region); + return 0; +} + /** * mcount_arch_dynamic_init - initialize arch-specific data structures to * perform runtime dynamic instrumentation @@ -262,6 +417,8 @@ int mcount_arch_dynamic_init(void) if (setup_synchronization_mechanism() < 0) return -1; + setup_clear_patch_region(); + return 0; } @@ -687,6 +844,24 @@ static int patch_code(struct mcount_dynamic_info *mdi, struct mcount_disasm_info synchronize_all_cores(); /* + * The second step is to move any thread out of the critical zone if still + * present. Threads in the critical zone resume execution out of line, in + * their dedicated OLX region. + * + * The method used to move the threads is to signal all the threads, so they + * check if their instruction pointer is in the patching region. If so, they + * move their instruction pointer to the corresponding one in the OLX + * region. + */ + + if (register_patch_region(origin_code_addr, info->orig_size) == -1) + pr_dbg3("failed to register patch region\n"); + clear_patch_region(); + unregister_patch_region(origin_code_addr, info->orig_size); + + /* The third step is to write the target address of the call. From the + * processor view the 4-bytes address can be any garbage instructions. + * * We fill the remaining part of the patching region with nops. * * 0x0: int3 @@ -698,8 +873,11 @@ static int patch_code(struct mcount_dynamic_info *mdi, struct mcount_disasm_info memcpy(&((uint8_t *)origin_code_addr)[1], &trampoline_rel_addr, CALL_INSN_SIZE - 1); memset(origin_code_addr + CALL_INSN_SIZE, 0x90, /* NOP */ info->orig_size - CALL_INSN_SIZE); + /* FIXME Need to sync cores? Store membarrier? */ /* + * The fourth and last step is to replace the trap with the call opcode. + * * 0x0: call * 0x5: * 0xb: diff --git a/libmcount/dynamic.c b/libmcount/dynamic.c index 6e7b56dd9..2d181b359 100644 --- a/libmcount/dynamic.c +++ b/libmcount/dynamic.c @@ -52,6 +52,7 @@ struct code_page { static LIST_HEAD(code_pages); +/* contains out-of-line execution code (return address -> modified instructions ptr) */ static struct Hashmap *code_hmap; /* minimum function size for dynamic update */ From c826911b256b239ec0609a1e55e21926cc41d454 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Guidi?= Date: Mon, 10 Apr 2023 08:52:33 -0400 Subject: [PATCH 13/18] dynamic: Batch function patching process MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Synchronization requires sending signals to threads inside the process. This is performed for every symbols, which means a lot of signal can be sent. This has a major performance impact. This commit batches the initial and final steps of the patching process so we only need to send one signal per thread for every batch. Co-authored-by: Gabriel-Andrew Pollo-Guilbert Signed-off-by: Clément Guidi --- arch/x86_64/mcount-dynamic.c | 179 +++++++++++++++++++++++++++-------- libmcount/dynamic.c | 6 ++ 2 files changed, 147 insertions(+), 38 deletions(-) diff --git a/arch/x86_64/mcount-dynamic.c b/arch/x86_64/mcount-dynamic.c index 6e0a6f07a..e6471f630 100644 --- a/arch/x86_64/mcount-dynamic.c +++ b/arch/x86_64/mcount-dynamic.c @@ -327,10 +327,10 @@ static void leave_patch_region(int sig, siginfo_t *info, void *ucontext) } /** - * clear_patch_region - move threads that are in a patching region out of line + * clear_patch_regions - move threads that are in a patching region out of line * @return - 0 */ -static int clear_patch_region(void) +static int clear_patch_regions(void) { int signal_count; int move_count = 0; @@ -398,6 +398,32 @@ int setup_clear_patch_region(void) return 0; } +/* This list is used to store functions that need to be optimized or cleaned up + * later in the code. In both case, a SIGRTMIN+n must be send. By optimizing or + * cleaning all of them up at the same time, we only need to send one signal per + * thread. + */ +LIST_HEAD(normal_funcs_patch); + +struct patch_dynamic_info { + struct list_head list; + struct mcount_dynamic_info *mdi; + struct mcount_disasm_info *info; +}; + +static void commit_normal_func(struct list_head *list, struct mcount_dynamic_info *mdi, + struct mcount_disasm_info *info) +{ + struct patch_dynamic_info *pdi; + pdi = xmalloc(sizeof(*pdi)); + + pdi->mdi = mdi; + pdi->info = info; + INIT_LIST_HEAD(&pdi->list); + + list_add(&pdi->list, list); +} + /** * mcount_arch_dynamic_init - initialize arch-specific data structures to * perform runtime dynamic instrumentation @@ -790,12 +816,14 @@ static int patch_xray_func(struct mcount_dynamic_info *mdi, struct uftrace_symbo } /** - * patch_code - inject a call to an instrumentation trampoline - * @mdi - dynamic info about the concerned module - * @info - disassembly info (instructions address and size) - * @return - instrumentation status + * patch_region_lock - insert a trap at the beginning of a patching region so no + * new incoming thread will execute it, and register the region as critical for + * next step + * @mdi - mcount dynamic info + * @info - disassembly info for the patched symbol + * @return - INSTRUMENT_SUCCESS */ -static int patch_code(struct mcount_dynamic_info *mdi, struct mcount_disasm_info *info) +static int patch_region_lock(struct mcount_dynamic_info *mdi, struct mcount_disasm_info *info) { /* * Let's assume we have the following instructions. @@ -814,14 +842,11 @@ static int patch_code(struct mcount_dynamic_info *mdi, struct mcount_disasm_info */ void *origin_code_addr; - void *trampoline_rel_addr; origin_code_addr = (void *)info->addr; if (info->has_intel_cet) origin_code_addr += ENDBR_INSN_SIZE; - trampoline_rel_addr = (void *)get_trampoline_offset(mdi, (unsigned long)origin_code_addr); - /* The first step is to insert a 1-byte trap-based probe point (atomic). * This will prevent threads to enter the critical zone while we patch it, * so no core will see partial modifications. @@ -832,19 +857,13 @@ static int patch_code(struct mcount_dynamic_info *mdi, struct mcount_disasm_info * 0xb: * * The trap will emulate a call to the trampoline while in place. - * - * We ensure that every core sees the trap before patching the critical - * zone, by synchronizing the them. */ if (register_trap(origin_code_addr, (void *)mdi->trampoline) == -1) return INSTRUMENT_FAILED; ((uint8_t *)origin_code_addr)[0] = 0xcc; - synchronize_all_cores(); - - /* - * The second step is to move any thread out of the critical zone if still + /* The second step is to move any thread out of the critical zone if still * present. Threads in the critical zone resume execution out of line, in * their dedicated OLX region. * @@ -856,8 +875,26 @@ static int patch_code(struct mcount_dynamic_info *mdi, struct mcount_disasm_info if (register_patch_region(origin_code_addr, info->orig_size) == -1) pr_dbg3("failed to register patch region\n"); - clear_patch_region(); - unregister_patch_region(origin_code_addr, info->orig_size); + + return INSTRUMENT_SUCCESS; +} + +/** + * patch_code - patch a region of the code with the operand of the call + * instruction used to probe a function. The call opcode needs to be inserted + * later. + * @mdi - mcount dynamic info for the current module + * @info - disassembly info for the patched symbol + */ +static void patch_code(struct mcount_dynamic_info *mdi, struct mcount_disasm_info *info) +{ + void *trampoline_rel_addr; + void *origin_code_addr; + + origin_code_addr = (void *)info->addr; + if (info->has_intel_cet) + origin_code_addr += ENDBR_INSN_SIZE; + trampoline_rel_addr = (void *)get_trampoline_offset(mdi, (unsigned long)origin_code_addr); /* The third step is to write the target address of the call. From the * processor view the 4-bytes address can be any garbage instructions. @@ -873,7 +910,23 @@ static int patch_code(struct mcount_dynamic_info *mdi, struct mcount_disasm_info memcpy(&((uint8_t *)origin_code_addr)[1], &trampoline_rel_addr, CALL_INSN_SIZE - 1); memset(origin_code_addr + CALL_INSN_SIZE, 0x90, /* NOP */ info->orig_size - CALL_INSN_SIZE); - /* FIXME Need to sync cores? Store membarrier? */ +} + +/** + * patch_region_unlock - unmark a region as critical and remove the trap that + * prevents execution. + * @info - disassembly info for the patched symbol + */ +static void patch_region_unlock(struct mcount_disasm_info *info) +{ + void *origin_code_addr; + + origin_code_addr = (void *)info->addr; + if (info->has_intel_cet) + origin_code_addr += ENDBR_INSN_SIZE; + + if (unregister_patch_region(origin_code_addr, info->orig_size) == -1) + pr_dbg3("failed to unregister patch region\n"); /* * The fourth and last step is to replace the trap with the call opcode. @@ -884,13 +937,19 @@ static int patch_code(struct mcount_dynamic_info *mdi, struct mcount_disasm_info */ ((uint8_t *)origin_code_addr)[0] = 0xe8; - unregister_trap(origin_code_addr); - - return INSTRUMENT_SUCCESS; } -static int patch_normal_func(struct mcount_dynamic_info *mdi, struct uftrace_symbol *sym, - struct mcount_disasm_engine *disasm) +/** + * patch_normal_func_init - perform the initial steps of the patching process, + * awaiting for sanitization of the critical region. This step is batched with + * subsequent ones. + * @mdi - mcount dynamic info for the current module + * @sym - symbol to patch + * @disasm - disassembly engine + * @return - instrumentation status + */ +static int patch_normal_func_init(struct mcount_dynamic_info *mdi, struct uftrace_symbol *sym, + struct mcount_disasm_engine *disasm) { uint8_t jmp_insn[15] = { 0x3e, @@ -898,20 +957,22 @@ static int patch_normal_func(struct mcount_dynamic_info *mdi, struct uftrace_sym 0x25, }; uint64_t jmp_target; - struct mcount_disasm_info info = { - .sym = sym, - .addr = mdi->map->start + sym->addr, - }; + struct mcount_disasm_info *info; unsigned call_offset = CALL_INSN_SIZE; int state; - state = disasm_check_insns(disasm, mdi, &info); + info = xmalloc(sizeof(*info)); + memset(info, 0, sizeof(*info)); + info->sym = sym; + info->addr = mdi->map->start + sym->addr; + + state = disasm_check_insns(disasm, mdi, info); if (state != INSTRUMENT_SUCCESS) { pr_dbg3(" >> %s: %s\n", state == INSTRUMENT_FAILED ? "FAIL" : "SKIP", sym->name); return state; } - pr_dbg2("force patch normal func: %s (patch size: %d)\n", sym->name, info.orig_size); + pr_dbg2("force patch normal func: %s (patch size: %d)\n", sym->name, info->orig_size); /* * stored origin instruction block: @@ -923,20 +984,62 @@ static int patch_normal_func(struct mcount_dynamic_info *mdi, struct uftrace_sym * | [Return address] | * ---------------------- */ - jmp_target = info.addr + info.orig_size; - if (info.has_intel_cet) { + jmp_target = info->addr + info->orig_size; + if (info->has_intel_cet) { jmp_target += ENDBR_INSN_SIZE; call_offset += ENDBR_INSN_SIZE; } memcpy(jmp_insn + CET_JMP_INSN_SIZE, &jmp_target, sizeof(jmp_target)); - if (info.has_jump) - mcount_save_code(&info, call_offset, jmp_insn, 0); + if (info->has_jump) + mcount_save_code(info, call_offset, jmp_insn, 0); else - mcount_save_code(&info, call_offset, jmp_insn, sizeof(jmp_insn)); + mcount_save_code(info, call_offset, jmp_insn, sizeof(jmp_insn)); - return patch_code(mdi, &info); + state = patch_region_lock(mdi, info); + commit_normal_func(&normal_funcs_patch, mdi, info); + + return state; +} + +/** + * mcount_patch_normal_func_fini - perform the final step of the patching process and cleanup + * @return - 0 + */ +void mcount_patch_normal_func_fini(void) +{ + struct patch_dynamic_info *pdi, *tmp; + + if (list_empty(&normal_funcs_patch)) + return; + + /* We ensure that every core sees the trap before patching the critical + * zone, by synchronizing the them. + */ + synchronize_all_cores(); + clear_patch_regions(); + + list_for_each_entry_safe(pdi, tmp, &normal_funcs_patch, list) { + patch_code(pdi->mdi, pdi->info); + write_memory_barrier(); + patch_region_unlock(pdi->info); + } + + synchronize_all_cores(); + + list_for_each_entry_safe(pdi, tmp, &normal_funcs_patch, list) { + void *origin_code_addr; + + origin_code_addr = (void *)pdi->info->addr; + if (pdi->info->has_intel_cet) + origin_code_addr += ENDBR_INSN_SIZE; + + unregister_trap(origin_code_addr); + list_del(&pdi->list); + free(pdi->info); + free(pdi); + } } static int unpatch_func(uint8_t *insn, char *name) @@ -1025,7 +1128,7 @@ int mcount_patch_func(struct mcount_dynamic_info *mdi, struct uftrace_symbol *sy break; case DYNAMIC_NONE: - result = patch_normal_func(mdi, sym, disasm); + result = patch_normal_func_init(mdi, sym, disasm); break; default: diff --git a/libmcount/dynamic.c b/libmcount/dynamic.c index 2d181b359..8b8cf0c90 100644 --- a/libmcount/dynamic.c +++ b/libmcount/dynamic.c @@ -225,6 +225,10 @@ __weak int mcount_patch_func(struct mcount_dynamic_info *mdi, struct uftrace_sym return -1; } +__weak void mcount_patch_normal_func_fini(void) +{ +} + __weak int mcount_unpatch_func(struct mcount_dynamic_info *mdi, struct uftrace_symbol *sym, struct mcount_disasm_engine *disasm) { @@ -568,6 +572,8 @@ static void patch_normal_func_matched(struct mcount_dynamic_info *mdi, struct uf if (!found) stats.nomatch++; + mcount_patch_normal_func_fini(); + free(soname); } From 5f3d75647711af6dd060ef980b0f3e87f1bdf3d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Guidi?= Date: Fri, 10 Mar 2023 11:00:43 -0500 Subject: [PATCH 14/18] dynamic: x86_64: Full-dynamic runtime unpatching MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit add for dynamically unpatching functions for the x86_64 architecture. The process is executed concurrently by replacing using a trap to resolve race conditions between thread (similar to how optimized kprobes are done). Co-authored-by: Gabriel-Andrew Pollo-Guilbert Signed-off-by: Clément Guidi --- arch/x86_64/mcount-dynamic.c | 111 ++++++++++++++++++++++++++++++++++- libmcount/dynamic.c | 5 +- 2 files changed, 111 insertions(+), 5 deletions(-) diff --git a/arch/x86_64/mcount-dynamic.c b/arch/x86_64/mcount-dynamic.c index e6471f630..06b7bb4d8 100644 --- a/arch/x86_64/mcount-dynamic.c +++ b/arch/x86_64/mcount-dynamic.c @@ -1042,6 +1042,110 @@ void mcount_patch_normal_func_fini(void) } } +extern int check_endbr64(unsigned long addr); + +/** + * unpatch_normal_func - remove full-dynamic instrumentation from a function + * entry and restore original instructions + * @mdi - mcount dynamic info for the current module + * @sym - symbol to patch + * @return - instrumentation status + */ +static int unpatch_normal_func(struct mcount_dynamic_info *mdi, struct uftrace_symbol *sym) +{ + /* + * Let assume that we have the following instructions. + * + * 0x0: call + * 0x5: + * 0xb: + * + * The goal is to modify the instructions in order to get the following + * instructions. + * + * 0x0: push %rbp + * 0x1: mov %rsp,%rbp + * 0x4: lea 0xeb0(%rip),%rdi + * 0xb: + */ + + unsigned long origin_code_addr; + unsigned call_offset = CALL_INSN_SIZE; + struct mcount_orig_insn *orig; + uint8_t *origin_code; + int orig_size; + + origin_code_addr = mdi->map->start + sym->addr; + if (check_endbr64(origin_code_addr)) + origin_code_addr += ENDBR_INSN_SIZE; + orig = mcount_find_insn(origin_code_addr + call_offset); + if (!orig) { + pr_dbg("cannot find original instructions for '%s' at %#lx\n", + origin_code_addr + call_offset, sym->name); + return -1; + } + origin_code = orig->orig; + orig_size = orig->orig_size; + + pr_dbg2("unpatch normal func: %s (patch size: %d)\n", sym->name, orig_size); + + /* + * The first step is to insert a trap. + * + * 0x0: int3 + * 0x1: + * 0x5: + * 0xb: + */ + + ((uint8_t *)origin_code_addr)[0] = 0xcc; + + /* + * The second step is to restore the bytes after the trap instruction. + * + * 0x0: int3 + * 0x1: mov %rsp,%rbp + * 0x4: lea 0xeb0(%rip),%rdi + * 0xb: + * + * Before restoring the last byte, a serialization instruction must be + * executed in order to synchronize the instruction cache of each processor. + * The easiest method is to execute a membarrier system call with + * MEMBARRIER_CMD_PRIVATE_EXPEDITED_SYNC_CORE. It will send a + * inter-processor interrupt that will execute the required serialization. + */ + + synchronize_all_cores(); + + if (!memcpy(&((uint8_t *)origin_code_addr)[1], &origin_code[1], orig_size - 1)) + return -1; + + /* + * The third step is to restore the last byte. + * + * 0x0: push %rbp + * 0x1: mov %rsp,%rbp + * 0x4: lea 0xeb0(%rip),%rdi + * 0xb: + */ + + ((uint8_t *)origin_code_addr)[0] = origin_code[0]; + + /* + * The fourth and last step is to move all thread that are currently + * executing in the modified instructions to the original instructions at + * the end of the trampoline. This is needed in order to free the memory + * allocated for the trampoline without any race condition. + * + * The method used to move the threads is to send the SIGRTMIN+n signal to + * all other threads. When their thread handler executes, it will check if + * the next instruction pointer is in the patching region. If it is, will + * move the next instruction pointer to the equivalent modified instruction. + */ + + return INSTRUMENT_SUCCESS; +} + static int unpatch_func(uint8_t *insn, char *name) { uint8_t nop5[] = { 0x0f, 0x1f, 0x44, 0x00, 0x00 }; @@ -1137,8 +1241,7 @@ int mcount_patch_func(struct mcount_dynamic_info *mdi, struct uftrace_symbol *sy return result; } -int mcount_unpatch_func(struct mcount_dynamic_info *mdi, struct uftrace_symbol *sym, - struct mcount_disasm_engine *disasm) +int mcount_unpatch_func(struct mcount_dynamic_info *mdi, struct uftrace_symbol *sym) { int result = INSTRUMENT_SKIPPED; @@ -1151,6 +1254,10 @@ int mcount_unpatch_func(struct mcount_dynamic_info *mdi, struct uftrace_symbol * result = unpatch_mcount_func(mdi, sym); break; + case DYNAMIC_NONE: + result = unpatch_normal_func(mdi, sym); + break; + default: break; } diff --git a/libmcount/dynamic.c b/libmcount/dynamic.c index 8b8cf0c90..fdb264ffb 100644 --- a/libmcount/dynamic.c +++ b/libmcount/dynamic.c @@ -229,8 +229,7 @@ __weak void mcount_patch_normal_func_fini(void) { } -__weak int mcount_unpatch_func(struct mcount_dynamic_info *mdi, struct uftrace_symbol *sym, - struct mcount_disasm_engine *disasm) +__weak int mcount_unpatch_func(struct mcount_dynamic_info *mdi, struct uftrace_symbol *sym) { return -1; } @@ -566,7 +565,7 @@ static void patch_normal_func_matched(struct mcount_dynamic_info *mdi, struct uf else if (match == 1) mcount_patch_func_with_stats(mdi, sym); else - mcount_unpatch_func(mdi, sym, NULL); + mcount_unpatch_func(mdi, sym); } if (!found) From 06d2cdf945e372b8dc868c3e5823af4bf94f3db4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Guidi?= Date: Mon, 10 Apr 2023 08:52:33 -0400 Subject: [PATCH 15/18] dynamic: Batch function unpatching process MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Synchronization requires sending signals to threads inside the process. This is performed for every symbols, which means a lot of signal can be sent. This has a major performance impact. This commit batches the initial and final steps of the patching process so we only need to send one signal per thread for every batch. Co-authored-by: Gabriel-Andrew Pollo-Guilbert Signed-off-by: Clément Guidi --- arch/x86_64/mcount-dynamic.c | 125 ++++++++++++++++++++++++----------- libmcount/dynamic.c | 5 ++ 2 files changed, 91 insertions(+), 39 deletions(-) diff --git a/arch/x86_64/mcount-dynamic.c b/arch/x86_64/mcount-dynamic.c index 06b7bb4d8..361b09d11 100644 --- a/arch/x86_64/mcount-dynamic.c +++ b/arch/x86_64/mcount-dynamic.c @@ -398,12 +398,13 @@ int setup_clear_patch_region(void) return 0; } -/* This list is used to store functions that need to be optimized or cleaned up - * later in the code. In both case, a SIGRTMIN+n must be send. By optimizing or - * cleaning all of them up at the same time, we only need to send one signal per - * thread. +/* These two lists are used to store functions that need to be optimized or + * cleaned up later in the code. In both case, a SIGRTMIN+n must be send. By + * optimizing or cleaning all of them up at the same time, we only need to send + * one signal per thread. */ LIST_HEAD(normal_funcs_patch); +LIST_HEAD(normal_funcs_unpatch); struct patch_dynamic_info { struct list_head list; @@ -935,7 +936,6 @@ static void patch_region_unlock(struct mcount_disasm_info *info) * 0x5: * 0xb: */ - ((uint8_t *)origin_code_addr)[0] = 0xe8; } @@ -1045,13 +1045,45 @@ void mcount_patch_normal_func_fini(void) extern int check_endbr64(unsigned long addr); /** - * unpatch_normal_func - remove full-dynamic instrumentation from a function - * entry and restore original instructions + * unpatch_code - restore original instructions except the trap at the beginning + * @info - mcount disassembly info + * @return - -1 on error, 0 on success + */ +int unpatch_code(struct mcount_disasm_info *info) +{ + uint8_t *origin_code = info->insns; + uint8_t *origin_code_addr = (uint8_t *)info->addr; + int orig_size = info->orig_size; + + /* + * The second step is to restore the bytes after the trap instruction. + * + * 0x0: int3 + * 0x1: mov %rsp,%rbp + * 0x4: lea 0xeb0(%rip),%rdi + * 0xb: + * + * Before restoring the last byte, a serialization instruction must be + * executed in order to synchronize the instruction cache of each processor. + * The easiest method is to execute a membarrier system call with + * MEMBARRIER_CMD_PRIVATE_EXPEDITED_SYNC_CORE. It will send a + * inter-processor interrupt that will execute the required serialization. + */ + + memcpy(&origin_code_addr[1], &origin_code[1], orig_size - 1); + + return 0; +} + +/** + * unpatch_normal_func_init - perform the initial steps of the unpatching + * process, awaiting for core synchronization. This step is batched with + * subsequent ones. * @mdi - mcount dynamic info for the current module * @sym - symbol to patch * @return - instrumentation status */ -static int unpatch_normal_func(struct mcount_dynamic_info *mdi, struct uftrace_symbol *sym) +static int unpatch_normal_func_init(struct mcount_dynamic_info *mdi, struct uftrace_symbol *sym) { /* * Let assume that we have the following instructions. @@ -1074,6 +1106,7 @@ static int unpatch_normal_func(struct mcount_dynamic_info *mdi, struct uftrace_s struct mcount_orig_insn *orig; uint8_t *origin_code; int orig_size; + struct mcount_disasm_info *info; origin_code_addr = mdi->map->start + sym->addr; if (check_endbr64(origin_code_addr)) @@ -1098,27 +1131,30 @@ static int unpatch_normal_func(struct mcount_dynamic_info *mdi, struct uftrace_s * 0xb: */ + register_trap((void *)origin_code_addr, (void *)mdi->trampoline); ((uint8_t *)origin_code_addr)[0] = 0xcc; - /* - * The second step is to restore the bytes after the trap instruction. - * - * 0x0: int3 - * 0x1: mov %rsp,%rbp - * 0x4: lea 0xeb0(%rip),%rdi - * 0xb: - * - * Before restoring the last byte, a serialization instruction must be - * executed in order to synchronize the instruction cache of each processor. - * The easiest method is to execute a membarrier system call with - * MEMBARRIER_CMD_PRIVATE_EXPEDITED_SYNC_CORE. It will send a - * inter-processor interrupt that will execute the required serialization. - */ + info = xmalloc(sizeof(*info)); + memset(info, 0, sizeof(*info)); + info->sym = sym; + info->addr = mdi->map->start + sym->addr; + ASSERT(orig_size < 64); /* fit into info->insns */ + memcpy(info->insns, origin_code, orig_size); + info->orig_size = orig_size; + commit_normal_func(&normal_funcs_unpatch, mdi, info); - synchronize_all_cores(); + return 0; +} - if (!memcpy(&((uint8_t *)origin_code_addr)[1], &origin_code[1], orig_size - 1)) - return -1; +/** + * unpatch_region_unlock - remove the trap at the start of the unpatched region + * @info - mcount disassembly info + * @return - 0 + */ +int unpatch_region_unlock(struct mcount_disasm_info *info) +{ + uint8_t *origin_code = info->insns; + uint8_t *origin_code_addr = (uint8_t *)info->addr; /* * The third step is to restore the last byte. @@ -1129,21 +1165,32 @@ static int unpatch_normal_func(struct mcount_dynamic_info *mdi, struct uftrace_s * 0xb: */ - ((uint8_t *)origin_code_addr)[0] = origin_code[0]; + origin_code_addr[0] = origin_code[0]; - /* - * The fourth and last step is to move all thread that are currently - * executing in the modified instructions to the original instructions at - * the end of the trampoline. This is needed in order to free the memory - * allocated for the trampoline without any race condition. - * - * The method used to move the threads is to send the SIGRTMIN+n signal to - * all other threads. When their thread handler executes, it will check if - * the next instruction pointer is in the patching region. If it is, will - * move the next instruction pointer to the equivalent modified instruction. - */ + return 0; +} - return INSTRUMENT_SUCCESS; +/** + * mcount_unpatch_normal_func_fini - perform the final steps of the unpatching + * process and cleanup + */ +void mcount_unpatch_normal_func_fini(void) +{ + struct patch_dynamic_info *pdi, *tmp; + + if (list_empty(&normal_funcs_unpatch)) + return; + + synchronize_all_cores(); + + list_for_each_entry_safe(pdi, tmp, &normal_funcs_unpatch, list) { + unpatch_code(pdi->info); + write_memory_barrier(); + unpatch_region_unlock(pdi->info); + list_del(&pdi->list); + free(pdi->info); + free(pdi); + } } static int unpatch_func(uint8_t *insn, char *name) @@ -1255,7 +1302,7 @@ int mcount_unpatch_func(struct mcount_dynamic_info *mdi, struct uftrace_symbol * break; case DYNAMIC_NONE: - result = unpatch_normal_func(mdi, sym); + result = unpatch_normal_func_init(mdi, sym); break; default: diff --git a/libmcount/dynamic.c b/libmcount/dynamic.c index fdb264ffb..9cf3b6cbb 100644 --- a/libmcount/dynamic.c +++ b/libmcount/dynamic.c @@ -229,6 +229,10 @@ __weak void mcount_patch_normal_func_fini(void) { } +__weak void mcount_unpatch_normal_func_fini(void) +{ +} + __weak int mcount_unpatch_func(struct mcount_dynamic_info *mdi, struct uftrace_symbol *sym) { return -1; @@ -572,6 +576,7 @@ static void patch_normal_func_matched(struct mcount_dynamic_info *mdi, struct uf stats.nomatch++; mcount_patch_normal_func_fini(); + mcount_unpatch_normal_func_fini(); free(soname); } From fe06e08abd00c1fc77de1111d4dcf7d580f832a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Guidi?= Date: Tue, 7 Mar 2023 10:25:02 -0500 Subject: [PATCH 16/18] uftrace: Apply dynamic (un)patching at runtime MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add support for the '--patch' and '--unpatch' options in the client. When used, the agent patches or unpatches symbols on the fly. Co-authored-by: Gabriel-Andrew Pollo-Guilbert Signed-off-by: Clément Guidi --- cmds/live.c | 9 ++++++++- libmcount/mcount.c | 7 ++++++- uftrace.h | 1 + 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/cmds/live.c b/cmds/live.c index d65c5cb86..75c6eb4a2 100644 --- a/cmds/live.c +++ b/cmds/live.c @@ -361,7 +361,7 @@ static int forward_options(struct uftrace_opts *opts) } /* provide a pattern type for options that need it */ - if (opts->filter || opts->caller || opts->trigger) { + if (opts->filter || opts->caller || opts->trigger || opts->patch) { status = forward_option(sfd, capabilities, UFTRACE_AGENT_OPT_PATTERN, &opts->patt_type, sizeof(opts->patt_type)); if (status < 0) @@ -389,6 +389,13 @@ static int forward_options(struct uftrace_opts *opts) goto close; } + if (opts->patch) { + status = forward_option(sfd, capabilities, UFTRACE_AGENT_OPT_PATCH, opts->patch, + strlen(opts->patch) + 1); + if (status < 0) + goto close; + } + close: status_close = agent_message_send(sfd, UFTRACE_MSG_AGENT_CLOSE, NULL, 0); if (status_close == 0) { diff --git a/libmcount/mcount.c b/libmcount/mcount.c index 6347fab3f..0c2a3182b 100644 --- a/libmcount/mcount.c +++ b/libmcount/mcount.c @@ -107,7 +107,7 @@ static volatile bool agent_run = false; #define MCOUNT_AGENT_CAPABILITIES \ (UFTRACE_AGENT_OPT_TRACE | UFTRACE_AGENT_OPT_DEPTH | UFTRACE_AGENT_OPT_THRESHOLD | \ UFTRACE_AGENT_OPT_PATTERN | UFTRACE_AGENT_OPT_FILTER | UFTRACE_AGENT_OPT_CALLER | \ - UFTRACE_AGENT_OPT_TRIGGER) + UFTRACE_AGENT_OPT_TRIGGER | UFTRACE_AGENT_OPT_PATCH) __weak void dynamic_return(void) { @@ -1988,6 +1988,11 @@ static int agent_apply_option(int opt, void *value, size_t size, agent_setup_trigger(value, triggers); break; + case UFTRACE_AGENT_OPT_PATCH: + pr_dbg3("apply patch '%s' (size=%d)\n", value, size); + mcount_dynamic_update(&mcount_sym_info, value, mcount_filter_setting.ptype); + break; + default: ret = -1; } diff --git a/uftrace.h b/uftrace.h index b483b177b..154d1451f 100644 --- a/uftrace.h +++ b/uftrace.h @@ -466,6 +466,7 @@ enum uftrace_agent_opt { UFTRACE_AGENT_OPT_FILTER = (1U << 4), /* tracing filters */ UFTRACE_AGENT_OPT_CALLER = (1U << 5), /* tracing caller filters */ UFTRACE_AGENT_OPT_TRIGGER = (1U << 6), /* tracing trigger actions */ + UFTRACE_AGENT_OPT_PATCH = (1U << 7), /* patch string */ }; extern struct uftrace_session *first_session; From 24d7b8efd55f4235f28a93e6913adfb7f78609bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Guidi?= Date: Mon, 10 Apr 2023 09:07:53 -0400 Subject: [PATCH 17/18] mcount: Use flag to indicate if target is running MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use a global flag to indicate the state of the target. When the target is not running, tasks such as dynamic patching can be performed with less constraints. If libmcount.so is dynamically injected (not implemented yet), the 'mcount_target_running' flag indicates that libmcount has to be initialized in a running target. Signed-off-by: Clément Guidi --- libmcount/mcount.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/libmcount/mcount.c b/libmcount/mcount.c index 0c2a3182b..5386d0b7c 100644 --- a/libmcount/mcount.c +++ b/libmcount/mcount.c @@ -52,6 +52,9 @@ bool mcount_auto_recover = ARCH_SUPPORT_AUTO_RECOVER; /* global flag to control mcount behavior */ unsigned long mcount_global_flags = MCOUNT_GFL_SETUP; +/* global to indicate if the target is executing */ +bool mcount_target_running = false; + /* TSD key to save mtd below */ pthread_key_t mtd_key = (pthread_key_t)-1; @@ -2342,6 +2345,7 @@ static __used void mcount_startup(void) mcount_global_flags &= ~MCOUNT_GFL_SETUP; mtd.recursion_marker = false; + mcount_target_running = true; } static void mcount_cleanup(void) From e5bbd27c131a622b9176cbe5baa6793bc7ba17d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Guidi?= Date: Mon, 10 Apr 2023 09:11:55 -0400 Subject: [PATCH 18/18] dynamic: x86_64: Use soft patching in cold targets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When patching a binary before its execution, we can skip the serialization and critical zone exclusion steps. These steps are only use full when cross-modification occurs, at runtime. Signed-off-by: Clément Guidi --- arch/x86_64/mcount-dynamic.c | 60 ++++++++++++++++++++++++------------ 1 file changed, 40 insertions(+), 20 deletions(-) diff --git a/arch/x86_64/mcount-dynamic.c b/arch/x86_64/mcount-dynamic.c index 361b09d11..6f113966c 100644 --- a/arch/x86_64/mcount-dynamic.c +++ b/arch/x86_64/mcount-dynamic.c @@ -28,6 +28,8 @@ static const unsigned char patchable_clang_nop[] = { 0x0f, 0x1f, 0x44, 0x00, 0x0 static const unsigned char endbr64[] = { 0xf3, 0x0f, 0x1e, 0xfa }; +extern bool mcount_target_running; + /* * Hashmap of trampoline locations for every installed int3 trap. The traps are * used when patching and unpatching functions: they prevent threads from @@ -997,8 +999,14 @@ static int patch_normal_func_init(struct mcount_dynamic_info *mdi, struct uftrac else mcount_save_code(info, call_offset, jmp_insn, sizeof(jmp_insn)); - state = patch_region_lock(mdi, info); - commit_normal_func(&normal_funcs_patch, mdi, info); + if (mcount_target_running) { + state = patch_region_lock(mdi, info); + commit_normal_func(&normal_funcs_patch, mdi, info); + } + else { + patch_code(mdi, info); + patch_region_unlock(info); + } return state; } @@ -1011,6 +1019,9 @@ void mcount_patch_normal_func_fini(void) { struct patch_dynamic_info *pdi, *tmp; + if (!mcount_target_running) + return; + if (list_empty(&normal_funcs_patch)) return; @@ -1122,26 +1133,32 @@ static int unpatch_normal_func_init(struct mcount_dynamic_info *mdi, struct uftr pr_dbg2("unpatch normal func: %s (patch size: %d)\n", sym->name, orig_size); - /* - * The first step is to insert a trap. - * - * 0x0: int3 - * 0x1: - * 0x5: - * 0xb: - */ + if (mcount_target_running) { + info = xmalloc(sizeof(*info)); + memset(info, 0, sizeof(*info)); + info->sym = sym; + info->addr = mdi->map->start + sym->addr; + ASSERT(orig_size < 64); /* fit into info->insns */ + memcpy(info->insns, origin_code, orig_size); + info->orig_size = orig_size; + + /* The first step is to insert a trap. + * + * 0x0: int3 + * 0x1: + * 0x5: + * 0xb: + */ - register_trap((void *)origin_code_addr, (void *)mdi->trampoline); - ((uint8_t *)origin_code_addr)[0] = 0xcc; + register_trap((void *)origin_code_addr, (void *)mdi->trampoline); + ((uint8_t *)origin_code_addr)[0] = 0xcc; - info = xmalloc(sizeof(*info)); - memset(info, 0, sizeof(*info)); - info->sym = sym; - info->addr = mdi->map->start + sym->addr; - ASSERT(orig_size < 64); /* fit into info->insns */ - memcpy(info->insns, origin_code, orig_size); - info->orig_size = orig_size; - commit_normal_func(&normal_funcs_unpatch, mdi, info); + commit_normal_func(&normal_funcs_unpatch, mdi, info); + } + else { + if (!memcpy((uint8_t *)origin_code_addr, origin_code, orig_size)) + return INSTRUMENT_FAILED; + } return 0; } @@ -1178,6 +1195,9 @@ void mcount_unpatch_normal_func_fini(void) { struct patch_dynamic_info *pdi, *tmp; + if (!mcount_target_running) + return; + if (list_empty(&normal_funcs_unpatch)) return;