ARM audit, seccomp, etc are broken wrt OABI syscalls

* ARM audit, seccomp, etc are broken wrt OABI syscalls
@ 2013-11-05 22:36 Andy Lutomirski
  2013-11-06  0:14 ` Kees Cook
  2013-11-06 15:32 ` [libseccomp-discuss] " Eric Paris
  0 siblings, 2 replies; 16+ messages in thread
From: Andy Lutomirski @ 2013-11-05 22:36 UTC (permalink / raw)
  To: Kees Cook
  Cc: Paul Moore, Richard Weinberger, libseccomp-discuss, Will Drewry,
	linux-kernel, linux-arm-kernel, Russell King

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

[cc: some ARM people]

After a bit of an adventure, I got QEMU working.  (Linux 3.12's smc91x
driver and qemu 1.6 don't get along.  It would be great if some
kernel.org page described a standard way to boot a modern Linux image
on a modern QEMU version, but I digress.)

The current state of affairs is unhealthy.  I wrote a program
(attached) that does 'svc $0x90002f' (silly GNU syntax for "Issue the
getgid syscall in OABI").  The registers I program are:

r0: 1
r1: 2
r2: 3
r3: 4
r4: 5
r5: 6

(i.e. the arguments are 1,2,3,4,5,6, although getgid ignores them)

r7: 1

(r7 is the EABI syscall register.  On a kernel without OABI support,
the immediate svc argument is ignored and syscall 1, i.e. _exit, will
be invoked).

Seccomp sees the registers as I set them (unsurprisingly) and it sees
nr == 0x2f.  It passes those values on to a SIGSYS handler, if one
exists.  This is, IMO, bad.  The OABI and EABI argument passing
conventions are *not* the same, and seccomp filters that check syscall
argument values may be spoofable by using OABI calls.

I suspect that audit, perf, ftrace, and maybe even ptrace are broken
as well for exactly the same reason.

I would argue that there are two reasonable fixes:

1. Set a different audit arch for OABI syscalls (e.g.
AUDIT_ARCH_ARMOABI).  That is, treat OABI syscall entries the same way
that x86_64 treats int 80.

2. Leave the 0x900000 bits set in the syscall nr.  That way OABI
syscalls would look like a different set of syscalls on the same
architecture.  That is, treat OABI syscall entries kind of like x86_64
treats x32 syscalls.  (There's probably no reason to accept 0x900000 +
N as an r7 value to cause 'svc 0' to invoke OABI syscall N, though.)

3. Unconditionally kill any process that makes an OABI syscall with
seccomp enabled (because there should be no such programs).  Eww.

Options 1 and 2 are both break ABI, but I doubt that anythink cares.
OABI is, AFAICT, mostly dead.  That being said, even if nothing
legitimate uses OABI, exploits against seccomp-using programs can
certainly use OABI, so I think that this needs to be fixed somehow.

Thoughts?  I think I prefer option 1.  I don't really want to make the
change because my ARM assembly skills are lacking.

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

#define _GNU_SOURCE
#include <stdio.h>
#include <unistd.h>
#include <linux/filter.h>
#include <sys/syscall.h>
#include <err.h>
#include <sys/prctl.h>
#include <stddef.h>
#include <sys/ucontext.h>

#ifndef __ARM_EABI__
#error Must be compiled for ARM EABI
#endif

struct seccomp_data {
	int nr;
	__u32 arch;
	__u64 instruction_pointer;
	__u64 args[6];
};

#ifndef PR_SET_NO_NEW_PRIVS
#define PR_SET_NO_NEW_PRIVS	38
#endif

#define SECCOMP_RET_KILL 0x00000000U
#define SECCOMP_RET_TRAP 0x00030000U
#define SECCOMP_RET_ERRNO 0x00050000U
#define SECCOMP_RET_TRACE 0x7ff00000U
#define SECCOMP_RET_ALLOW 0x7fff0000U

struct sifields_sigsys {
	void *_call_addr;
	int _syscall;
	unsigned int _arch;
};

#define SIGINFO_SIGSYS(si) ((struct sifields_sigsys *)&(si)->_sifields)

__attribute__((noinline,optimize("2"))) long call_getgid_oabi(void)
{
	// If OABI compatibility is disabled, this will call exit instead
	// (That's what r7==1 means.)
	register long ret asm("r0");
	asm volatile("mov r0, $1\n\tmov r1, $2\n\tmov r2, $3\n\t"
		     "mov r3, $4\n\tmov r4, $5\n\tmov r5, $6\n\t"
		     "mov r7, $1\n\tsvc $0x90002f\n\t" : : :
		     "r0", "r1", "r2", "r3", "r4", "r5", "r6", "r7", "memory");
	return ret;
}

void handler(int signum, siginfo_t *si, void *uc_void)
{
	const struct ucontext *uc = uc_void;
	const struct sifields_sigsys *ss = SIGINFO_SIGSYS(si);

	printf("SIGSYS\n\n");
	printf("nr: 0x%X (__NR_getgid = 0x%X, OABI nr = 0x90002F)\n",
	       ss->_syscall, __NR_getgid);

#define DO_REG(i) printf("r" #i ":  0x%08X\n", uc->uc_mcontext.arm_r##i);
	DO_REG(0);
	DO_REG(1);
	DO_REG(2);
	DO_REG(3);
	DO_REG(4);
	DO_REG(5);
	DO_REG(6);
	DO_REG(7);
}

int main()
{
	int rc;

	struct sock_filter filter[] = {
		BPF_STMT(BPF_LD+BPF_W+BPF_ABS,
			 (offsetof(struct seccomp_data, nr))),
		BPF_JUMP(BPF_JMP+BPF_JEQ+BPF_K, __NR_getgid, 0, 1),
		BPF_STMT(BPF_RET+BPF_K, SECCOMP_RET_TRAP),
		BPF_STMT(BPF_RET+BPF_K, SECCOMP_RET_ALLOW),
	};

	struct sock_fprog prog = {
		.len = (unsigned short)(sizeof(filter)/sizeof(filter[0])),
		.filter = filter,
	};

	struct sigaction sa = {};
	sa.sa_sigaction = handler;
	sa.sa_flags = SA_SIGINFO;
	if (sigaction(SIGSYS, &sa, NULL) != 0)
		err(1, "sigaction");

	if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0))
		err(1, "PR_SET_NO_NEW_PRIVS");
	if (prctl(PR_SET_SECCOMP, 2, &prog))
		err(1, "PR_SET_SECCOMP");

	call_getgid_oabi();

	return 0;
}

^ permalink raw reply	[flat|nested] 16+ messages in thread