From: will.deacon@arm.com (Will Deacon)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] arm: ptrace: fix syscall modification under PTRACE_O_TRACESECCOMP
Date: Fri, 20 Jun 2014 11:22:59 +0100 [thread overview]
Message-ID: <20140620102258.GA26626@arm.com> (raw)
In-Reply-To: <20140618202748.GA9022@www.outflux.net>
Hi Kees,
I'm struggling to see the bug in the current code, so apologies if my
questions aren't helpful.
On Wed, Jun 18, 2014 at 09:27:48PM +0100, Kees Cook wrote:
> An x86 tracer wanting to change the syscall uses PTRACE_SETREGS
> (stored to regs->orig_ax), and an ARM tracer uses PTRACE_SET_SYSCALL
> (stored to current_thread_info()->syscall). When this happens, the
> syscall can change across the call to secure_computing(), since it may
> block on tracer notification, and the tracer can then make changes
> to the process, before we return from secure_computing(). This
> means the code must respect the changed syscall after the
> secure_computing() call in syscall_trace_enter(). The same is true
> for tracehook_report_syscall_entry() which may also block and change
> the syscall.
I don't think I understand what you mean by `the code must respect the
changed syscall'. The current code does indeed issue the new syscall, so are
you more concerned with secure_computing changing ->syscall, then the
debugger can't see the new syscall when it sees the trap from tracehook?
Are these even supposed to inter-operate?
> The x86 code handles this (it expects orig_ax to always be the
> desired syscall). In the ARM case, this means we should not be touching
> current_thread_info()->syscall after its initial assignment. All failures
> should result in a -1 syscall, though.
The only time we explicitly touch ->syscall is when we're aborting the call
(i.e. writing -1), which I think is fine.
Will
WARNING: multiple messages have this Message-ID (diff)
From: Will Deacon <will.deacon@arm.com>
To: Kees Cook <keescook@chromium.org>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"Russell King" <linux@arm.linux.org.uk>,
"Andy Lutomirski" <luto@amacapital.net>,
"Andrew Morton" <akpm@linux-foundation.org>,
"Jonathan Austin" <Jonathan.Austin@arm.com>,
"André Hentschel" <nerv@dawncrow.de>,
"Oleg Nesterov" <oleg@redhat.com>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
"Ricky Zhou" <rickyz@google.com>
Subject: Re: [PATCH] arm: ptrace: fix syscall modification under PTRACE_O_TRACESECCOMP
Date: Fri, 20 Jun 2014 11:22:59 +0100 [thread overview]
Message-ID: <20140620102258.GA26626@arm.com> (raw)
In-Reply-To: <20140618202748.GA9022@www.outflux.net>
Hi Kees,
I'm struggling to see the bug in the current code, so apologies if my
questions aren't helpful.
On Wed, Jun 18, 2014 at 09:27:48PM +0100, Kees Cook wrote:
> An x86 tracer wanting to change the syscall uses PTRACE_SETREGS
> (stored to regs->orig_ax), and an ARM tracer uses PTRACE_SET_SYSCALL
> (stored to current_thread_info()->syscall). When this happens, the
> syscall can change across the call to secure_computing(), since it may
> block on tracer notification, and the tracer can then make changes
> to the process, before we return from secure_computing(). This
> means the code must respect the changed syscall after the
> secure_computing() call in syscall_trace_enter(). The same is true
> for tracehook_report_syscall_entry() which may also block and change
> the syscall.
I don't think I understand what you mean by `the code must respect the
changed syscall'. The current code does indeed issue the new syscall, so are
you more concerned with secure_computing changing ->syscall, then the
debugger can't see the new syscall when it sees the trap from tracehook?
Are these even supposed to inter-operate?
> The x86 code handles this (it expects orig_ax to always be the
> desired syscall). In the ARM case, this means we should not be touching
> current_thread_info()->syscall after its initial assignment. All failures
> should result in a -1 syscall, though.
The only time we explicitly touch ->syscall is when we're aborting the call
(i.e. writing -1), which I think is fine.
Will
next prev parent reply other threads:[~2014-06-20 10:22 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-06-18 20:27 [PATCH] arm: ptrace: fix syscall modification under PTRACE_O_TRACESECCOMP Kees Cook
2014-06-18 20:27 ` Kees Cook
2014-06-18 20:51 ` Andy Lutomirski
2014-06-18 20:51 ` Andy Lutomirski
2014-06-20 10:22 ` Will Deacon [this message]
2014-06-20 10:22 ` Will Deacon
2014-06-20 16:44 ` Kees Cook
2014-06-20 16:44 ` Kees Cook
2014-06-20 17:23 ` Will Deacon
2014-06-20 17:23 ` Will Deacon
2014-06-20 17:36 ` Kees Cook
2014-06-20 17:36 ` Kees Cook
2014-06-20 18:10 ` Kees Cook
2014-06-20 18:10 ` Kees Cook
2014-06-23 8:46 ` Will Deacon
2014-06-23 8:46 ` Will Deacon
2014-06-23 19:46 ` Kees Cook
2014-06-23 19:46 ` Kees Cook
2014-06-24 8:54 ` Will Deacon
2014-06-24 8:54 ` Will Deacon
2014-06-24 9:20 ` AKASHI Takahiro
2014-06-24 9:20 ` AKASHI Takahiro
2014-07-03 7:43 ` AKASHI Takahiro
2014-07-03 7:43 ` AKASHI Takahiro
2014-07-03 10:24 ` Will Deacon
2014-07-03 10:24 ` Will Deacon
2014-07-03 15:39 ` Andy Lutomirski
2014-07-03 15:39 ` Andy Lutomirski
2014-07-03 16:11 ` Will Deacon
2014-07-03 16:11 ` Will Deacon
2014-07-03 16:13 ` Andy Lutomirski
2014-07-03 16:13 ` Andy Lutomirski
2014-07-03 16:32 ` Will Deacon
2014-07-03 16:32 ` Will Deacon
2014-07-04 23:05 ` Andy Lutomirski
2014-07-04 23:05 ` Andy Lutomirski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20140620102258.GA26626@arm.com \
--to=will.deacon@arm.com \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.