From: will.deacon@arm.com (Will Deacon)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v5 3/3] arm64: Add seccomp support
Date: Fri, 15 Aug 2014 15:33:46 +0100 [thread overview]
Message-ID: <20140815143346.GV27466@arm.com> (raw)
In-Reply-To: <53E9F7E1.9090405@linaro.org>
On Tue, Aug 12, 2014 at 12:17:53PM +0100, AKASHI Takahiro wrote:
> On 08/12/2014 06:40 PM, Will Deacon wrote:
> > On Tue, Aug 12, 2014 at 07:57:25AM +0100, AKASHI Takahiro wrote:
> >>
> >> > case SECCOMP_MODE_FILTER:
> >> > case SECCOMP_RET_TRACE:
> >> > ...
> >> > if (syscall_get_nr(current, regs) < 0)
> >> > goto skip;
> >>
> >> This implies that we should modify syscallno *before* __secure_computing()
> >> returns.
> >
> > Why does it imply that? There are four competing entities here:
> >
> > - seccomp
> > - tracehook
> > - ftrace (trace_sys_*)
> > - audit
> >
> > With the exception of ftrace, they can all potentially rewrite the pt_regs
> > (the code you cite above is just below a ptrace_event call), so we have
> > to choose some order in which to call them.
>
> (audit won't change registers.)
Sorry, you're quite right.
> > On entry, x86 and arm call them in the order I listed above, so it seems
> > sensible to follow that.
>
> Right, but as far as I understand, ptrace_event() in __secure_computing()
> calls ptrace_notify(), and eventually executes ptrace_stop(), which can
> be stopped while tracer runs (until ptrace(PTRACE_CONT)?).
> So syscall_get_nr() is expected to return -1 if trace changes a syscall number to -1
> (as far as sycall_get_nr() refers to syscallno in pt_regs).
>
> That is why I think we should have PTRACE_SET_SYSCALL.
Gotcha, yeah that looks like the cleanest approach after all. Thanks for the
explanation.
Will
WARNING: multiple messages have this Message-ID (diff)
From: Will Deacon <will.deacon@arm.com>
To: AKASHI Takahiro <takahiro.akashi@linaro.org>
Cc: "linaro-kernel@lists.linaro.org" <linaro-kernel@lists.linaro.org>,
Will Drewry <wad@chromium.org>, Kees Cook <keescook@chromium.org>,
Catalin Marinas <Catalin.Marinas@arm.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
Andy Lutomirski <luto@amacapital.net>,
Deepak Saxena <dsaxena@linaro.org>,
Lee Campbell <leecam@google.com>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH v5 3/3] arm64: Add seccomp support
Date: Fri, 15 Aug 2014 15:33:46 +0100 [thread overview]
Message-ID: <20140815143346.GV27466@arm.com> (raw)
In-Reply-To: <53E9F7E1.9090405@linaro.org>
On Tue, Aug 12, 2014 at 12:17:53PM +0100, AKASHI Takahiro wrote:
> On 08/12/2014 06:40 PM, Will Deacon wrote:
> > On Tue, Aug 12, 2014 at 07:57:25AM +0100, AKASHI Takahiro wrote:
> >>
> >> > case SECCOMP_MODE_FILTER:
> >> > case SECCOMP_RET_TRACE:
> >> > ...
> >> > if (syscall_get_nr(current, regs) < 0)
> >> > goto skip;
> >>
> >> This implies that we should modify syscallno *before* __secure_computing()
> >> returns.
> >
> > Why does it imply that? There are four competing entities here:
> >
> > - seccomp
> > - tracehook
> > - ftrace (trace_sys_*)
> > - audit
> >
> > With the exception of ftrace, they can all potentially rewrite the pt_regs
> > (the code you cite above is just below a ptrace_event call), so we have
> > to choose some order in which to call them.
>
> (audit won't change registers.)
Sorry, you're quite right.
> > On entry, x86 and arm call them in the order I listed above, so it seems
> > sensible to follow that.
>
> Right, but as far as I understand, ptrace_event() in __secure_computing()
> calls ptrace_notify(), and eventually executes ptrace_stop(), which can
> be stopped while tracer runs (until ptrace(PTRACE_CONT)?).
> So syscall_get_nr() is expected to return -1 if trace changes a syscall number to -1
> (as far as sycall_get_nr() refers to syscallno in pt_regs).
>
> That is why I think we should have PTRACE_SET_SYSCALL.
Gotcha, yeah that looks like the cleanest approach after all. Thanks for the
explanation.
Will
next prev parent reply other threads:[~2014-08-15 14:33 UTC|newest]
Thread overview: 72+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-07-22 9:14 [PATCH v5 0/3] arm64: Add seccomp support AKASHI Takahiro
2014-07-22 9:14 ` AKASHI Takahiro
2014-07-22 9:14 ` [PATCH v5 1/3] arm64: ptrace: reload a syscall number after ptrace operations AKASHI Takahiro
2014-07-22 9:14 ` AKASHI Takahiro
2014-07-22 20:15 ` Kees Cook
2014-07-22 20:15 ` Kees Cook
2014-07-23 7:03 ` AKASHI Takahiro
2014-07-23 7:03 ` AKASHI Takahiro
2014-07-23 8:25 ` Will Deacon
2014-07-23 8:25 ` Will Deacon
2014-07-23 9:09 ` AKASHI Takahiro
2014-07-23 9:09 ` AKASHI Takahiro
2014-07-23 15:13 ` Kees Cook
2014-07-23 15:13 ` Kees Cook
2014-07-24 3:54 ` Andy Lutomirski
2014-07-24 3:54 ` Andy Lutomirski
2014-07-24 5:57 ` AKASHI Takahiro
2014-07-24 5:57 ` AKASHI Takahiro
2014-07-24 15:01 ` Andy Lutomirski
2014-07-24 15:01 ` Andy Lutomirski
2014-07-25 10:36 ` AKASHI Takahiro
2014-07-25 10:36 ` AKASHI Takahiro
2014-07-25 11:03 ` Will Deacon
2014-07-25 11:03 ` Will Deacon
2014-07-29 6:49 ` AKASHI Takahiro
2014-07-29 6:49 ` AKASHI Takahiro
2014-07-29 13:26 ` Will Deacon
2014-07-29 13:26 ` Will Deacon
2014-07-22 9:14 ` [PATCH v5 2/3] asm-generic: Add generic seccomp.h for secure computing mode 1 AKASHI Takahiro
2014-07-22 9:14 ` AKASHI Takahiro
2014-07-24 3:40 ` Andy Lutomirski
2014-07-24 3:40 ` Andy Lutomirski
2014-07-24 4:41 ` Kees Cook
2014-07-24 4:41 ` Kees Cook
2014-07-24 5:17 ` AKASHI Takahiro
2014-07-24 5:17 ` AKASHI Takahiro
2014-07-24 14:57 ` Andy Lutomirski
2014-07-24 14:57 ` Andy Lutomirski
2014-07-25 8:52 ` AKASHI Takahiro
2014-07-25 8:52 ` AKASHI Takahiro
2014-07-22 9:14 ` [PATCH v5 3/3] arm64: Add seccomp support AKASHI Takahiro
2014-07-22 9:14 ` AKASHI Takahiro
2014-07-24 3:52 ` Andy Lutomirski
2014-07-24 3:52 ` Andy Lutomirski
2014-07-24 5:40 ` AKASHI Takahiro
2014-07-24 5:40 ` AKASHI Takahiro
2014-07-24 15:00 ` Andy Lutomirski
2014-07-24 15:00 ` Andy Lutomirski
2014-07-24 15:16 ` Catalin Marinas
2014-07-24 15:16 ` Catalin Marinas
2014-07-25 9:37 ` AKASHI Takahiro
2014-07-25 9:37 ` AKASHI Takahiro
2014-08-05 15:08 ` Kees Cook
2014-08-05 15:08 ` Kees Cook
2014-08-08 7:35 ` AKASHI Takahiro
2014-08-08 7:35 ` AKASHI Takahiro
2014-08-11 9:24 ` Will Deacon
2014-08-11 9:24 ` Will Deacon
2014-08-12 6:57 ` AKASHI Takahiro
2014-08-12 6:57 ` AKASHI Takahiro
2014-08-12 9:40 ` Will Deacon
2014-08-12 9:40 ` Will Deacon
2014-08-12 11:17 ` AKASHI Takahiro
2014-08-12 11:17 ` AKASHI Takahiro
2014-08-15 14:33 ` Will Deacon [this message]
2014-08-15 14:33 ` Will Deacon
2014-07-22 20:16 ` [PATCH v5 0/3] " Kees Cook
2014-07-22 20:16 ` Kees Cook
2014-07-23 7:09 ` AKASHI Takahiro
2014-07-23 7:09 ` AKASHI Takahiro
2014-07-23 15:36 ` Kees Cook
2014-07-23 15:36 ` Kees Cook
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=20140815143346.GV27466@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.