[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20120522173942.GJ11775@ZenIV.linux.org.uk>
Date: Tue, 22 May 2012 18:39:43 +0100
From: Al Viro <viro@...IV.linux.org.uk>
To: Will Drewry <wad@...omium.org>
Cc: Indan Zupancic <indan@....nu>, Roland McGrath <mcgrathr@...gle.com>,
Eric Paris <netdev@...isplace.org>, linux-kernel@...r.kernel.org,
linux-security-module@...r.kernel.org,
kernel-hardening@...ts.openwall.com, hpa@...or.com,
mingo@...hat.com, oleg@...hat.com, peterz@...radead.org,
rdunlap@...otime.net, tglx@...utronix.de, luto@....edu,
eparis@...hat.com, serge.hallyn@...onical.com, pmoore@...hat.com,
akpm@...ux-foundation.org, corbet@....net, eric.dumazet@...il.com,
markus@...omium.org, coreyb@...ux.vnet.ibm.com,
keescook@...omium.org
Subject: Re: seccomp and ptrace. what is the correct order?
On Tue, May 22, 2012 at 11:23:06AM -0500, Will Drewry wrote:
> However(!), if we did move secure_computing() to after ptrace _and_
> added a check after SECCOMP_RET_TRACE's ptrace_event call, we could
> ensure the system call was not changed by the tracer. This would give
> strong assurances that whatever system call is executed was explicitly
> allowed by seccomp policy is the one that was executed.
BTW, after grepping around a bit, I have to say that some callers of those
hooks make very little sense
Exhibit A: sh32 has in do_syscall_trace_enter(regs)
secure_computing(regs->regs[0]);
Syscall number in r0, right?
[usual PTRACE_SYSCALL bits]
if (unlikely(test_thread_flag(TIF_SYSCALL_TRACEPOINT)))
trace_sys_enter(regs, regs->regs[0]);
Ditto
audit_syscall_entry(audit_arch(), regs->regs[3],
regs->regs[4], regs->regs[5],
regs->regs[6], regs->regs[7]);
Oops - that one says syscall number in r3, first 4 arguments in r4..r7
return ret ?: regs->regs[0];
and the caller of that sucker is
syscall_trace_entry:
! Yes it is traced.
mov r15, r4
mov.l 7f, r11 ! Call do_syscall_trace_enter which notifies
jsr @r11 ! superior (will chomp R[0-7])
nop
mov.l r0, @(OFF_R0,r15) ! Save return value
! Reload R0-R4 from kernel stack, where the
! parent may have modified them using
! ptrace(POKEUSR). (Note that R0-R2 are
! used by the system call handler directly
! from the kernel stack anyway, so don't need
! to be reloaded here.) This allows the parent
! to rewrite system calls and args on the fly.
mov.l @(OFF_R4,r15), r4 ! arg0
mov.l @(OFF_R5,r15), r5
mov.l @(OFF_R6,r15), r6
mov.l @(OFF_R7,r15), r7 ! arg3
mov.l @(OFF_R3,r15), r3 ! syscall_nr
!
mov.l 2f, r10 ! Number of syscalls
cmp/hs r10, r3
bf syscall_call
[...]
7: .long do_syscall_trace_enter
... and syscall_call very clearly picks an index in syscall table from r3.
Incidentally, r0 is the fifth syscall argument... So what we have is
* b0rken hookups for seccomp and tracepoint
* b0rken cancelling of syscalls by ptrace (replacing syscall number
with -1 would've worked; doing that to the 5th argument - not really)
Exhibit B: sh64; makes even less sense, but there the assembler glue had
been too dense for me to get through. At the very least, seccomp and
tracepoint are assuming that syscall number is in r9, while audit is
assuming that it's in r1. I'm not too inclined to trust audit in this
case, though. The _really_ interesting part is that by the look of
their pt_regs syscall number is stored separately - not in regs->regs[]
at all. And the caller
* shoves the return value of do_syscall_trace_enter() into regs->regs[2]
* picks syscall number from regs->syscall_nr and uses that as index
in sys_call_table
* seems to imply that arguments are in regs->regs[2..7]
* code around the (presumable) path leading there seems to imply
that syscall number comes from the trap number and isn't in regs->regs[]
at all. But I might be misreading that assembler. Easily.
Exhibit C:
mips is consistent these days, but it has no tracepoint hookup *and* it does
open-code tracehook_report_syscall_entry(), except for its return value...
Used to pass the wrong register to seccomp, IIRC.
We really ought to look into merging those suckers. It's a source of PITA
that keeps coming back; what we need is
regs_syscall_number(struct pt_regs *)
regs_syscall_arg1(struct pt_regs *)
...
regs_syscall_arg6(struct pt_regs *)
in addition to existing
regs_return_value(struct pt_regs *)
added on all platforms and made mandatory for new ones. With that we
could go a long way towards merging these implementations...
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
