From 795c59019edafb0135560e4a8c9630164b4ef784 Mon Sep 17 00:00:00 2001 From: Giuliano Belinassi Date: Mon, 26 Aug 2024 18:04:30 -0300 Subject: [PATCH] Remove insn_queue from libpulp The insn_queue was introduced to overcome issues related to seccomp. However there is a simpler way of overcoming this issue which is to use /proc/self/mem. Signed-off-by: Giuliano Belinassi --- include/Makefile.am | 3 +- include/insn_queue_lib.h | 35 ----- lib/Makefile.am | 4 - lib/insn_queue.c | 172 ----------------------- lib/ulp.c | 59 ++++++-- tests/Makefile.am | 6 - tests/insn_queue.c | 292 --------------------------------------- tests/insn_queue.py | 27 ---- tools/insn_queue.c | 3 +- tools/trigger.c | 18 ++- 10 files changed, 61 insertions(+), 558 deletions(-) delete mode 100644 include/insn_queue_lib.h delete mode 100644 lib/insn_queue.c delete mode 100644 tests/insn_queue.c delete mode 100644 tests/insn_queue.py diff --git a/include/Makefile.am b/include/Makefile.am index 6bf95980..5dc4130a 100644 --- a/include/Makefile.am +++ b/include/Makefile.am @@ -26,5 +26,4 @@ noinst_HEADERS = \ error_common.h \ terminal_colors.h \ ld_rtld.h \ - insn_queue.h \ - insn_queue_lib.h + insn_queue.h diff --git a/include/insn_queue_lib.h b/include/insn_queue_lib.h deleted file mode 100644 index cddbe78e..00000000 --- a/include/insn_queue_lib.h +++ /dev/null @@ -1,35 +0,0 @@ -/* - * libpulp - User-space Livepatching Library - * - * Copyright (C) 2023 SUSE Software Solutions GmbH - * - * This file is part of libpulp. - * - * libpulp is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 2.1 of the License, or (at your option) any later version. - * - * libpulp is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with libpulp. If not, see . - */ - -#ifndef INSNQ_TOOL_H -#define INSNQ_TOOL_H - -#include "insn_queue.h" - -void *insnq_get_writable_area(insn_queue_t *, size_t insn_size); - -ulp_error_t insnq_insert_print(const char *string); - -ulp_error_t insnq_insert_write(void *addr, int n, const void *bytes); - -int insnq_ensure_emptiness(void); - -#endif diff --git a/lib/Makefile.am b/lib/Makefile.am index 4a6e9753..6c8122eb 100644 --- a/lib/Makefile.am +++ b/lib/Makefile.am @@ -23,7 +23,6 @@ libpulp_la_SOURCES = \ ulp.c \ interpose.c \ msg_queue.c \ - insn_queue.c \ error.c \ ulp_prologue.S \ ulp_interface.S @@ -39,7 +38,4 @@ libpulp_la_LIBADD = $(top_builddir)/common/libcommon.la AM_CFLAGS += -I$(top_srcdir)/include -# Add -fno-strict-alias to the insn_queue code. -insn_queue.lo : CFLAGS += -fno-strict-aliasing - EXTRA_DIST = libpulp.versions diff --git a/lib/insn_queue.c b/lib/insn_queue.c deleted file mode 100644 index e1c0ea73..00000000 --- a/lib/insn_queue.c +++ /dev/null @@ -1,172 +0,0 @@ -/* - * libpulp - User-space Livepatching Library - * - * Copyright (C) 2021 SUSE Software Solutions GmbH - * - * This file is part of libpulp. - * - * libpulp is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 2.1 of the License, or (at your option) any later version. - * - * libpulp is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with libpulp. If not, see . - */ - -#include "insn_queue.h" - -#include -#include -#include - -#include "error.h" -#include "ulp_common.h" - -/** Global instruction queue object. */ -insn_queue_t __ulp_insn_queue = { .version = INSNQ_CURR_VERSION }; - -static int -align_to(int val, int bytes) -{ - int mask = bytes - 1; - return (val + mask) & (~mask); -} - -/** @brief Get memory area to write an instruction to in the queue. - * - * This function will retrieve an area of memory in the queue object in which - * an instruction of size `msg_size` can be writen to. The instruction is - * appended to the end of the queue, and depending of the queue attribute - * `discard_old_content` it may return NULL if there is pending operations in - * the queue, or overwrite the instruction that is on the begining of the - * queue. In case the instruction do not fit in the queue, NULL is returned. - * - * @param queue The instruction queue object. - * @param msg_size Size of instruction to allocate area to. - * - * @return Valid pointer to write to in success, NULL otherwise. - - */ -void * -insnq_get_writable_area(struct insn_queue *queue, size_t msg_size) -{ - /* Write the msg_queue values in variables for briefness. */ - int num_insns = queue->num_insns; - int size = queue->size; - char *buffer = queue->buffer; - - /* In case the message is empty or it is too large for the buffer, don't - * bother even trying to insert it. */ - if (msg_size == 0) - return NULL; - - /* In case the instruction won't fit the queue, then quickly return with - NULL as answer. */ - if (msg_size + size > INSN_BUFFER_MAX) { - return NULL; - } - - /* Reserve area for write. This breaks strict aliasing rules, so this file - must be compiled with -fno-strict-aliasing. */ - void *ret = &buffer[size]; - - /* Update number of bytes. */ - size += msg_size; - num_insns++; - - /* Commit back to original object. */ - - queue->num_insns = num_insns; - queue->size = size; - - return ret; -} - -/** @brief Insert print instruction into the queue. - * - * @param queue The instruction queue object. - * @param string String to print. - */ -ulp_error_t -insnq_insert_print(const char *string) -{ - insn_queue_t *queue = &__ulp_insn_queue; - - int string_size = strlen(string) + 1; - int insn_size = align_to(sizeof(struct ulp_insn_print) + string_size, 4); - struct ulp_insn_print *insn = insnq_get_writable_area(queue, insn_size); - - if (insn == NULL) { - set_libpulp_error_state(EINSNQ); - return EINSNQ; - } - - insn->base.type = ULP_INSN_PRINT; - insn->base.size = insn_size; - memcpy(insn->bytes, string, string_size); - - return ENONE; -} - -/** @brief Insert write instruction into the queue. - * - * @param queue The instruction queue object. - * @param addr Address to patch. - * @param n Number of bytes to patch. - * @param bytes Bytes to patch with. - */ -ulp_error_t -insnq_insert_write(void *addr, int n, const void *bytes) -{ - insn_queue_t *queue = &__ulp_insn_queue; - - int insn_size = align_to(sizeof(struct ulp_insn_write) + n, 8); - struct ulp_insn_write *insn = insnq_get_writable_area(queue, insn_size); - - if (insn == NULL) { - set_libpulp_error_state(EINSNQ); - return EINSNQ; - } - - insn->base.type = ULP_INSN_WRITE; - insn->base.size = insn_size; - insn->n = n; - insn->address = (uintptr_t)addr; - memcpy(insn->bytes, bytes, n); - - return ENONE; -} - -/** @brief Ensure that the instruction queue is empty. - * - * When a livepatch is triggered, the instruction queue must be empty in order - * to safely insert instructions on it. Otherwise, this means something bad - * occured on ulp side which prevented the queue to be updated after the insns - * were executed. - * - * This function will block livepatching if not empty. - * - * @return 0 if success, anything else if not empty - * - */ -int -insnq_ensure_emptiness(void) -{ - insn_queue_t *queue = &__ulp_insn_queue; - - if (queue->num_insns > 0 || queue->size > 0) { - WARN("WARN: instruction queue not empty. This is an indication that " - "something went wrong on ulp side."); - - set_libpulp_error_state(EINSNQ); - return 1; - } - - return 0; -} diff --git a/lib/ulp.c b/lib/ulp.c index 18f7458f..8f36e97f 100644 --- a/lib/ulp.c +++ b/lib/ulp.c @@ -36,7 +36,6 @@ #include "config.h" #include "error.h" -#include "insn_queue_lib.h" #include "interpose.h" #include "msg_queue.h" #include "ulp.h" @@ -89,6 +88,38 @@ begin(void) msgq_push("libpulp loaded...\n"); } +/** @brief Write into memory bypassing memory protections + * + * The process may be launched with mprotect through seccomp, which + * will block certain addresses to be written. This function + * circunvent this by writing through /proc/self/map. + * + * @param dest Destination address + * @param src Source address + * @param n number of bytes + * @return dest on success, NULL if error. + */ +void * +memwrite(void *dest, const void *src, size_t n) +{ + FILE *file = fopen("/proc/self/mem", "r+"); + + /* SLE have some processes which chroots into /proc. If the above fopen + fails then try this to check if this is the case. */ + if (file == NULL) { + file = fopen("/self/mem", "r+"); + libpulp_assert(file != NULL); + } + + libpulp_assert(fseek(file, (size_t)dest, SEEK_SET) == 0); + libpulp_assert(fwrite(src, 1, n, file) == n); + + fflush(file); + fclose(file); + + return dest; +} + /** @brief Revert all live patches associated with library `lib_name` * * The user may have applied a series of live patches on a library named @@ -157,10 +188,6 @@ __ulp_revert_patches_from_lib() if (libpulp_is_in_error_state()) return get_libpulp_error_state(); - /* If the instruction queue is in an weird state, we cannot continue. */ - if (insnq_ensure_emptiness()) - return get_libpulp_error_state(); - /* * If the target process is busy within functions from the malloc or * dlopen implementations, applying a live patch could lead to a @@ -173,6 +200,10 @@ __ulp_revert_patches_from_lib() /* Otherwise, try to apply the live patch. */ result = revert_all_patches_from_lib(__ulp_metadata_buffer); + /* If we entered in an error state, then return the error. */ + if (libpulp_is_in_error_state()) + return get_libpulp_error_state(); + /* * Live patching could fail for a couple of different reasons, thus * check the result and return either zero for success or one for @@ -191,10 +222,6 @@ __ulp_apply_patch() if (libpulp_is_in_error_state()) return get_libpulp_error_state(); - /* If the instruction queue is in an weird state, we cannot continue. */ - if (insnq_ensure_emptiness()) - return get_libpulp_error_state(); - /* * If the target process is busy within functions from the malloc or * dlopen implementations, applying a live patch could lead to a @@ -207,6 +234,10 @@ __ulp_apply_patch() /* Otherwise, try to apply the live patch. */ result = load_patch(); + /* If we entered in an error state, then return the error. */ + if (libpulp_is_in_error_state()) + return get_libpulp_error_state(); + /* * Live patching could fail for a couple of different reasons, thus * check the result and return either zero for success or whatever @@ -743,11 +774,11 @@ ulp_apply_all_units(struct ulp_metadata *ulp) if (ref->tls) { tls_index ti = { .ti_module = tls_idx, .ti_offset = ref->target_offset }; - insnq_insert_write((void *)patch_address, sizeof(ti), &ti); + memwrite((void *)patch_address, &ti, sizeof(ti)); } else { uintptr_t target_address = target_base + ref->target_offset; - insnq_insert_write((void *)patch_address, sizeof(void *), &target_address); + memwrite((void *)patch_address, &target_address, sizeof(void *)); } ref = ref->next; } @@ -1064,7 +1095,7 @@ check_build_id(struct ulp_metadata *ulp) static void ulp_patch_prologue_layout(void *old_fentry, const char *prologue, int len) { - insnq_insert_write(old_fentry, len, prologue); + memwrite(old_fentry, prologue, len); } /** @brief skip the ulp prologue. @@ -1090,7 +1121,7 @@ ulp_skip_prologue(void *fentry) bias += sizeof(insn_endbr64); /* Do not jump backwards on function entry (0x6690 is a nop on x86). */ - insnq_insert_write((char *)fentry + bias, sizeof(insn_nop2), insn_nop2); + memwrite((char *)fentry + bias, insn_nop2, sizeof(insn_nop2)); } /** @brief Get patched address of function with universe index = idx. @@ -1193,7 +1224,7 @@ void ulp_patch_addr_absolute(void *old_fentry, void *manager) { char *dst = (char *)old_fentry + ULP_DATA_OFFSET; - insnq_insert_write(dst, sizeof(void *), &manager); + memwrite(dst, &manager, sizeof(void *)); } /** @brief Actually patch the old function with the new function diff --git a/tests/Makefile.am b/tests/Makefile.am index 0f3a95e6..32f70c30 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -444,7 +444,6 @@ check_PROGRAMS = \ pcqueue \ comments \ block_mprotect \ - insn_queue \ chroot \ visibility @@ -580,10 +579,6 @@ block_mprotect_SOURCES = block_mprotect.c block_mprotect_CFLAGS = $(AM_CFLAGS) -I/usr/include/libseccomp/ block_mprotect_LDADD = -lseccomp -insn_queue_SOURCES = insn_queue.c -insn_queue_CFLAGS = $(AM_CFLAGS) -I$(srcdir)/../include -I$(srcdir)/../tools/include -insn_queue_LDADD = - chroot_SOURCES = chroot.c chroot_CFLAGS = $(AM_CFLAGS) chroot_LDADD = libparameters.la @@ -643,7 +638,6 @@ TESTS = \ mprotect_patch.py \ patches.py \ mprotect_patch.py \ - insn_queue.py \ chroot.py \ visibility.py diff --git a/tests/insn_queue.c b/tests/insn_queue.c deleted file mode 100644 index 5fa2bd1c..00000000 --- a/tests/insn_queue.c +++ /dev/null @@ -1,292 +0,0 @@ -#define _GNU_SOURCE -#include -#include - -void -ulp_warn(const char *format, ...) -{ - va_list args; - va_start(args, format); - vprintf(format, args); - va_end(args); -} - -void -ulp_debug(const char *format, ...) -{ - va_list args; - va_start(args, format); - vprintf(format, args); - va_end(args); -} - -void -msgq_push(const char *format, ...) -{ - va_list arglist; - - va_start(arglist, format); - vprintf(format, arglist); - va_end(arglist); -} - -/* Disable the poisoning in error.h. */ -#define DISABLE_ERR_POISON - -#include "../lib/error.c" -#include "../lib/insn_queue.c" -#include "../tools/insn_queue.c" -#include "../tools/ptrace.c" - -/* Set a two-way communcation channel between child and parent. */ -static int fd[2][2]; -static int is_child = false; - -/* Send message. */ -static void -send(char c) -{ - int r; - if (is_child) { - r = write(fd[0][1], &c, 1); - } - else { - r = write(fd[1][1], &c, 1); - } - - assert(r == 1); -} - -/* Wait for message. */ -static void -wait_for(char x) -{ - int r; - char c; - do { - if (is_child) { - r = read(fd[1][0], &c, 1); - } - else { - r = read(fd[0][0], &c, 1); - } - } - while (c != x); - - assert(r == 1); -} - -/* Test1: Fill the queue with print messages. All messages should be inserted - successfully. */ -static void -test1_parent(int child_pid) -{ - printf("Test 1 start\n"); - wait_for('a'); - int stdout_copy = dup(1); - close(1); - if (insnq_interpret_from_process_(child_pid, - (Elf64_Addr)&__ulp_insn_queue)) { - abort(); - } - fflush(stdout); - dup2(stdout_copy, 1); - send('b'); -} - -static void -test1_child(void) -{ - int n = INSN_BUFFER_MAX / 8; - const char *string = "abc"; - for (int i = 0; i < n; i++) { - insnq_insert_print(string); - } - - send('a'); - wait_for('b'); -} - -/* Test2: Check if the queue correctly fails if it detects that the client is - outdated. */ -static void -test2_parent(int child_pid) -{ - printf("Test 2 start\n"); - wait_for('c'); - if (insnq_interpret_from_process_( - child_pid, (Elf64_Addr)&__ulp_insn_queue) != EOLDULP) { - abort(); - } - send('d'); - wait_for('1'); -} - -void -test2_child(void) -{ - /* Modify the queue version. */ - int old_ver = __ulp_insn_queue.version; - __ulp_insn_queue.version = 1 << 30; - send('c'); - wait_for('d'); - __ulp_insn_queue.version = old_ver; - send('1'); -} - -/* Test3: Fill the queue with write messages. All messages should be inserted - successfully, and there should be a write into the target process related to - the address we passed. */ -volatile char write_frame[8]; -static void -test3_parent(int child_pid) -{ - printf("Test 3 start\n"); - - wait_for('e'); - /* We need to attach to proess in the test. On the ULP tool that would - already be done. */ - attach(child_pid); - - ulp_error_t ret = - insnq_interpret_from_process_(child_pid, (Elf64_Addr)&__ulp_insn_queue); - if (ret) { - printf("Error interpreting queue on test 3\n"); - abort(); - } - detach(child_pid); - - send('f'); -} - -static void -test3_child(void) -{ - char buf[8]; - for (int i = 0; i < 8; i++) { - buf[i] = 'a'; - } - - int n = INSN_BUFFER_MAX / align_to(sizeof(struct ulp_insn_write) + 8, 8); - for (int i = 0; i < n; i++) { - insnq_insert_write((void *)write_frame, 8, buf); - } - - send('e'); - wait_for('f'); - - if (memcmp(buf, (void *)write_frame, 8) != 0) { - abort(); - } -} - -/* Test4: Try to add more messages than supported in the queue. It should fail. - */ -static void -test4_child(void) -{ - char buf[8]; - for (int i = 0; i < 8; i++) { - buf[i] = 'a'; - } - - int n = (INSN_BUFFER_MAX) / align_to(sizeof(struct ulp_insn_write) + 8, 8); - for (int i = 0; i < n; i++) { - insnq_insert_write((void *)write_frame, 8, buf); - } - - /* Last write should be blocked. */ - ulp_error_t ret = insnq_insert_write((void *)write_frame, 8, buf); - - /* Should detect that we are out of memory in the queue and fail. */ - if (ret != EINSNQ) { - abort(); - } - - send('g'); - wait_for('h'); -} - -static void -test4_parent(int child_pid) -{ - printf("Test 4 start\n"); - - wait_for('g'); - /* We need to attach to proess in the test. On the ULP tool that would - already be done. */ - attach(child_pid); - - ulp_error_t ret = - insnq_interpret_from_process_(child_pid, (Elf64_Addr)&__ulp_insn_queue); - if (ret) { - printf("Error interpreting queue on test 3\n"); - abort(); - } - detach(child_pid); - - send('h'); -} - -static int -parent(pid_t child_pid) -{ - test1_parent(child_pid); - test2_parent(child_pid); - test3_parent(child_pid); - test4_parent(child_pid); - - if (insnq_ensure_emptiness()) { - /* Ensure that the queue ends empty. */ - abort(); - } - return 0; -} - -static void -child(void) -{ - test1_child(); - test2_child(); - test3_child(); - test4_child(); -} - -int -main(void) -{ - pid_t pid; - assert(pipe(fd[0]) == 0); - assert(pipe(fd[1]) == 0); - - pid = fork(); - if (pid == 0) { - is_child = true; - child(); - } - else { - parent(pid); - - int wstatus; - waitpid(pid, &wstatus, 0); - - if (WIFEXITED(wstatus)) { - int r = WEXITSTATUS(wstatus); - if (r) { - printf("Process %d returned non-zero: %d\n", pid, r); - return 1; - } - } - else { - printf("Process %d ended without calling exit\n", pid); - return 1; - } - } - - close(fd[0][0]); - close(fd[0][1]); - close(fd[1][0]); - close(fd[1][1]); - printf("Success\n"); - return 0; -} diff --git a/tests/insn_queue.py b/tests/insn_queue.py deleted file mode 100644 index 70ac7407..00000000 --- a/tests/insn_queue.py +++ /dev/null @@ -1,27 +0,0 @@ -#!/usr/bin/env python3 -# -# This file is part of libpulp. -# -# libpulp is free software; you can redistribute it and/or -# modify it under the terms of the GNU Lesser General Public -# License as published by the Free Software Foundation; either -# version 2.1 of the License, or (at your option) any later version. -# -# libpulp is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU -# Lesser General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with libpulp. If not, see . - -# This test checks the integrity of the instruction queue between libpulp and -# the ULP tool. - -import sys -import testsuite -import subprocess - -child = testsuite.spawn('insn_queue', timeout = 20) -child.expect("Success") -child.close(force=False) diff --git a/tools/insn_queue.c b/tools/insn_queue.c index 88fb075f..390a0241 100644 --- a/tools/insn_queue.c +++ b/tools/insn_queue.c @@ -271,7 +271,8 @@ insnq_interpret_from_process_(int pid, Elf64_Addr queue_addr) static insn_queue_t queue; if (queue_addr == 0) { - /* Libpulp is old and do not have a instruction queue. */ + /* Libpulp is old and do not have a instruction queue, or is too new and + the instruction queue was overcame. */ return EOLDLIBPULP; } diff --git a/tools/trigger.c b/tools/trigger.c index 28638bf0..a1de2b43 100644 --- a/tools/trigger.c +++ b/tools/trigger.c @@ -192,11 +192,19 @@ trigger_one_process(struct ulp_process *target, int retries, "fatal error during live patch application (hijacked execution)."); retry = 0; } - if (result) - DEBUG("live patching %d failed (attempt #%d).", target->pid, - (retries - retry)); - else - retry = 0; + switch (result) { + /* In case of success or a failure because the patch were already + applied, then do not retry. */ + case 0: + case EAPPLIED: + retry = 0; + break; + + default: + DEBUG("live patching %d failed (attempt #%d).", target->pid, + (retries - retry)); + break; + } } #if defined ENABLE_STACK_CHECK && ENABLE_STACK_CHECK range_check_failed: