All of lore.kernel.org
 help / color / mirror / Atom feed
From: Will Deacon <will@kernel.org>
To: Keno Fischer <keno@juliacomputing.com>
Cc: linux-arm-kernel@lists.infradead.org,
	Catalin Marinas <catalin.marinas@arm.com>,
	Oleg Nesterov <oleg@redhat.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Kyle Huey <khuey@pernos.co>
Subject: Re: arm64: Register modification during syscall entry/exit stop
Date: Wed, 20 May 2020 18:41:50 +0100	[thread overview]
Message-ID: <20200520174149.GB27629@willie-the-truck> (raw)
In-Reply-To: <CABV8kRzYzBrdzC1_opmmdpW63N2htfOsAUZ+RjiSDsy=SJW6Yg@mail.gmail.com>

Hi Keno,

On Tue, May 19, 2020 at 04:37:34AM -0400, Keno Fischer wrote:
> > Yes, we inherited this from ARM and I think strace relies on it. In
> > hindsight, it is a little odd, although x7 is a parameter register in the
> > PCS and so it won't be live on entry to a system call.
> 
> I'm not familiar with the PCS acronym, but I assume you mean the
> calling convention? You have more faith in userspace than I do ;). For
> example, cursory googling brought up this arm64 syscall definition in musl:
> 
> https://github.com/bminor/musl/blob/593caa456309714402ca4cb77c3770f4c24da9da/arch/aarch64/syscall_arch.h

Hmm, does that actually result in the SVC instruction getting inlined? I
think that's quite dangerous, since we document that we can trash the SVE
register state on a system call, for example. I'm also surprised that
the register variables are honoured by compilers if that inlining can occur.

> The constraints on those asm blocks allow the compiler to assume that
> x7 is preserved across the syscall. If a ptracer accidentally modified it
> (which is easy to do in the situations that I mentioned), it could
> absolutely cause incorrect execution of the userspace program.
> 
> > Although the examples you've
> > listed above are interesting, I don't see why x7 is important in any of
> > them (and we only support up to 6 system call arguments).
> 
> It's not so much that x7 is important, it's that lying to the ptracer is
> problematic, because it might remember that lie and act on it later.
> I did run into exactly this problem, where my ptracer accidentally
> changed the value of x7 and caused incorrect execution in the tracee
> (now that incorrect execution happened to be an assertion, because
> my application is paranoid about these kinds of issues, but it was
> incorrect nevertheless)
> 
> If it would be helpful, I can code up the syscall entry -> signal trap example
> ptracer to have a concrete example.

I guess I'm more interested in situations where the compiler thinks x7 is
live, yet we clobber it.

Will

WARNING: multiple messages have this Message-ID (diff)
From: Will Deacon <will@kernel.org>
To: Keno Fischer <keno@juliacomputing.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
	Kyle Huey <khuey@pernos.co>, Oleg Nesterov <oleg@redhat.com>,
	linux-arm-kernel@lists.infradead.org,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: arm64: Register modification during syscall entry/exit stop
Date: Wed, 20 May 2020 18:41:50 +0100	[thread overview]
Message-ID: <20200520174149.GB27629@willie-the-truck> (raw)
In-Reply-To: <CABV8kRzYzBrdzC1_opmmdpW63N2htfOsAUZ+RjiSDsy=SJW6Yg@mail.gmail.com>

Hi Keno,

On Tue, May 19, 2020 at 04:37:34AM -0400, Keno Fischer wrote:
> > Yes, we inherited this from ARM and I think strace relies on it. In
> > hindsight, it is a little odd, although x7 is a parameter register in the
> > PCS and so it won't be live on entry to a system call.
> 
> I'm not familiar with the PCS acronym, but I assume you mean the
> calling convention? You have more faith in userspace than I do ;). For
> example, cursory googling brought up this arm64 syscall definition in musl:
> 
> https://github.com/bminor/musl/blob/593caa456309714402ca4cb77c3770f4c24da9da/arch/aarch64/syscall_arch.h

Hmm, does that actually result in the SVC instruction getting inlined? I
think that's quite dangerous, since we document that we can trash the SVE
register state on a system call, for example. I'm also surprised that
the register variables are honoured by compilers if that inlining can occur.

> The constraints on those asm blocks allow the compiler to assume that
> x7 is preserved across the syscall. If a ptracer accidentally modified it
> (which is easy to do in the situations that I mentioned), it could
> absolutely cause incorrect execution of the userspace program.
> 
> > Although the examples you've
> > listed above are interesting, I don't see why x7 is important in any of
> > them (and we only support up to 6 system call arguments).
> 
> It's not so much that x7 is important, it's that lying to the ptracer is
> problematic, because it might remember that lie and act on it later.
> I did run into exactly this problem, where my ptracer accidentally
> changed the value of x7 and caused incorrect execution in the tracee
> (now that incorrect execution happened to be an assertion, because
> my application is paranoid about these kinds of issues, but it was
> incorrect nevertheless)
> 
> If it would be helpful, I can code up the syscall entry -> signal trap example
> ptracer to have a concrete example.

I guess I'm more interested in situations where the compiler thinks x7 is
live, yet we clobber it.

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2020-05-20 17:41 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-19  1:05 arm64: Register modification during syscall entry/exit stop Keno Fischer
2020-05-19  1:05 ` Keno Fischer
2020-05-19  8:15 ` Will Deacon
2020-05-19  8:15   ` Will Deacon
2020-05-19  8:37   ` Keno Fischer
2020-05-19  8:37     ` Keno Fischer
2020-05-20 17:41     ` Will Deacon [this message]
2020-05-20 17:41       ` Will Deacon
2020-05-23  5:35       ` Keno Fischer
2020-05-23  5:35         ` Keno Fischer
2020-05-24  6:56         ` Keno Fischer
2020-05-24  6:56           ` Keno Fischer
2020-05-27  9:55           ` Will Deacon
2020-05-27  9:55             ` Will Deacon
2020-05-27 10:19             ` Dave Martin
2020-05-27 10:19               ` Dave Martin
2020-05-31  9:33               ` Will Deacon
2020-05-31  9:33                 ` Will Deacon
2020-05-31 16:13                 ` Keno Fischer
2020-05-31 16:13                   ` Keno Fischer
2020-06-01  9:14                   ` Dave Martin
2020-06-01  9:14                     ` Dave Martin
2020-06-01  9:23                     ` Keno Fischer
2020-06-01  9:23                       ` Keno Fischer
2020-06-01  9:52                       ` Dave Martin
2020-06-01  9:52                         ` Dave Martin
2020-05-31 16:20               ` Keno Fischer
2020-05-31 16:20                 ` Keno Fischer
2020-06-01  9:23                 ` Dave Martin
2020-06-01  9:23                   ` Dave Martin
2020-06-01  9:40                   ` Keno Fischer
2020-06-01  9:40                     ` Keno Fischer
2020-06-01  9:59                     ` Dave Martin
2020-06-01  9:59                       ` Dave Martin

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=20200520174149.GB27629@willie-the-truck \
    --to=will@kernel.org \
    --cc=catalin.marinas@arm.com \
    --cc=keno@juliacomputing.com \
    --cc=khuey@pernos.co \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=oleg@redhat.com \
    /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.