ptrace interface does not permit modification of syscall return

ptrace interface does not permit modification of syscall return

[Mike Frysinger]

[-- Attachment #1.1: Type: text/plain, Size: 1529 bytes --]

i have a ptrace program that watches for specific syscalls and when
matched, will:
 - on entry change the syscall nr to -1 (so the kernel will skip it)
 - on exit change the return to -EPERM so the userspace sees a denial

i have this working on most arches (x86, x86_64, arm, alpha, ia64, etc...).
on parisc, the kernel (using 3.18.7 currently) appears to be wrong.  in my
tests, if i don't mess with the syscall nr, i can change the return value
fine (to EPERM or whatever).  but the syscall executed which i do not want.
if i change the syscall to -1, then i can't change the return value (so the
child sees ENOSYS), but the kernel still executes the original syscall.

i have a simple test case attached to show the issue.  the code does:
 - spawn a child with the parent tracing it
 - child will do:
  - dupe stderr to another fd
  - unlink a file named ".test.flag"
  - write a message through the new fd
  - close a magic # so the parent knows to start denying
    - should see EPERM but it sees ENOSYS
  - close the new fd
    - should see EPERM but it is closed!
  - write to the new fd
    - should work, but the fd is closed
  - call create on ".test.flag"
    - should see EPERM, but the file is created!
 - parent will do:
  - log the syscalls until child runs close(-12345)
  - the parent will then try to deny all close/creat calls
  - uses PTRACE_POKEUSER w/PT_GR20 to set syscall to -1
  - uses PTRACE_POKEUSER w/PT_GR28 to set return to -EPERM

you can run the test case by doing:
$ gcc test.c && ./a.out
-mike

[-- Attachment #1.2: test.c --]
[-- Type: text/x-c, Size: 5650 bytes --]

#define _GNU_SOURCE

#include <err.h>
#include <errno.h>
#include <fcntl.h>
#include <sched.h>
#include <signal.h>
#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <sys/ptrace.h>
#include <sys/syscall.h>
#include <sys/wait.h>
#include <asm/offset.h>
#include <asm/ptrace.h>

static pid_t trace_pid;

static long _do_ptrace(enum __ptrace_request request, const char *srequest, void *addr, void *data)
{
	long ret;
 try_again:
	errno = 0;
	ret = ptrace(request, trace_pid, addr, data);
	if (ret == -1) {
		/* Child hasn't gotten to the next marker yet ? */
		if (errno == ESRCH) {
			int status;
			if (waitpid(trace_pid, &status, 0) == -1) {
				/* nah, it's dead ... should we whine though ? */
				_exit(0);
			}
			sched_yield();
			goto try_again;
		} else if (errno == EIO || errno == EFAULT) {
			/* This comes up when the child itself tries to use a bad pointer.
			 * That's not something the sandbox should abort on. #560396
			 */
			return ret;
		} else if (!errno)
			if (request == PTRACE_PEEKDATA ||
			    request == PTRACE_PEEKTEXT ||
			    request == PTRACE_PEEKUSER)
				return ret;

		err(1, "do_ptrace: ptrace(%s, ..., %p, %p)", srequest, addr, data);
	}
	return ret;
}
#define do_ptrace(request, addr, data) _do_ptrace(request, #request, addr, data)

static long do_peekuser(long offset)
{
	return do_ptrace(PTRACE_PEEKUSER, (void *)offset, NULL);
}

static long do_pokeuser(long offset, long val)
{
	return do_ptrace(PTRACE_POKEUSER, (void *)offset, (void *)val);
}

static void trace_child_signal(int signo, siginfo_t *info, void *context)
{
#if 0
	warnx("got sig %s(%i): code:%s(%i) status:%s(%i)",
		strsignal(signo), signo,
		"---", info->si_code,
		strsignal(info->si_status), info->si_status);
#endif

	switch (info->si_code) {
		case CLD_DUMPED:
		case CLD_KILLED:
			_exit(128 + info->si_status);

		case CLD_EXITED:
			_exit(info->si_status);

		case CLD_TRAPPED:
			switch (info->si_status) {
				case SIGSTOP:
					kill(trace_pid, SIGCONT);
				case SIGTRAP:
				case SIGCONT:
					return;
			}

			/* For whatever signal the child caught, let's ignore it and
			 * continue on.  If it aborted, segfaulted, whatever, that's
			 * its problem, not ours, so don't whine about it.  We just
			 * have to be sure to bubble it back up.  #265072
			 */
			do_ptrace(PTRACE_CONT, NULL, (void *)(long)info->si_status);
			return;
	}

	errx(1, "unhandled signal case");
}

static const char *lookup_syscall(long nr)
{
	switch (nr) {
#define X(n) case SYS_##n: return #n;
	X(access)
	X(brk)
	X(close)
	X(creat)
	X(dup)
	X(fstat64)
	X(mmap)
	X(mprotect)
	X(munmap)
	X(open)
	X(read)
	X(uname)
	X(write)
#undef X
	}
	return "";
}

void child_main(void)
{
	char test_file[] = ".test.flag";
	char msg[] = "child: you should see two of these\n";
	int fd = dup(2);

	unlink(test_file);
	write(fd, msg, sizeof(msg));

	/* Marker for the parent to watch. */
	errno = 0;
	close(-12345);
	fprintf(stderr, "child: close marker (should be EPERM): %m\n");
	errno = 0;
	close(fd);
	fprintf(stderr, "child: real close (should be EPERM): %m\n");
	errno = 0;
	write(fd, msg, sizeof(msg));
	fprintf(stderr, "child: write (should be success): %m\n");
	errno = 0;
	creat(test_file, 0660);
	fprintf(stderr, "child: creat (should be EPERM): %m\n");
	errno = 0;
	access(test_file, F_OK);
	fprintf(stderr, "child: access (should be ENOENT): %m\n");

	unlink(test_file);
	exit(0);
}

static void parent_main(void)
{
	int status;
	long nr, arg1;

	/* Wait for the child to exec. */
	while (1) {
		do_ptrace(PTRACE_SYSCALL, NULL, NULL);
		waitpid(trace_pid, &status, 0);

		unsigned event = ((unsigned)status >> 16);
		if (event == PTRACE_EVENT_EXEC) {
			warnx("parent: hit exec!");
			break;
		} else
			warnx("parent: waiting for exec; status: %#x", status);
	}

	/* Main loop. */
	bool saw_close = false;
	bool before_syscall = false;
	bool fake_syscall_ret = false;
	while (1) {
		do_ptrace(PTRACE_SYSCALL, NULL, NULL);
		waitpid(trace_pid, &status, 0);

		nr = do_peekuser(PT_GR20);
		arg1 = do_peekuser(PT_GR26);
		if (before_syscall) {
			warnx("parent: NR:%3li %s", nr, lookup_syscall(nr));
			/* Once the child hits the marker, deny all close & creat calls */
			if (nr == __NR_close || nr == __NR_creat) {
				if (saw_close || arg1 == -12345) {
					saw_close = true;
					warnx("parent: setting NR to -1");
					do_pokeuser(PT_GR20, -1);
					fake_syscall_ret = true;
				}
			}
		} else if (fake_syscall_ret) {
			warnx("parent: forcing EPERM");
			do_pokeuser(PT_GR28, -EPERM);
			fake_syscall_ret = false;
		}

		before_syscall = !before_syscall;
	}
}

int main(int argc, char *argv[])
{
	struct sigaction sa, old_sa;

	/* Child will re-exec us so the ptrace is clean for the parent. */
	if (argc > 1)
		child_main();

	/* Set up signal handler to watch for child events. */
	sa.sa_flags = SA_RESTART | SA_SIGINFO;
	sa.sa_sigaction = trace_child_signal;
	sigaction(SIGCHLD, &sa, &old_sa);

	/* Fork a child and have the parent do some early ptrace init. */
	trace_pid = fork();
	if (trace_pid == -1) {
		err(1, "fork() failed");
	} else if (trace_pid) {
		warn("parent waiting for child (pid=%i) to signal", trace_pid);
		waitpid(trace_pid, NULL, 0);
		do_ptrace(PTRACE_SETOPTIONS, NULL,
			(void *)(PTRACE_O_TRACESYSGOOD | PTRACE_O_TRACEEXEC));
		parent_main();
		errx(1, "child should have quit, as should we");
	}

	/* Have the child set itself up for tracing before execing again. */
	warnx("child setting up ...");
	sigaction(SIGCHLD, &old_sa, NULL);
	do_ptrace(PTRACE_TRACEME, NULL, NULL);
	kill(getpid(), SIGSTOP);
	execl(argv[0], argv[0], "--child", NULL);

	return 0;
}

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: ptrace interface does not permit modification of syscall return

[Helge Deller]

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

Hi Mike,

On 21.12.2015 18:55, Mike Frysinger wrote:
> i have a ptrace program that watches for specific syscalls and when
> matched, will:
>  - on entry change the syscall nr to -1 (so the kernel will skip it)
>  - on exit change the return to -EPERM so the userspace sees a denial
> 
> i have this working on most arches (x86, x86_64, arm, alpha, ia64, etc...).
> on parisc, the kernel (using 3.18.7 currently) appears to be wrong.  in my
> tests, if i don't mess with the syscall nr, i can change the return value
> fine (to EPERM or whatever).  but the syscall executed which i do not want.
> if i change the syscall to -1, then i can't change the return value (so the
> child sees ENOSYS), but the kernel still executes the original syscall.
> 
> i have a simple test case attached to show the issue.  the code does:
>  - spawn a child with the parent tracing it
>  - child will do:
>   - dupe stderr to another fd
>   - unlink a file named ".test.flag"
>   - write a message through the new fd
>   - close a magic # so the parent knows to start denying
>     - should see EPERM but it sees ENOSYS
>   - close the new fd
>     - should see EPERM but it is closed!
>   - write to the new fd
>     - should work, but the fd is closed
>   - call create on ".test.flag"
>     - should see EPERM, but the file is created!
>  - parent will do:
>   - log the syscalls until child runs close(-12345)
>   - the parent will then try to deny all close/creat calls
>   - uses PTRACE_POKEUSER w/PT_GR20 to set syscall to -1
>   - uses PTRACE_POKEUSER w/PT_GR28 to set return to -EPERM
> 
> you can run the test case by doing:
> $ gcc test.c && ./a.out

I agree, something is fishy :-)

I did some tests with your testcase.
First problem I had was, that compiling failed since it didn't found the asm/offset.h header file.
Which one did you used? I know it usually should come with the kernel headers, but there it is asm-offsets.h.
If you used debian, which package did you installed?

Instead I used asm-offsets.h.
First problem: I had to install the 64bit header file. PT_GR20 in this one was much higher than it should be for 32bit userspace.

So, I used those defines (taken from the strace source package):
#define PT_GR20 (20*4)
#define PT_GR26 (26*4)
#define PT_GR28 (28*4)
#define PT_IAOQ0 (106*4)
#define PT_IAOQ1 (107*4)

With that I got those output:
root@c3000:~# ./a.out
a.out: parent waiting for child (pid=1344) to signal: Success
a.out: child setting up ...
a.out: parent: waiting for exec; status: 0x1a7f
a.out: parent: waiting for exec; status: 0x857f
a.out: parent: hit exec!
a.out: parent: NR: 45 brk
a.out: parent: NR: 59 uname
a.out: parent: NR: 33 access
a.out: parent: NR: 90 mmap
a.out: parent: NR: 33 access
a.out: parent: NR:  5 open
a.out: parent: NR:112 fstat64
a.out: parent: NR: 90 mmap
a.out: parent: NR:  6 close
a.out: parent: NR: 33 access
a.out: parent: NR:  5 open
a.out: parent: NR:  3 read
a.out: parent: NR:112 fstat64
a.out: parent: NR: 90 mmap
a.out: parent: NR: 90 mmap
a.out: parent: NR: 90 mmap
a.out: parent: NR:  6 close
a.out: parent: NR: 90 mmap
a.out: parent: NR:125 mprotect
a.out: parent: NR: 90 mmap
a.out: parent: NR:125 mprotect
a.out: parent: NR: 91 munmap
a.out: parent: NR: 41 dup
a.out: parent: NR: 10 
a.out: parent: NR:  4 write
child: you should see two of these
a.out: parent: NR:  6 close
a.out: parent: setting NR to -1
a.out: parent: forcing EPERM
child: close marker (should be EPERM): Function not implemented
a.out: parent: NR:  4 write
a.out: parent: NR:  6 close
a.out: parent: setting NR to -1
a.out: parent: forcing EPERM
child: real close (should be EPERM): Success
a.out: parent: NR:  4 write
a.out: parent: NR:  4 write
child: write (should be success): Bad file descriptor
a.out: parent: NR:  4 write
a.out: parent: NR:  8 creat
a.out: parent: setting NR to -1
a.out: parent: forcing EPERM
child: creat (should be EPERM): Success
a.out: parent: NR:  4 write
a.out: parent: NR: 33 access
child: access (should be ENOENT): Success
a.out: parent: NR:  4 write
a.out: parent: NR: 10 

Seems not like what it should be.
I need to look closer at it during the next few days.

Regarding on how to get correct 32bit PT_REG #defines/values even on 64bit kernel,
maybe the attached patch is a way to go.

Helge

[-- Attachment #2: pt_reg_32bit.patch --]
[-- Type: text/x-diff, Size: 6943 bytes --]

diff --git a/arch/parisc/include/uapi/asm/ptrace.h b/arch/parisc/include/uapi/asm/ptrace.h
index c4fa6c8..1f2f892 100644
--- a/arch/parisc/include/uapi/asm/ptrace.h
+++ b/arch/parisc/include/uapi/asm/ptrace.h
@@ -33,6 +33,26 @@ struct pt_regs {
 	unsigned long ipsw;	/* CR22 */
 };
 
+#if defined(__LP64__)
+struct compat_pt_regs {
+	unsigned int gr[32];	/* PSW is in gr[0] */
+	__u64 fr[32];
+	unsigned int sr[ 8];
+	unsigned int iasq[2];
+	unsigned int iaoq[2];
+	unsigned int cr27;
+	unsigned int pad0;     /* available for other uses */
+	unsigned int orig_r28;
+	unsigned int ksp;
+	unsigned int kpc;
+	unsigned int sar;	/* CR11 */
+	unsigned int iir;	/* CR19 */
+	unsigned int isr;	/* CR20 */
+	unsigned int ior;	/* CR21 */
+	unsigned int ipsw;	/* CR22 */
+};
+#endif
+
 /*
  * The numbers chosen here are somewhat arbitrary but absolutely MUST
  * not overlap with any of the number assigned in <linux/ptrace.h>.
diff --git a/arch/parisc/kernel/asm-offsets.c b/arch/parisc/kernel/asm-offsets.c
index d2f6257..e2a8030 100644
--- a/arch/parisc/kernel/asm-offsets.c
+++ b/arch/parisc/kernel/asm-offsets.c
@@ -240,6 +240,96 @@ int main(void)
 	DEFINE(PT_SIZE, sizeof(struct pt_regs));
 	/* PT_SZ_ALGN includes space for a stack frame. */
 	DEFINE(PT_SZ_ALGN, align_frame(sizeof(struct pt_regs), FRAME_ALIGN));
+#ifdef CONFIG_64BIT
+	COMMENT("for 32bit userspace:");
+	DEFINE(PT_32_PSW, offsetof(struct compat_pt_regs, gr[ 0]));
+	DEFINE(PT_32_GR1, offsetof(struct compat_pt_regs, gr[ 1]));
+	DEFINE(PT_32_GR2, offsetof(struct compat_pt_regs, gr[ 2]));
+	DEFINE(PT_32_GR3, offsetof(struct compat_pt_regs, gr[ 3]));
+	DEFINE(PT_32_GR4, offsetof(struct compat_pt_regs, gr[ 4]));
+	DEFINE(PT_32_GR5, offsetof(struct compat_pt_regs, gr[ 5]));
+	DEFINE(PT_32_GR6, offsetof(struct compat_pt_regs, gr[ 6]));
+	DEFINE(PT_32_GR7, offsetof(struct compat_pt_regs, gr[ 7]));
+	DEFINE(PT_32_GR8, offsetof(struct compat_pt_regs, gr[ 8]));
+	DEFINE(PT_32_GR9, offsetof(struct compat_pt_regs, gr[ 9]));
+	DEFINE(PT_32_GR10, offsetof(struct compat_pt_regs, gr[10]));
+	DEFINE(PT_32_GR11, offsetof(struct compat_pt_regs, gr[11]));
+	DEFINE(PT_32_GR12, offsetof(struct compat_pt_regs, gr[12]));
+	DEFINE(PT_32_GR13, offsetof(struct compat_pt_regs, gr[13]));
+	DEFINE(PT_32_GR14, offsetof(struct compat_pt_regs, gr[14]));
+	DEFINE(PT_32_GR15, offsetof(struct compat_pt_regs, gr[15]));
+	DEFINE(PT_32_GR16, offsetof(struct compat_pt_regs, gr[16]));
+	DEFINE(PT_32_GR17, offsetof(struct compat_pt_regs, gr[17]));
+	DEFINE(PT_32_GR18, offsetof(struct compat_pt_regs, gr[18]));
+	DEFINE(PT_32_GR19, offsetof(struct compat_pt_regs, gr[19]));
+	DEFINE(PT_32_GR20, offsetof(struct compat_pt_regs, gr[20]));
+	DEFINE(PT_32_GR21, offsetof(struct compat_pt_regs, gr[21]));
+	DEFINE(PT_32_GR22, offsetof(struct compat_pt_regs, gr[22]));
+	DEFINE(PT_32_GR23, offsetof(struct compat_pt_regs, gr[23]));
+	DEFINE(PT_32_GR24, offsetof(struct compat_pt_regs, gr[24]));
+	DEFINE(PT_32_GR25, offsetof(struct compat_pt_regs, gr[25]));
+	DEFINE(PT_32_GR26, offsetof(struct compat_pt_regs, gr[26]));
+	DEFINE(PT_32_GR27, offsetof(struct compat_pt_regs, gr[27]));
+	DEFINE(PT_32_GR28, offsetof(struct compat_pt_regs, gr[28]));
+	DEFINE(PT_32_GR29, offsetof(struct compat_pt_regs, gr[29]));
+	DEFINE(PT_32_GR30, offsetof(struct compat_pt_regs, gr[30]));
+	DEFINE(PT_32_GR31, offsetof(struct compat_pt_regs, gr[31]));
+	DEFINE(PT_32_FR0, offsetof(struct compat_pt_regs, fr[ 0]));
+	DEFINE(PT_32_FR1, offsetof(struct compat_pt_regs, fr[ 1]));
+	DEFINE(PT_32_FR2, offsetof(struct compat_pt_regs, fr[ 2]));
+	DEFINE(PT_32_FR3, offsetof(struct compat_pt_regs, fr[ 3]));
+	DEFINE(PT_32_FR4, offsetof(struct compat_pt_regs, fr[ 4]));
+	DEFINE(PT_32_FR5, offsetof(struct compat_pt_regs, fr[ 5]));
+	DEFINE(PT_32_FR6, offsetof(struct compat_pt_regs, fr[ 6]));
+	DEFINE(PT_32_FR7, offsetof(struct compat_pt_regs, fr[ 7]));
+	DEFINE(PT_32_FR8, offsetof(struct compat_pt_regs, fr[ 8]));
+	DEFINE(PT_32_FR9, offsetof(struct compat_pt_regs, fr[ 9]));
+	DEFINE(PT_32_FR10, offsetof(struct compat_pt_regs, fr[10]));
+	DEFINE(PT_32_FR11, offsetof(struct compat_pt_regs, fr[11]));
+	DEFINE(PT_32_FR12, offsetof(struct compat_pt_regs, fr[12]));
+	DEFINE(PT_32_FR13, offsetof(struct compat_pt_regs, fr[13]));
+	DEFINE(PT_32_FR14, offsetof(struct compat_pt_regs, fr[14]));
+	DEFINE(PT_32_FR15, offsetof(struct compat_pt_regs, fr[15]));
+	DEFINE(PT_32_FR16, offsetof(struct compat_pt_regs, fr[16]));
+	DEFINE(PT_32_FR17, offsetof(struct compat_pt_regs, fr[17]));
+	DEFINE(PT_32_FR18, offsetof(struct compat_pt_regs, fr[18]));
+	DEFINE(PT_32_FR19, offsetof(struct compat_pt_regs, fr[19]));
+	DEFINE(PT_32_FR20, offsetof(struct compat_pt_regs, fr[20]));
+	DEFINE(PT_32_FR21, offsetof(struct compat_pt_regs, fr[21]));
+	DEFINE(PT_32_FR22, offsetof(struct compat_pt_regs, fr[22]));
+	DEFINE(PT_32_FR23, offsetof(struct compat_pt_regs, fr[23]));
+	DEFINE(PT_32_FR24, offsetof(struct compat_pt_regs, fr[24]));
+	DEFINE(PT_32_FR25, offsetof(struct compat_pt_regs, fr[25]));
+	DEFINE(PT_32_FR26, offsetof(struct compat_pt_regs, fr[26]));
+	DEFINE(PT_32_FR27, offsetof(struct compat_pt_regs, fr[27]));
+	DEFINE(PT_32_FR28, offsetof(struct compat_pt_regs, fr[28]));
+	DEFINE(PT_32_FR29, offsetof(struct compat_pt_regs, fr[29]));
+	DEFINE(PT_32_FR30, offsetof(struct compat_pt_regs, fr[30]));
+	DEFINE(PT_32_FR31, offsetof(struct compat_pt_regs, fr[31]));
+	DEFINE(PT_32_SR0, offsetof(struct compat_pt_regs, sr[ 0]));
+	DEFINE(PT_32_SR1, offsetof(struct compat_pt_regs, sr[ 1]));
+	DEFINE(PT_32_SR2, offsetof(struct compat_pt_regs, sr[ 2]));
+	DEFINE(PT_32_SR3, offsetof(struct compat_pt_regs, sr[ 3]));
+	DEFINE(PT_32_SR4, offsetof(struct compat_pt_regs, sr[ 4]));
+	DEFINE(PT_32_SR5, offsetof(struct compat_pt_regs, sr[ 5]));
+	DEFINE(PT_32_SR6, offsetof(struct compat_pt_regs, sr[ 6]));
+	DEFINE(PT_32_SR7, offsetof(struct compat_pt_regs, sr[ 7]));
+	DEFINE(PT_32_IASQ0, offsetof(struct compat_pt_regs, iasq[0]));
+	DEFINE(PT_32_IASQ1, offsetof(struct compat_pt_regs, iasq[1]));
+	DEFINE(PT_32_IAOQ0, offsetof(struct compat_pt_regs, iaoq[0]));
+	DEFINE(PT_32_IAOQ1, offsetof(struct compat_pt_regs, iaoq[1]));
+	DEFINE(PT_32_CR27, offsetof(struct compat_pt_regs, cr27));
+	DEFINE(PT_32_ORIG_R28, offsetof(struct compat_pt_regs, orig_r28));
+	DEFINE(PT_32_KSP, offsetof(struct compat_pt_regs, ksp));
+	DEFINE(PT_32_KPC, offsetof(struct compat_pt_regs, kpc));
+	DEFINE(PT_32_SAR, offsetof(struct compat_pt_regs, sar));
+	DEFINE(PT_32_IIR, offsetof(struct compat_pt_regs, iir));
+	DEFINE(PT_32_ISR, offsetof(struct compat_pt_regs, isr));
+	DEFINE(PT_32_IOR, offsetof(struct compat_pt_regs, ior));
+	DEFINE(PT_32_SIZE, sizeof(struct compat_pt_regs));
+	/* PT_32_SZ_ALGN includes space for a stack frame. */
+	DEFINE(PT_32_SZ_ALGN, align_frame(sizeof(struct compat_pt_regs), FRAME_ALIGN));
+#endif
 	BLANK();
 	DEFINE(TI_TASK, offsetof(struct thread_info, task));
 	DEFINE(TI_FLAGS, offsetof(struct thread_info, flags));

Re: ptrace interface does not permit modification of syscall return

[Mike Frysinger]

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

On 22 Dec 2015 22:10, Helge Deller wrote:
> On 21.12.2015 18:55, Mike Frysinger wrote:
> > i have a ptrace program that watches for specific syscalls and when
> > matched, will:
> >  - on entry change the syscall nr to -1 (so the kernel will skip it)
> >  - on exit change the return to -EPERM so the userspace sees a denial
> > 
> > i have this working on most arches (x86, x86_64, arm, alpha, ia64, etc...).
> > on parisc, the kernel (using 3.18.7 currently) appears to be wrong.  in my
> > tests, if i don't mess with the syscall nr, i can change the return value
> > fine (to EPERM or whatever).  but the syscall executed which i do not want.
> > if i change the syscall to -1, then i can't change the return value (so the
> > child sees ENOSYS), but the kernel still executes the original syscall.
> > 
> > i have a simple test case attached to show the issue.  the code does:
> >  - spawn a child with the parent tracing it
> >  - child will do:
> >   - dupe stderr to another fd
> >   - unlink a file named ".test.flag"
> >   - write a message through the new fd
> >   - close a magic # so the parent knows to start denying
> >     - should see EPERM but it sees ENOSYS
> >   - close the new fd
> >     - should see EPERM but it is closed!
> >   - write to the new fd
> >     - should work, but the fd is closed
> >   - call create on ".test.flag"
> >     - should see EPERM, but the file is created!
> >  - parent will do:
> >   - log the syscalls until child runs close(-12345)
> >   - the parent will then try to deny all close/creat calls
> >   - uses PTRACE_POKEUSER w/PT_GR20 to set syscall to -1
> >   - uses PTRACE_POKEUSER w/PT_GR28 to set return to -EPERM
> > 
> > you can run the test case by doing:
> > $ gcc test.c && ./a.out
> 
> I agree, something is fishy :-)
> 
> I did some tests with your testcase.
> First problem I had was, that compiling failed since it didn't found the asm/offset.h header file.
> Which one did you used? I know it usually should come with the kernel headers, but there it is asm-offsets.h.

hmm, looks like it got installed by hand at some point (Jul 2004 datestamp!)
and never cleaned up.

> First problem: I had to install the 64bit header file. PT_GR20 in this one was much higher than it should be for 32bit userspace.
> 
> So, I used those defines (taken from the strace source package):
> #define PT_GR20 (20*4)
> #define PT_GR26 (26*4)
> #define PT_GR28 (28*4)
> #define PT_IAOQ0 (106*4)
> #define PT_IAOQ1 (107*4)

these are the values in my local asm/offset.h, and what i was using
in my original code -- the register # multiplied by 4.

> With that I got those output:

looks like you're seeing the same as me.  i'm only testing 32bit user
and 32bit kernel currently as we don't have a 64bit userspace :).
-mike

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: ptrace interface does not permit modification of syscall return

[Helge Deller]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On 28.12.2015 19:03, Mike Frysinger wrote:
> On 22 Dec 2015 22:10, Helge Deller wrote:
>> On 21.12.2015 18:55, Mike Frysinger wrote:
>>> i have a ptrace program that watches for specific syscalls and when
>>> matched, will:
>>>  - on entry change the syscall nr to -1 (so the kernel will skip it)
>>>  - on exit change the return to -EPERM so the userspace sees a denial
>>>
>>> i have this working on most arches (x86, x86_64, arm, alpha, ia64, etc...).
>>> on parisc, the kernel (using 3.18.7 currently) appears to be wrong.  in my
>>> tests, if i don't mess with the syscall nr, i can change the return value
>>> fine (to EPERM or whatever).  but the syscall executed which i do not want.
>>> if i change the syscall to -1, then i can't change the return value (so the
>>> child sees ENOSYS), but the kernel still executes the original syscall.
>>>
>>> i have a simple test case attached to show the issue.  the code does:
>>>  - spawn a child with the parent tracing it
>>>  - child will do:
>>>   - dupe stderr to another fd
>>>   - unlink a file named ".test.flag"
>>>   - write a message through the new fd
>>>   - close a magic # so the parent knows to start denying
>>>     - should see EPERM but it sees ENOSYS
>>>   - close the new fd
>>>     - should see EPERM but it is closed!
>>>   - write to the new fd
>>>     - should work, but the fd is closed
>>>   - call create on ".test.flag"
>>>     - should see EPERM, but the file is created!
>>>  - parent will do:
>>>   - log the syscalls until child runs close(-12345)
>>>   - the parent will then try to deny all close/creat calls
>>>   - uses PTRACE_POKEUSER w/PT_GR20 to set syscall to -1
>>>   - uses PTRACE_POKEUSER w/PT_GR28 to set return to -EPERM
>>>
>>> you can run the test case by doing:
>>> $ gcc test.c && ./a.out
>>
>> I agree, something is fishy :-)
>>
>> I did some tests with your testcase.
>> First problem I had was, that compiling failed since it didn't found the asm/offset.h header file.
>> Which one did you used? I know it usually should come with the kernel headers, but there it is asm-offsets.h.
> 
> hmm, looks like it got installed by hand at some point (Jul 2004 datestamp!)
> and never cleaned up.
> 
>> First problem: I had to install the 64bit header file. PT_GR20 in this one was much higher than it should be for 32bit userspace.
>>
>> So, I used those defines (taken from the strace source package):
>> #define PT_GR20 (20*4)
>> #define PT_GR26 (26*4)
>> #define PT_GR28 (28*4)
>> #define PT_IAOQ0 (106*4)
>> #define PT_IAOQ1 (107*4)
> 
> these are the values in my local asm/offset.h, and what i was using
> in my original code -- the register # multiplied by 4.
> 
>> With that I got those output:
> 
> looks like you're seeing the same as me.  i'm only testing 32bit user
> and 32bit kernel currently as we don't have a 64bit userspace :).

Mike, can you please test the patch I just posted:
https://patchwork.kernel.org/patch/8063301/

Helge
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBCAAGBQJWnlIGAAoJEKGDlV8wpRJB01AP/1e5cO5DfF2UtDGAC8SNqB1x
4uCIEyCrKyAkNbrfy3HSQ2OVdenuZnxq/z5LiQeY9k8j4IfZgpU8v7fADZz76L1z
CsVyIAwi7BQqUUeV6otWo5o1mAZ1F9dvS4jPFpxROotgmBCkhDcAYP6xGpoKbAET
VDhgwjgsZLXOuONoxfONaveZGRMjZGbm5nrvra30kfQhzilew5EmWGchWvv5QFAE
scukm0Prfi9roefagIllvZrLjGLenQ7Wa/opWrH/KxbkN3t8cQprXUv8ejEEUG8v
mCrUQRfHdbgVpzMFyCbdtoRVBRRtRl3Z9Ht8DqgYiuaW34iDA6Una/ntBC/Z0SdD
WySBINwuG6VL8gLrS7zMuKyBrhY37PV+eeo+GC0C6bNr37oPgJ1HLqOj8B2tlHU+
/EJYaLSDMXHsrukF+XSOzq9pbiCpPvU3ApzpZnPcypNMid3/6LKf6fIvu/4P/8xK
09gCZrHRXI5J65Qya6oQHP2N1YJYRJzHUct1N6hEJ01DEDfUZxkHun8EDYHC879f
RZ7UE2jNJlBAMXTel0HGthgXrqPyM18ePlA4yv5YVK1Z7Ai+wT7j+CL5hnI46x9X
71PugI5vMzZZQ+N9alW9KG2E/RAzcx1Mu/SjntoXBD7jdv1u+DiQtbhD/ahP61Pt
+4I+Icqo62/Ql3Y2r8Ia
=2LNU
-----END PGP SIGNATURE-----