From f76ef068f1209f878296cfa83109298f3024cb08 Mon Sep 17 00:00:00 2001 From: Gabor Buella Date: Tue, 8 Aug 2017 14:04:13 +0200 Subject: [PATCH] Better detection of error codes in syscall return values It was problematic before, especially e.g. the case in xmmap_anon: the function checked for MMAP_FAILED, but that is only correct while using the libc mmap interface. The raw syscall on Linux can return one of several negative integers to indicate errors. --- doc/generated/libsyscall_intercept.3 | 29 +++++++++--- doc/libsyscall_intercept.3.md | 13 ++++++ include/libsyscall_intercept_hook_point.h | 14 ++++++ src/intercept.c | 56 ++++++++++++++++++++--- src/intercept.h | 6 ++- src/intercept_desc.c | 6 +-- src/intercept_util.c | 33 +++++++------ src/patcher.c | 27 +++++++---- 8 files changed, 139 insertions(+), 45 deletions(-) diff --git a/doc/generated/libsyscall_intercept.3 b/doc/generated/libsyscall_intercept.3 index f212c6d7..04295d54 100644 --- a/doc/generated/libsyscall_intercept.3 +++ b/doc/generated/libsyscall_intercept.3 @@ -98,17 +98,32 @@ after a clone syscall creating a thread returns in a child thread: .IP .nf \f[C] -long\ (*intercept_hook_point_clone_child)(void); +void\ (*intercept_hook_point_clone_child)(void); \f[] .fi .PP Using \f[C]intercept_hook_point_clone_child\f[], one can be notified of -thread creations easily, and even override the syscall\[aq]s return -value in the child thread (which is normally zero). -Note: overriding the zero return value is discouraged \-\- this syscall -is usually issued by libc, and a non\-zero return value is interpreted -as the execution remaining in the parent thread, thus using attempting -to use the same stack space as the actual parent thread. +thread creations. +.PP +To make it easy to detect syscall return values indicating errors, one +can use the syscall_error_code function: +.IP +.nf +\f[C] +int\ syscall_error_code(long\ result); +\f[] +.fi +.PP +When passed a return value from syscall_no_intercept, this function can +translate it to an error code equivalent to a libc error code: +.IP +.nf +\f[C] +int\ fd\ =\ (int)syscall_no_intercept(SYS_open,\ "file",\ O_RDWR); +if\ (syscall_error_code(fd)\ !=\ 0) +\ \ \ \ fprintf(stderr,\ strerror(syscall_error_code(fd))); +\f[] +.fi .SH ENVIRONMENT VARIABLES .PP Three environment variables control the operation of the library: diff --git a/doc/libsyscall_intercept.3.md b/doc/libsyscall_intercept.3.md index dd465ef3..a5af9014 100644 --- a/doc/libsyscall_intercept.3.md +++ b/doc/libsyscall_intercept.3.md @@ -101,6 +101,19 @@ void (*intercept_hook_point_clone_child)(void); Using `intercept_hook_point_clone_child`, one can be notified of thread creations. +To make it easy to detect syscall return values indicating errors, one +can use the syscall_error_code function: +```c +int syscall_error_code(long result); +``` +When passed a return value from syscall_no_intercept, this function +can translate it to an error code equivalent to a libc error code: +```c +int fd = (int)syscall_no_intercept(SYS_open, "file", O_RDWR); +if (syscall_error_code(fd) != 0) + fprintf(stderr, strerror(syscall_error_code(fd))); +``` + # ENVIRONMENT VARIABLES # Three environment variables control the operation of the library: diff --git a/include/libsyscall_intercept_hook_point.h b/include/libsyscall_intercept_hook_point.h index 10d786b7..55c16f47 100644 --- a/include/libsyscall_intercept_hook_point.h +++ b/include/libsyscall_intercept_hook_point.h @@ -69,6 +69,20 @@ extern void (*intercept_hook_point_clone_child)(void); */ long syscall_no_intercept(long syscall_number, ...); +/* + * syscall_error_code - examines a return value from + * syscall_no_intercept, and returns an error code if said + * return value indicates an error. + */ +static inline int +syscall_error_code(long result) +{ + if (result < 0 && result >= -0x1000) + return (int)-result; + + return 0; +} + /* * The syscall intercepting library checks for the * INTERCEPT_HOOK_CMDLINE_FILTER environment variable, with which one can diff --git a/src/intercept.c b/src/intercept.c index 0e7488da..94d955bd 100644 --- a/src/intercept.c +++ b/src/intercept.c @@ -448,21 +448,65 @@ log_header(void) } /* - * xabort - speaks for itself + * xabort_errno - print a message to stderr, and exit the process. * Calling abort() in libc might result other syscalls being called * by libc. + * + * If error_code is not zero, it is also printed. */ void -xabort(const char *msg) +xabort_errno(int error_code, const char *msg) { static const char main_msg[] = " libsyscall_intercept error\n"; - if (msg != NULL) - syscall_no_intercept(SYS_write, 2, msg, strlen(msg)); - syscall_no_intercept(SYS_write, 2, main_msg, sizeof(main_msg)); + if (msg != NULL) { + /* not using libc - inline strlen */ + size_t len = 0; + while (msg[len] != '\0') + ++len; + syscall_no_intercept(SYS_write, 2, msg, len); + } + + if (error_code != 0) { + char buf[0x10]; + size_t len = 1; + char *c = buf + sizeof(buf) - 1; + + /* not using libc - inline sprintf */ + do { + *c-- = error_code % 10; + ++len; + error_code /= 10; + } while (error_code != 0); + *c = ' '; + + syscall_no_intercept(SYS_write, 2, c, len); + } + + syscall_no_intercept(SYS_write, 2, main_msg, sizeof(main_msg) - 1); syscall_no_intercept(SYS_exit_group, 1); - __builtin_trap(); + __builtin_unreachable(); +} + +/* + * xabort - print a message to stderr, and exit the process. + */ +void +xabort(const char *msg) +{ + xabort_errno(0, msg); +} + +/* + * xabort_on_syserror -- examines the return value of syscall_no_intercept, + * and calls xabort_errno if the said return value indicates an error. + */ +void +xabort_on_syserror(long syscall_result, const char *msg) +{ + if (syscall_error_code(syscall_result) != 0) + xabort_errno(syscall_error_code(syscall_result), msg); } /* diff --git a/src/intercept.h b/src/intercept.h index ffd6ee88..9cafaa33 100644 --- a/src/intercept.h +++ b/src/intercept.h @@ -62,7 +62,11 @@ void intercept_patch_with_postfix(unsigned char *syscall_addr, #define INTERCEPTOR_EXIT_CODE 111 -__attribute__((noreturn)) void xabort(const char *); +__attribute__((noreturn)) void xabort_errno(int error_code, const char *msg); + +__attribute__((noreturn)) void xabort(const char *msg); + +void xabort_on_syserror(long syscall_result, const char *msg); struct range { unsigned char *address; diff --git a/src/intercept_desc.c b/src/intercept_desc.c index 93e8f118..b2ba1543 100644 --- a/src/intercept_desc.c +++ b/src/intercept_desc.c @@ -64,11 +64,7 @@ open_orig_file(const struct intercept_desc *desc) fd = syscall_no_intercept(SYS_open, desc->path, O_RDONLY); - if (fd < 0) { - syscall_no_intercept(SYS_write, 2, - desc->path, strlen(desc->path)); - xabort(" open_orig_file"); - } + xabort_on_syserror(fd, __func__); return fd; } diff --git a/src/intercept_util.c b/src/intercept_util.c index 416fa566..b2790499 100644 --- a/src/intercept_util.c +++ b/src/intercept_util.c @@ -32,6 +32,7 @@ #include "intercept_util.h" #include "intercept.h" +#include "libsyscall_intercept_hook_point.h" #include #include @@ -53,34 +54,33 @@ static int log_fd = -1; void * xmmap_anon(size_t size) { - void *addr = (void *) syscall_no_intercept(SYS_mmap, + long addr = syscall_no_intercept(SYS_mmap, NULL, size, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANON, -1, (off_t)0); - if (addr == MAP_FAILED) - xabort("xmmap_anon"); + xabort_on_syserror(addr, __func__); - return addr; + return (void *) addr; } void * xmremap(void *addr, size_t old, size_t new) { - addr = (void *) syscall_no_intercept(SYS_mremap, addr, + long new_addr = syscall_no_intercept(SYS_mremap, addr, old, new, MREMAP_MAYMOVE); - if (addr == MAP_FAILED) - xabort("xmremap"); + xabort_on_syserror(new_addr, __func__); - return addr; + return (void *) new_addr; } void xmunmap(void *addr, size_t len) { - if (syscall_no_intercept(SYS_munmap, addr, len) != 0) - xabort("xmunmap"); + long result = syscall_no_intercept(SYS_munmap, addr, len); + + xabort_on_syserror(result, __func__); } long @@ -88,8 +88,7 @@ xlseek(long fd, unsigned long off, int whence) { long result = syscall_no_intercept(SYS_lseek, fd, off, whence); - if (result < 0) - xabort("xlseek"); + xabort_on_syserror(result, __func__); return result; } @@ -97,9 +96,10 @@ xlseek(long fd, unsigned long off, int whence) void xread(long fd, void *buffer, size_t size) { - if (syscall_no_intercept(SYS_read, fd, - buffer, size) != (ssize_t)size) - xabort("xread"); + long result = syscall_no_intercept(SYS_read, fd, buffer, size); + + if (result != (long)size) + xabort_errno(syscall_error_code(result), __func__); } /* @@ -132,8 +132,7 @@ intercept_setup_log(const char *path_base, const char *trunc) log_fd = (int)syscall_no_intercept(SYS_open, path, flags, (mode_t)0700); - if (log_fd < 0) - xabort("setup_log"); + xabort_on_syserror(log_fd, "opening log"); } /* diff --git a/src/patcher.c b/src/patcher.c index 82abeead..ca342b13 100644 --- a/src/patcher.c +++ b/src/patcher.c @@ -808,6 +808,15 @@ after_nop(const struct range *nop) return nop->address + nop->size; } +static void +mprotect_no_intercept(void *addr, size_t len, int prot, + const char *msg_on_error) +{ + long result = syscall_no_intercept(SYS_mprotect, addr, len, prot); + + xabort_on_syserror(result, msg_on_error); +} + /* * activate_patches() * Loop over all the patches, and and overwrite each syscall. @@ -824,9 +833,9 @@ activate_patches(struct intercept_desc *desc) first_page = round_down_address(desc->text_start); size = (size_t)(desc->text_end - first_page); - if (syscall_no_intercept(SYS_mprotect, first_page, size, - PROT_READ | PROT_WRITE | PROT_EXEC) != 0) - xabort("mprotect PROT_READ | PROT_WRITE | PROT_EXEC"); + mprotect_no_intercept(first_page, size, + PROT_READ | PROT_WRITE | PROT_EXEC, + "mprotect PROT_READ | PROT_WRITE | PROT_EXEC"); for (unsigned i = 0; i < desc->count; ++i) { const struct patch_desc *patch = desc->items + i; @@ -902,9 +911,9 @@ activate_patches(struct intercept_desc *desc) } } - if (syscall_no_intercept(SYS_mprotect, first_page, size, - PROT_READ | PROT_EXEC) != 0) - xabort("mprotect PROT_READ | PROT_EXEC"); + mprotect_no_intercept(first_page, size, + PROT_READ | PROT_EXEC, + "mprotect PROT_READ | PROT_EXEC"); } /* @@ -941,9 +950,9 @@ next_asm_wrapper_space(void) void mprotect_asm_wrappers(void) { - if (syscall_no_intercept(SYS_mprotect, + mprotect_no_intercept( round_down_address(asm_wrapper_space + PAGE_SIZE), sizeof(asm_wrapper_space) - PAGE_SIZE, - PROT_READ | PROT_EXEC) != 0) - xabort("mprotect_asm_wrappers PROT_READ | PROT_EXEC"); + PROT_READ | PROT_EXEC, + "mprotect_asm_wrappers PROT_READ | PROT_EXEC"); }