From 297c34d854ba113b2f5730279d76ad667ee5c957 Mon Sep 17 00:00:00 2001 From: Giuliano Belinassi Date: Thu, 17 Oct 2024 16:31:53 -0300 Subject: [PATCH] Revert "Remove insn_queue from libpulp" Relying on /proc/self/mem to bypass mprotect issues turn out to be unreliable: the process can change into another user which does not have access to this file, hence ptrace as `ulp` being root is the only way out. This reverts commit 795c59019edafb0135560e4a8c9630164b4ef784. --- include/Makefile.am | 3 +- include/insn_queue_lib.h | 35 +++++ lib/Makefile.am | 4 + lib/insn_queue.c | 172 +++++++++++++++++++++++ lib/ulp.c | 38 ++--- tests/Makefile.am | 28 ++-- tests/insn_queue.c | 292 +++++++++++++++++++++++++++++++++++++++ tests/insn_queue.py | 27 ++++ tools/insn_queue.c | 3 +- tools/trigger.c | 18 +-- 10 files changed, 571 insertions(+), 49 deletions(-) create mode 100644 include/insn_queue_lib.h create mode 100644 lib/insn_queue.c create mode 100644 tests/insn_queue.c create mode 100644 tests/insn_queue.py diff --git a/include/Makefile.am b/include/Makefile.am index eee2b5d8..98a1dcb3 100644 --- a/include/Makefile.am +++ b/include/Makefile.am @@ -26,7 +26,8 @@ noinst_HEADERS = \ error_common.h \ terminal_colors.h \ ld_rtld.h \ - insn_queue.h + insn_queue.h \ + insn_queue_lib.h # Workaround a bug in Autoconf 2.69 if CPU_X86_64 diff --git a/include/insn_queue_lib.h b/include/insn_queue_lib.h new file mode 100644 index 00000000..cddbe78e --- /dev/null +++ b/include/insn_queue_lib.h @@ -0,0 +1,35 @@ +/* + * 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 c6886955..77b64c7c 100644 --- a/lib/Makefile.am +++ b/lib/Makefile.am @@ -23,6 +23,7 @@ libpulp_la_SOURCES = \ ulp.c \ interpose.c \ msg_queue.c \ + insn_queue.c \ error.c libpulp_la_LDFLAGS = \ @@ -56,4 +57,7 @@ libpulp_la_LIBADD = $(top_builddir)/common/libcommon.la AM_CFLAGS += -I$(top_srcdir)/include -I$(top_srcdir)/include/arch/$(target_cpu) +# 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 new file mode 100644 index 00000000..e1c0ea73 --- /dev/null +++ b/lib/insn_queue.c @@ -0,0 +1,172 @@ +/* + * 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 c8ba19ad..b841c919 100644 --- a/lib/ulp.c +++ b/lib/ulp.c @@ -36,6 +36,7 @@ #include "config.h" #include "error.h" +#include "insn_queue_lib.h" #include "interpose.h" #include "msg_queue.h" #include "ulp.h" @@ -64,7 +65,8 @@ begin(void) * * 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. + * circunvent this by queuing up an instruction to the ulp insn queue + * for the `ulp` tool to process later. * * @param dest Destination address * @param src Source address @@ -74,21 +76,11 @@ begin(void) 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); + error_t e = insnq_insert_write(dest, n, src); + if (e != ENONE) { + return 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; } @@ -160,6 +152,10 @@ __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 @@ -172,10 +168,6 @@ __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 @@ -194,6 +186,10 @@ __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 @@ -206,10 +202,6 @@ __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 @@ -1121,7 +1113,7 @@ push_new_detour(unsigned long universe, unsigned char *patch_id, detour = calloc(1, sizeof(struct ulp_detour)); if (!detour) { - WARN("Unable to acllocate memory for ulp detour"); + WARN("Unable to allocate memory for ulp detour"); return 0; } diff --git a/tests/Makefile.am b/tests/Makefile.am index bc2a15e0..7d58701b 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -444,6 +444,7 @@ check_PROGRAMS = \ pcqueue \ comments \ block_mprotect \ + insn_queue \ chroot \ visibility @@ -562,16 +563,6 @@ dlsym_SOURCES = dlsym.c dlsym_LDADD = -lpthread -ldl -lrt dlsym_DEPENDENCIES = $(POST_PROCESS) $(METADATA) -# Workaround a bug in Autoconf 2.69 -if CPU_X86_64 -dlsym_LDADD += \ - -l:ld-linux-x86-64.so.2 -endif -if CPU_PPC64LE -dlsym_LDADD += \ - -l:ld64.so.2 -endif - stress_SOURCES = stress.c stress_LDADD = libstress.la stress_DEPENDENCIES = $(POST_PROCESS) $(METADATA) @@ -589,6 +580,22 @@ 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 = + +# Workaround a bug in Autoconf 2.69 +if CPU_X86_64 +dlsym_LDADD += \ + -l:ld-linux-x86-64.so.2 +insn_queue_CFLAGS += -I$(srcdir)/../include/arch/x86_64/ +endif +if CPU_PPC64LE +dlsym_LDADD += \ + -l:ld64.so.2 +insn_queue_CFLAGS += -I$(srcdir)/../include/powerpc64le/x86_64/ +endif + chroot_SOURCES = chroot.c chroot_CFLAGS = $(AM_CFLAGS) chroot_LDADD = libparameters.la @@ -648,6 +655,7 @@ 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 new file mode 100644 index 00000000..5fa2bd1c --- /dev/null +++ b/tests/insn_queue.c @@ -0,0 +1,292 @@ +#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 new file mode 100644 index 00000000..70ac7407 --- /dev/null +++ b/tests/insn_queue.py @@ -0,0 +1,27 @@ +#!/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 390a0241..88fb075f 100644 --- a/tools/insn_queue.c +++ b/tools/insn_queue.c @@ -271,8 +271,7 @@ 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, or is too new and - the instruction queue was overcame. */ + /* Libpulp is old and do not have a instruction queue. */ return EOLDLIBPULP; } diff --git a/tools/trigger.c b/tools/trigger.c index a1de2b43..28638bf0 100644 --- a/tools/trigger.c +++ b/tools/trigger.c @@ -192,19 +192,11 @@ trigger_one_process(struct ulp_process *target, int retries, "fatal error during live patch application (hijacked execution)."); 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 (result) + DEBUG("live patching %d failed (attempt #%d).", target->pid, + (retries - retry)); + else + retry = 0; } #if defined ENABLE_STACK_CHECK && ENABLE_STACK_CHECK range_check_failed: