From: Mark Rutland <mark.rutland@arm.com>
To: Daniel Thompson <daniel.thompson@linaro.org>
Cc: "Catalin Marinas" <catalin.marinas@arm.com>,
"David Long" <dave.long@linaro.org>,
"Petr Mladek" <pmladek@suse.com>,
"Zi Shen Lim" <zlim.lnx@gmail.com>,
"Will Deacon" <will.deacon@arm.com>,
"Andrey Ryabinin" <ryabinin.a.a@gmail.com>,
"yalin wang" <yalin.wang2010@gmail.com>,
"Li Bin" <huawei.libin@huawei.com>,
"John Blackwood" <john.blackwood@ccur.com>,
"Pratyush Anand" <panand@redhat.com>,
"Huang Shijie" <shijie.huang@arm.com>,
"Dave P Martin" <Dave.Martin@arm.com>,
"Jisheng Zhang" <jszhang@marvell.com>,
"Vladimir Murzin" <Vladimir.Murzin@arm.com>,
"Steve Capper" <steve.capper@linaro.org>,
"Suzuki K Poulose" <suzuki.poulose@arm.com>,
"Marc Zyngier" <marc.zyngier@arm.com>,
"Yang Shi" <yang.shi@linaro.org>,
"Mark Brown" <broonie@kernel.org>,
"Sandeepa Prabhu" <sandeepa.s.prabhu@gmail.com>,
"William Cohen" <wcohen@redhat.com>,
"Alex Bennée" <alex.bennee@linaro.org>,
"Adam Buchbinder" <adam.buchbinder@gmail.com>,
linux-arm-kernel@lists.infradead.org,
"Ard Biesheuvel" <ard.biesheuvel@linaro.org>,
linux-kernel@vger.kernel.org, "James Morse" <james.morse@arm.com>,
"Masami Hiramatsu" <mhiramat@kernel.org>,
"Andrew Morton" <akpm@linux-foundation.org>,
"Robin Murphy" <robin.murphy@arm.com>,
"Jens Wiklander" <jens.wiklander@linaro.org>,
"Christoffer Dall" <christoffer.dall@linaro.org>
Subject: Re: [PATCH v15 04/10] arm64: Kprobes with single stepping support
Date: Tue, 26 Jul 2016 18:54:47 +0100 [thread overview]
Message-ID: <20160726175447.GA28376@leverpostej> (raw)
In-Reply-To: <ce78f577-d14a-7b12-c96f-a6ebd8866283@linaro.org>
On Tue, Jul 26, 2016 at 10:50:08AM +0100, Daniel Thompson wrote:
> On 25/07/16 18:13, Catalin Marinas wrote:
> >You get more unexpected side effects by not saving/restoring the whole
> >stack. We looked into this on Friday and came to the conclusion that
> >there is no safe way for kprobes to know which arguments passed on the
> >stack should be preserved, at least not with the current API.
> >
> >Basically the AArch64 PCS states that for arguments passed on the stack
> >(e.g. they can't fit in registers), the caller allocates memory for them
> >(on its own stack) and passes the pointer to the callee. Unfortunately,
> >the frame pointer seems to be decremented correspondingly to cover the
> >arguments, so we don't really have a way to tell how much to copy.
> >Copying just the caller's stack frame isn't safe either since a
> >callee/caller receiving such argument on the stack may passed it down to
> >a callee without copying (I couldn't find anything in the PCS stating
> >that this isn't allowed).
>
> The PCS[1] seems (at least to me) to be pretty clear that "the
> address of the first stacked argument is defined to be the initial
> value of SP".
>
> I think it is only the return value (when stacked via the x8
> pointer) that can be passed through an intermediate function in the
> way described above. Isn't it OK for a jprobe to clobber this
> memory? The underlying function will overwrite whatever the jprobe
> put there anyway.
>
> Am I overlooking some additional detail in the PCS?
I suspect that the "initial value of SP" is simply meant to be relative to the
base of the region of stack reserved for callee parameters. While it also uses
the phrase "current stack-pointer value", I suspect that this is overly
prescriptive.
In practice, GCC allocates callee parameters *above* the frame record
for the caller, which is above the SP and FP. e.g. with:
----
#define NLARGE 128
struct large {
unsigned long v[NLARGE];
};
unsigned long __attribute__ ((noinline)) large_func(const struct large l)
{
return l.v[0];
}
int main(int argc, char *argv[])
{
struct large l = {
.v = { 1, },
};
return large_func(l);
}
----
Which yields the following assembly:
----
00000000004005d0 <large_func>:
4005d0: f81f0ff3 str x19, [sp,#-16]!
4005d4: aa0003f3 mov x19, x0
4005d8: f9400260 ldr x0, [x19]
4005dc: f84107f3 ldr x19, [sp],#16
4005e0: d65f03c0 ret
00000000004005e4 <main>:
4005e4: d12043ff sub sp, sp, #0x810
4005e8: a9bf7bfd stp x29, x30, [sp,#-16]!
4005ec: 910003fd mov x29, sp
4005f0: b9041fa0 str w0, [x29,#1052]
4005f4: f9020ba1 str x1, [x29,#1040]
4005f8: 911083a0 add x0, x29, #0x420
4005fc: d2808001 mov x1, #0x400 // #1024
400600: aa0103e2 mov x2, x1
400604: 52800001 mov w1, #0x0 // #0
400608: 97ffff92 bl 400450 <memset@plt>
40060c: d2800020 mov x0, #0x1 // #1
400610: f90213a0 str x0, [x29,#1056]
400614: 910043a0 add x0, x29, #0x10
400618: 911083a1 add x1, x29, #0x420
40061c: d2808002 mov x2, #0x400 // #1024
400620: 97ffff84 bl 400430 <memcpy@plt>
400624: 910043a0 add x0, x29, #0x10
400628: 97ffffea bl 4005d0 <large_func>
40062c: a8c17bfd ldp x29, x30, [sp],#16
400630: 912043ff add sp, sp, #0x810
400634: d65f03c0 ret
----
Please ignore the redundant copy GCC generates and copies; I can't seem
to convince it to not do that. The important part is that at 400614 the
argument to the function is the address immediately above the frame
record for main.
In local testing, it seems that additional locals can appear between the
frame record and argument.
Given this, callees can't rely on any relationship between their initial sp and
stacked arguments. Given that, I see no reason why an intermediary could not
simply pass the pointer on while creating further intermediary stack frames.
Thanks,
Mark.
next prev parent reply other threads:[~2016-07-26 17:55 UTC|newest]
Thread overview: 71+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-07-08 16:35 [PATCH v15 00/10] arm64: Add kernel probes (kprobes) support David Long
2016-07-08 16:35 ` [PATCH v15 01/10] arm64: Add HAVE_REGS_AND_STACK_ACCESS_API feature David Long
2016-07-15 10:57 ` Catalin Marinas
2016-07-15 14:51 ` David Long
2016-07-15 15:13 ` Catalin Marinas
2016-07-15 17:51 ` David Long
2016-07-19 14:17 ` Catalin Marinas
2016-07-08 16:35 ` [PATCH v15 02/10] arm64: Add more test functions to insn.c David Long
2016-07-08 16:35 ` [PATCH v15 03/10] arm64: add conditional instruction simulation support David Long
2016-07-08 16:35 ` [PATCH v15 04/10] arm64: Kprobes with single stepping support David Long
2016-07-20 9:36 ` Marc Zyngier
2016-07-20 11:16 ` Catalin Marinas
2016-07-20 19:08 ` David Long
2016-07-21 8:44 ` Marc Zyngier
2016-07-20 15:49 ` Catalin Marinas
2016-07-21 14:50 ` David Long
2016-07-20 16:09 ` Marc Zyngier
2016-07-20 16:28 ` Catalin Marinas
2016-07-20 16:31 ` Marc Zyngier
2016-07-20 16:46 ` Marc Zyngier
2016-07-20 17:04 ` Catalin Marinas
2016-07-21 16:33 ` David Long
2016-07-21 17:16 ` Catalin Marinas
2016-07-21 17:23 ` Marc Zyngier
2016-07-21 18:33 ` David Long
2016-07-22 10:16 ` Catalin Marinas
2016-07-22 15:51 ` David Long
2016-07-25 17:13 ` Catalin Marinas
2016-07-25 22:27 ` David Long
2016-07-27 11:50 ` Daniel Thompson
2016-07-27 22:13 ` David Long
2016-07-28 14:40 ` Catalin Marinas
2016-07-29 9:01 ` Daniel Thompson
2016-08-04 4:47 ` David Long
2016-08-08 11:13 ` Daniel Thompson
2016-08-08 14:29 ` David Long
2016-08-08 22:49 ` Masami Hiramatsu
2016-08-09 17:23 ` Catalin Marinas
2016-08-10 20:41 ` David Long
2016-08-08 22:19 ` Masami Hiramatsu
2016-07-26 9:50 ` Daniel Thompson
2016-07-26 16:55 ` Catalin Marinas
2016-07-27 10:01 ` Dave Martin
2016-07-26 17:54 ` Mark Rutland [this message]
2016-07-27 11:19 ` Daniel Thompson
2016-07-27 11:38 ` Dave Martin
2016-07-27 11:42 ` Daniel Thompson
2016-07-27 13:38 ` Mark Rutland
2016-07-08 16:35 ` [PATCH v15 05/10] arm64: Blacklist non-kprobe-able symbol David Long
2016-07-08 16:35 ` [PATCH v15 06/10] arm64: Treat all entry code as non-kprobe-able David Long
2016-07-15 16:47 ` Catalin Marinas
2016-07-19 0:53 ` David Long
2016-07-08 16:35 ` [PATCH v15 07/10] arm64: kprobes instruction simulation support David Long
2016-07-10 22:51 ` Paul Gortmaker
2016-07-08 16:35 ` [PATCH v15 08/10] arm64: Add trampoline code for kretprobes David Long
2016-07-19 13:46 ` Catalin Marinas
2016-07-20 18:28 ` David Long
2016-07-08 16:35 ` [PATCH v15 09/10] arm64: Add kernel return probes support (kretprobes) David Long
2016-07-08 16:35 ` [PATCH v15 10/10] kprobes: Add arm64 case in kprobe example module David Long
2016-07-14 16:22 ` [PATCH v15 00/10] arm64: Add kernel probes (kprobes) support Catalin Marinas
2016-07-14 17:09 ` William Cohen
2016-07-15 7:50 ` Catalin Marinas
2016-07-15 8:01 ` Marc Zyngier
2016-07-15 8:59 ` Alex Bennée
2016-07-15 9:04 ` Marc Zyngier
2016-07-15 9:53 ` Marc Zyngier
2016-07-14 17:56 ` David Long
2016-07-19 13:57 ` Catalin Marinas
2016-07-19 14:01 ` David Long
2016-07-19 18:27 ` Catalin Marinas
2016-07-19 19:38 ` David Long
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=20160726175447.GA28376@leverpostej \
--to=mark.rutland@arm.com \
--cc=Dave.Martin@arm.com \
--cc=Vladimir.Murzin@arm.com \
--cc=adam.buchbinder@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=alex.bennee@linaro.org \
--cc=ard.biesheuvel@linaro.org \
--cc=broonie@kernel.org \
--cc=catalin.marinas@arm.com \
--cc=christoffer.dall@linaro.org \
--cc=daniel.thompson@linaro.org \
--cc=dave.long@linaro.org \
--cc=huawei.libin@huawei.com \
--cc=james.morse@arm.com \
--cc=jens.wiklander@linaro.org \
--cc=john.blackwood@ccur.com \
--cc=jszhang@marvell.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=marc.zyngier@arm.com \
--cc=mhiramat@kernel.org \
--cc=panand@redhat.com \
--cc=pmladek@suse.com \
--cc=robin.murphy@arm.com \
--cc=ryabinin.a.a@gmail.com \
--cc=sandeepa.s.prabhu@gmail.com \
--cc=shijie.huang@arm.com \
--cc=steve.capper@linaro.org \
--cc=suzuki.poulose@arm.com \
--cc=wcohen@redhat.com \
--cc=will.deacon@arm.com \
--cc=yalin.wang2010@gmail.com \
--cc=yang.shi@linaro.org \
--cc=zlim.lnx@gmail.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 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).