linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
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

  parent reply	other threads:[~2014-06-20 10:22 UTC|newest]

Thread overview: 18+ 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:51 ` Andy Lutomirski
2014-06-20 10:22 ` Will Deacon [this message]
2014-06-20 16:44   ` Kees Cook
2014-06-20 17:23     ` Will Deacon
2014-06-20 17:36       ` Kees Cook
2014-06-20 18:10         ` Kees Cook
2014-06-23  8:46           ` Will Deacon
2014-06-23 19:46             ` Kees Cook
2014-06-24  8:54               ` Will Deacon
2014-06-24  9:20                 ` AKASHI Takahiro
2014-07-03  7:43                 ` AKASHI Takahiro
2014-07-03 10:24                   ` Will Deacon
2014-07-03 15:39                     ` Andy Lutomirski
2014-07-03 16:11                       ` Will Deacon
2014-07-03 16:13                         ` Andy Lutomirski
2014-07-03 16:32                           ` Will Deacon
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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).