All of lore.kernel.org
 help / color / mirror / Atom feed
From: will.deacon@arm.com (Will Deacon)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v5 1/3] arm64: ptrace: reload a syscall number after ptrace operations
Date: Wed, 23 Jul 2014 09:25:05 +0100	[thread overview]
Message-ID: <20140723082505.GB27260@arm.com> (raw)
In-Reply-To: <53CF5E53.3060409@linaro.org>

On Wed, Jul 23, 2014 at 08:03:47AM +0100, AKASHI Takahiro wrote:
> On 07/23/2014 05:15 AM, Kees Cook wrote:
> > On Tue, Jul 22, 2014 at 2:14 AM, AKASHI Takahiro
> > <takahiro.akashi@linaro.org> wrote:
> >>   asmlinkage int syscall_trace_enter(struct pt_regs *regs)
> >>   {
> >> +       unsigned long saved_x0, saved_x8;
> >> +
> >> +       saved_x0 = regs->regs[0];
> >> +       saved_x8 = regs->regs[8];
> >> +
> >>          if (test_thread_flag(TIF_SYSCALL_TRACE))
> >>                  tracehook_report_syscall(regs, PTRACE_SYSCALL_ENTER);
> >>
> >> +       regs->syscallno = regs->regs[8];
> >> +       if ((long)regs->syscallno == ~0UL) { /* skip this syscall */
> >> +               regs->regs[8] = saved_x8;
> >> +               if (regs->regs[0] == saved_x0) /* not changed by user */
> >> +                       regs->regs[0] = -ENOSYS;
> >
> > I'm not sure this is right compared to other architectures. Generally
> > when a tracer performs a syscall skip, it's up to them to also adjust
> > the return value. They may want to be faking a syscall, and what if
> > the value they want to return happens to be what x0 was going into the
> > tracer? It would have no way to avoid this -ENOSYS case. I think
> > things are fine without this test.
> 
> Yeah, I know this issue, but was not sure that setting a return value
> is mandatory. (x86 seems to return -ENOSYS by default if not explicitly
> specified.)
> Is "fake a system call" a more appropriate word than "skip"?
> 
> I will defer to Will.

I agree with Kees -- iirc, I only suggested restoring x8.

Will

WARNING: multiple messages have this Message-ID (diff)
From: Will Deacon <will.deacon@arm.com>
To: AKASHI Takahiro <takahiro.akashi@linaro.org>
Cc: Kees Cook <keescook@chromium.org>, Will Drewry <wad@chromium.org>,
	Catalin Marinas <Catalin.Marinas@arm.com>,
	"dsaxena@linaro.org" <dsaxena@linaro.org>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	"linaro-kernel@lists.linaro.org" <linaro-kernel@lists.linaro.org>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v5 1/3] arm64: ptrace: reload a syscall number after ptrace operations
Date: Wed, 23 Jul 2014 09:25:05 +0100	[thread overview]
Message-ID: <20140723082505.GB27260@arm.com> (raw)
In-Reply-To: <53CF5E53.3060409@linaro.org>

On Wed, Jul 23, 2014 at 08:03:47AM +0100, AKASHI Takahiro wrote:
> On 07/23/2014 05:15 AM, Kees Cook wrote:
> > On Tue, Jul 22, 2014 at 2:14 AM, AKASHI Takahiro
> > <takahiro.akashi@linaro.org> wrote:
> >>   asmlinkage int syscall_trace_enter(struct pt_regs *regs)
> >>   {
> >> +       unsigned long saved_x0, saved_x8;
> >> +
> >> +       saved_x0 = regs->regs[0];
> >> +       saved_x8 = regs->regs[8];
> >> +
> >>          if (test_thread_flag(TIF_SYSCALL_TRACE))
> >>                  tracehook_report_syscall(regs, PTRACE_SYSCALL_ENTER);
> >>
> >> +       regs->syscallno = regs->regs[8];
> >> +       if ((long)regs->syscallno == ~0UL) { /* skip this syscall */
> >> +               regs->regs[8] = saved_x8;
> >> +               if (regs->regs[0] == saved_x0) /* not changed by user */
> >> +                       regs->regs[0] = -ENOSYS;
> >
> > I'm not sure this is right compared to other architectures. Generally
> > when a tracer performs a syscall skip, it's up to them to also adjust
> > the return value. They may want to be faking a syscall, and what if
> > the value they want to return happens to be what x0 was going into the
> > tracer? It would have no way to avoid this -ENOSYS case. I think
> > things are fine without this test.
> 
> Yeah, I know this issue, but was not sure that setting a return value
> is mandatory. (x86 seems to return -ENOSYS by default if not explicitly
> specified.)
> Is "fake a system call" a more appropriate word than "skip"?
> 
> I will defer to Will.

I agree with Kees -- iirc, I only suggested restoring x8.

Will

  reply	other threads:[~2014-07-23  8:25 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 [this message]
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
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=20140723082505.GB27260@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.