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

Richard Weinberger
2015-12-21T10:13:44Z

[-- Attachment #1: Type: text/plain, Size: 1090 bytes --]

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

[-- Attachment #2: ptsc.c --]
[-- Type: text/x-csrc, Size: 4641 bytes --]

#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;
}