From 5ed0da18d9055b0428e5823d8a19ecae754fb66a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Bednarski?= Date: Wed, 25 Oct 2017 18:41:29 +0200 Subject: [PATCH 1/6] Initial workaround for Android Oreo seccomp policy When tracee receives SIGSYS and syscalls appears to be set_robust_list assume that it was seccomp violation and set errno to -ENOSYS instead killing target Initial workaround version, needs to be tested on other archs and I have to see if SIGSYS handling doesn't collide with anything else termux/proot#5 --- src/tracee/event.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/tracee/event.c b/src/tracee/event.c index 120a2608..4597b810 100644 --- a/src/tracee/event.c +++ b/src/tracee/event.c @@ -580,6 +580,16 @@ int handle_tracee_event(Tracee *tracee, int tracee_status) } break; + case SIGSYS: { + int sigsys_status = fetch_regs(tracee); + if (sigsys_status == 0 && get_sysnum(tracee, ORIGINAL) == PR_set_robust_list) { + signal = 0; + poke_reg(tracee, SYSARG_RESULT, -ENOSYS); + push_specific_regs(tracee, false); + } + break; + } + default: /* Deliver this signal as-is. */ break; From cb7efff1fe2129aea4aa5b905a821823d815b884 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Bednarski?= Date: Sat, 28 Oct 2017 20:05:21 +0200 Subject: [PATCH 2/6] Allow setting verbose through PROOT_VERBOSE env Effect is the same as -v option, but envrionement variable can be easily specified when proot is invoked from script without modifying that script --- src/cli/cli.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/cli/cli.c b/src/cli/cli.c index 73f0bd45..2518c9cc 100644 --- a/src/cli/cli.c +++ b/src/cli/cli.c @@ -453,6 +453,15 @@ int main(int argc, char *const argv[]) goto error; tracee->pid = getpid(); + /* Set verboseness from env variable, may be overriden by option */ + { + const char *verbose_env = getenv("PROOT_VERBOSE"); + if (verbose_env != NULL) { + tracee->verbose = strtol(verbose_env, NULL, 10); + global_verbose_level = tracee->verbose; + } + } + /* Pre-configure the first tracee. */ status = parse_config(tracee, argc, argv); if (status < 0) From e2e23dd883842a0397898f62f2bf6bc56582b2e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Bednarski?= Date: Sat, 28 Oct 2017 20:10:28 +0200 Subject: [PATCH 3/6] Check if SIGSYS was caused by seccomp using siginfo --- src/tracee/event.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/tracee/event.c b/src/tracee/event.c index 4597b810..e37abfa4 100644 --- a/src/tracee/event.c +++ b/src/tracee/event.c @@ -581,11 +581,21 @@ int handle_tracee_event(Tracee *tracee, int tracee_status) break; case SIGSYS: { - int sigsys_status = fetch_regs(tracee); - if (sigsys_status == 0 && get_sysnum(tracee, ORIGINAL) == PR_set_robust_list) { - signal = 0; + siginfo_t siginfo = {}; + ptrace(PTRACE_GETSIGINFO, tracee->pid, NULL, &siginfo); + if (siginfo.si_code == SYS_SECCOMP) { + int sigsys_fetch_status = fetch_regs(tracee); + if (sigsys_fetch_status != 0) { + VERBOSE(tracee, 1, "Couldn't fetch regs on seccomp SIGSYS"); + break; + } + print_current_regs(tracee, 3, "seccomp SIGSYS"); poke_reg(tracee, SYSARG_RESULT, -ENOSYS); + tracee->restore_original_regs = false; push_specific_regs(tracee, false); + signal = 0; + } else { + VERBOSE(tracee, 1, "non-seccomp SIGSYS"); } break; } From b41e43937e94a78276151f0c8de7a486d9a55836 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Bednarski?= Date: Sat, 4 Nov 2017 15:12:40 +0100 Subject: [PATCH 4/6] Seccomp and NT_ARM_SYSTEM_CALL workaround fixes Block signals while executing syscall chain When we set errno for syscall, but are unable to use NT_ARM_SYSTEM_CALL, intercept syscall exit (as we do when we don't use seccomp acceleration) Reset tracee->status upon receiving seccomp SIGSYS --- src/syscall/chain.c | 5 +++++ src/syscall/syscall.c | 31 +++++++++++++++++++++---------- src/tracee/event.c | 27 +++++++++++++++++++++++++-- src/tracee/tracee.h | 1 + 4 files changed, 52 insertions(+), 12 deletions(-) diff --git a/src/syscall/chain.c b/src/syscall/chain.c index d0fc89f5..f866be82 100644 --- a/src/syscall/chain.c +++ b/src/syscall/chain.c @@ -25,6 +25,7 @@ #include /* E*, */ #include /* assert(3), */ +#include "cli/note.h" #include "syscall/chain.h" #include "syscall/sysnum.h" #include "tracee/tracee.h" @@ -118,9 +119,13 @@ void chain_next_syscall(Tracee *tracee) tracee->chain.force_final_result = false; tracee->chain.final_result = 0; + VERBOSE(tracee, 2, "chain_next_syscall finish"); + return; } + VERBOSE(tracee, 2, "chain_next_syscall continue"); + /* Original register values will be restored right after the * last chained syscall. */ tracee->restore_original_regs = false; diff --git a/src/syscall/syscall.c b/src/syscall/syscall.c index 0a2ad656..4bc62edb 100644 --- a/src/syscall/syscall.c +++ b/src/syscall/syscall.c @@ -111,6 +111,8 @@ void translate_syscall(Tracee *tracee) if (status < 0) return; + int suppressed_syscall_status = 0; + if (is_enter_stage) { /* Never restore original register values at the end * of this stage. */ @@ -151,6 +153,7 @@ void translate_syscall(Tracee *tracee) * the sysexit stage (i.e. when seccomp is enabled and * there's nothing else to do). */ if (tracee->restart_how == PTRACE_CONT) { + suppressed_syscall_status = tracee->status; tracee->status = 0; poke_reg(tracee, STACK_POINTER, peek_reg(tracee, ORIGINAL, STACK_POINTER)); } @@ -187,6 +190,7 @@ void translate_syscall(Tracee *tracee) int push_regs_status = push_specific_regs(tracee, override_sysnum); /* Handle inability to change syscall number */ + print_current_regs(tracee, 4, "pre_push"); if (push_regs_status < 0 && override_sysnum) { word_t orig_sysnum = peek_reg(tracee, ORIGINAL, SYSARG_NUM); word_t current_sysnum = peek_reg(tracee, CURRENT, SYSARG_NUM); @@ -194,22 +198,29 @@ void translate_syscall(Tracee *tracee) /* Restart current syscall as chained */ if (current_sysnum != SYSCALL_AVOIDER) { restart_current_syscall_as_chained(tracee); + } else if (suppressed_syscall_status) { + /* If we've decided to fail this syscall + * by setting it to no-op and continuing, but turns out + * that we can't just make syscall nop, restore tracee->status + * and intercept syscall exit */ + tracee->status = suppressed_syscall_status; + tracee->restart_how = PTRACE_SYSCALL; } /* Set syscall arguments to make it fail - * TODO: More reliable way to make invalid arguments */ + * TODO: More reliable way to make invalid arguments + * For most syscalls we set all args to -1 + * Hoping there is among them invalid request/address/fd/value that will make syscall fail */ + poke_reg(tracee, SYSARG_1, -1); + poke_reg(tracee, SYSARG_2, -1); + poke_reg(tracee, SYSARG_3, -1); + poke_reg(tracee, SYSARG_4, -1); + poke_reg(tracee, SYSARG_5, -1); + poke_reg(tracee, SYSARG_6, -1); + if (get_sysnum(tracee, ORIGINAL) == PR_brk) { /* For brk() we pass 0 as first arg; this is used to query value without changing it */ poke_reg(tracee, SYSARG_1, 0); - } else { - /* For other syscalls we set all args to -1 - * Hoping there is among them invalid request/address/fd/value that will make syscall fail */ - poke_reg(tracee, SYSARG_1, -1); - poke_reg(tracee, SYSARG_2, -1); - poke_reg(tracee, SYSARG_3, -1); - poke_reg(tracee, SYSARG_4, -1); - poke_reg(tracee, SYSARG_5, -1); - poke_reg(tracee, SYSARG_6, -1); } /* Push regs again without changing syscall */ diff --git a/src/tracee/event.c b/src/tracee/event.c index e37abfa4..7f0a0954 100644 --- a/src/tracee/event.c +++ b/src/tracee/event.c @@ -377,7 +377,7 @@ int handle_tracee_event(Tracee *tracee, int tracee_status) * seccomp) is not cleared due to an event that would happen * before the exit stage, eg. PTRACE_EVENT_EXEC for the exit * stage of execve(2). */ - if (tracee->seccomp == ENABLED && !tracee->sysexit_pending) + if (tracee->seccomp == ENABLED && !tracee->sysexit_pending && tracee->chain.syscalls == NULL) tracee->restart_how = PTRACE_CONT; else tracee->restart_how = PTRACE_SYSCALL; @@ -475,6 +475,14 @@ int handle_tracee_event(Tracee *tracee, int tracee_status) if (!tracee->seccomp_already_handled_enter) { translate_syscall(tracee); + + /* Redeliver signal suppressed during + * syscall chain once it's finished. */ + if (tracee->chain.suppressed_signal && tracee->chain.syscalls == NULL) { + signal = tracee->chain.suppressed_signal; + tracee->chain.suppressed_signal = 0; + VERBOSE(tracee, 6, "vpid %" PRIu64 ": redelivering suppressed signal %d", tracee->vpid, signal); + } } else { assert(!IS_IN_SYSENTER(tracee)); @@ -584,6 +592,7 @@ int handle_tracee_event(Tracee *tracee, int tracee_status) siginfo_t siginfo = {}; ptrace(PTRACE_GETSIGINFO, tracee->pid, NULL, &siginfo); if (siginfo.si_code == SYS_SECCOMP) { + /* Set errno to -ENOSYS */ int sigsys_fetch_status = fetch_regs(tracee); if (sigsys_fetch_status != 0) { VERBOSE(tracee, 1, "Couldn't fetch regs on seccomp SIGSYS"); @@ -593,7 +602,13 @@ int handle_tracee_event(Tracee *tracee, int tracee_status) poke_reg(tracee, SYSARG_RESULT, -ENOSYS); tracee->restore_original_regs = false; push_specific_regs(tracee, false); + + /* Swallow signal */ signal = 0; + + /* Reset status so next SIGTRAP | 0x80 is + * recognized as syscall entry */ + tracee->status = 0; } else { VERBOSE(tracee, 1, "non-seccomp SIGSYS"); } @@ -601,7 +616,15 @@ int handle_tracee_event(Tracee *tracee, int tracee_status) } default: - /* Deliver this signal as-is. */ + /* Deliver this signal as-is, + * unless we're chaining syscall. */ + if (tracee->chain.syscalls != NULL) { + VERBOSE(tracee, 5, + "vpid %" PRIu64 ": suppressing signal during chain signal=%d, prev suppressed_signal=%d", + tracee->vpid, signal, tracee->chain.suppressed_signal); + tracee->chain.suppressed_signal = signal; + signal = 0; + } break; } } diff --git a/src/tracee/tracee.h b/src/tracee/tracee.h index ecd6e9bc..9edd413d 100644 --- a/src/tracee/tracee.h +++ b/src/tracee/tracee.h @@ -202,6 +202,7 @@ typedef struct tracee { SYSNUM_WORKAROUND_PROCESS_FAULTY_CALL, SYSNUM_WORKAROUND_PROCESS_REPLACED_CALL } sysnum_workaround_state; + int suppressed_signal; } chain; /* Load info generated during execve sysenter and used during From 16a6181f31d005044b5a04b49ab978780b7a0334 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Bednarski?= Date: Sat, 11 Nov 2017 18:22:03 +0100 Subject: [PATCH 5/6] Comment out assertion failing on /proc/device-tree https://github.com/termux/termux-packages/issues/1679#issuecomment-337707411 --- src/path/proc.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/path/proc.c b/src/path/proc.c index dc49611c..1c816dfd 100644 --- a/src/path/proc.c +++ b/src/path/proc.c @@ -49,7 +49,10 @@ Action readlink_proc(const Tracee *tracee, char result[PATH_MAX], int status; pid_t pid; - assert(comparison == compare_paths("/proc", base)); + /* TODO: Following assertion fails on some devices + * https://github.com/termux/termux-packages/issues/1679 + */ + //assert(comparison == compare_paths("/proc", base)); /* Remember: comparison = compare_paths("/proc", base) */ switch (comparison) { From ada5d05ea5df176f0d357302a4ef54b296c5cf82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Bednarski?= Date: Sun, 12 Nov 2017 10:44:35 +0100 Subject: [PATCH 6/6] Define SYS_SECCOMP if it's not available --- src/tracee/event.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/tracee/event.c b/src/tracee/event.c index 7f0a0954..f419a81e 100644 --- a/src/tracee/event.c +++ b/src/tracee/event.c @@ -48,6 +48,10 @@ #include "attribute.h" #include "compat.h" +#ifndef SYS_SECCOMP +#define SYS_SECCOMP 1 +#endif + /** * Start @tracee->exe with the given @argv[]. This function * returns -errno if an error occurred, otherwise 0.