Re: [PATCH v1 1/4] um: Fix ptrace GETREGS/SETREGS bugs

[Richard Weinberger <richard@xxxxxx>]

Am 21.12.2015 um 10:23 schrieb Mickaël Salaün:
>>>> Doesn't this break the support for changing syscall numbers using PTRACE_SETREGS?
>>>
>>> The logic is unchanged except updating the UPT_SYSCALL_NR before syscall_trace_enter(). I did my last tests with the x86_32 subarchitecture and all tests (from selftest/seccomp), including PTRACE_SETREGS for syscall numbers tests, passed. However, 2 of this tests still fail for x86_64 (only).
>>
>> No, the logic is different.
>> syscall_trace_enter(regs) enters the ptrace() path and here registers can be changed.
>> Hence "syscall = UPT_SYSCALL_NR(r);" will see the old syscall number.
>> UPT_SYSCALL_NR() returns the syscall number before the ptrace() path...
> 
> The thing is, PTRACE_SETREGS give access to *orig_ax* in the user_regs_struct from arch/x86/include/asm/user_*.h and selftest/seccomp only update this (virtual) register, not the EAX/RAX. Am I missing something?

Sorry, meant orig...

Please see the attached program. It proves that your patch is breaking stuff.
The test is extracted from UML's selftests.

Thanks,
//richard

#include <stdio.h>
#include <unistd.h>
#include <stdlib.h>
#include <errno.h>
#include <signal.h>
#include <sys/ptrace.h>
#include <sys/types.h>
#include <sys/wait.h>
#include <sys/user.h>
#include <sys/syscall.h>
#include <sys/reg.h>

#define CATCH_EINTR(expr) while ((errno = 0, ((expr) < 0)) && (errno == EINTR))
#define PT_OFFSET(r) ((r) * sizeof(long))
#define PT_SYSCALL_NR_OFFSET PT_OFFSET(ORIG_RAX)
#define PTRACE_OLDSETOPTIONS      21

static void ptrace_child(void)
{
        int ret;
        int pid = syscall(__NR_getpid), ppid = getppid();
        int sc_result;

        if (ptrace(PTRACE_TRACEME, 0, 0, 0) < 0) {
                perror("ptrace");
                kill(pid, SIGKILL);
        }
        kill(pid, SIGSTOP);

        /*
         * This syscall will be intercepted by the parent. Don't call more than
         * once, please.
         */
        sc_result = syscall(__NR_getpid);

        if (sc_result == pid)
                /* Nothing modified by the parent, we are running normally. */
                ret = 1;
        else if (sc_result == ppid)
                /*
                 * Expected in check_ptrace and check_sysemu when they succeed
                 * in modifying the stack frame
                 */
                ret = 0;
        else
                /* Serious trouble! This could be caused by a bug in host 2.6
                 * SKAS3/2.6 patch before release -V6, together with a bug in
                 * the UML code itself.
                 */
                ret = 2;

        exit(ret);
}

static int stop_ptraced_child(int pid, int exitcode, int mustexit)
{
        int status, n, ret = 0;

        if (ptrace(PTRACE_CONT, pid, 0, 0) < 0) {
                perror("stop_ptraced_child : ptrace failed");
                return -1;
        }
        CATCH_EINTR(n = waitpid(pid, &status, 0));
        if (!WIFEXITED(status) || (WEXITSTATUS(status) != exitcode)) {
                int exit_with = WEXITSTATUS(status);
                if (exit_with == 2)
                        printf("check_ptrace : child exited with status 2. "
                                  "\nDisabling SYSEMU support.\n");
                printf("check_ptrace : child exited with exitcode %d, while "
                          "expecting %d; status 0x%x\n", exit_with,
                          exitcode, status);
                if (mustexit)
                        exit(1);
                ret = -1;
        }

        return ret;
}


static int start_ptraced_child(void)
{
        int pid, n, status;

        pid = fork();
        if (pid == 0)
                ptrace_child();
        else if (pid < 0)
                printf("start_ptraced_child : fork failed");

        CATCH_EINTR(n = waitpid(pid, &status, WUNTRACED));
        if (n < 0)
                printf("check_ptrace : waitpid failed");
        if (!WIFSTOPPED(status) || (WSTOPSIG(status) != SIGSTOP))
                printf("check_ptrace : expected SIGSTOP, got status = %d",
                      status);

        return pid;
}

static void check_ptrace(void)
{       
        int pid, syscall, n, status;
        
        printf("Checking that ptrace can change system call numbers...");
	fflush(stdout);

        pid = start_ptraced_child();

        if ((ptrace(PTRACE_OLDSETOPTIONS, pid, 0,
                   (void *) PTRACE_O_TRACESYSGOOD) < 0))
                printf("check_ptrace: PTRACE_OLDSETOPTIONS failed");

        while (1) {
                if (ptrace(PTRACE_SYSCALL, pid, 0, 0) < 0)
                        printf("check_ptrace : ptrace failed");
                
                CATCH_EINTR(n = waitpid(pid, &status, WUNTRACED));
                if (n < 0)
                        printf("check_ptrace : wait failed");
                
                if (!WIFSTOPPED(status) ||
                   (WSTOPSIG(status) != (SIGTRAP | 0x80)))
                        printf("check_ptrace : expected (SIGTRAP|0x80), "
                               "got status = %d", status);
                
                syscall = ptrace(PTRACE_PEEKUSER, pid, PT_SYSCALL_NR_OFFSET,
                                 0);
                if (syscall == __NR_getpid) {
                        n = ptrace(PTRACE_POKEUSER, pid, PT_SYSCALL_NR_OFFSET,
                                   __NR_getppid);
                        if (n < 0)
                                printf("check_ptrace : failed to modify "
                                             "system call");
                        break;
                }
        }
        stop_ptraced_child(pid, 0, 1);
        printf("OK\n");
}

int main(void)
{
	check_ptrace();

	return 0;
}