[PATCH v15 04/10] arm64: Kprobes with single stepping support
Catalin Marinas
catalin.marinas at arm.com
Tue Jul 26 09:55:43 PDT 2016
On Tue, Jul 26, 2016 at 10:50:08AM +0100, Daniel Thompson wrote:
> On 25/07/16 18:13, Catalin Marinas wrote:
> >On Fri, Jul 22, 2016 at 11:51:32AM -0400, David Long wrote:
> >>On 07/22/2016 06:16 AM, Catalin Marinas wrote:
> >>>On Thu, Jul 21, 2016 at 02:33:52PM -0400, David Long wrote:
> >>>[...]
> >>>The document states: "Up to MAX_STACK_SIZE bytes are copied". That means
> >>>the arch code could always copy less but never more than MAX_STACK_SIZE.
> >>>What we are proposing is that we should try to guess how much to copy
> >>>based on the FP value (caller's frame) and, if larger than
> >>>MAX_STACK_SIZE, skip the probe hook entirely. I don't think this goes
> >>>against the kprobes.txt document but at least it (a) may improve the
> >>>performance slightly by avoiding unnecessary copy and (b) it avoids
> >>>undefined behaviour if we ever encounter a jprobe with arguments passed
> >>>on the stack beyond MAX_STACK_SIZE.
> >>
> >>OK, it sounds like an improvement. I do worry a little about unexpected side
> >>effects.
> >
> >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'm not sure I fully understand the PCS. I played with some random hacks
to test_kprobes.c (see below) and the address passed for a big struct
didn't look like the bottom of the stack.
diff --git a/kernel/test_kprobes.c b/kernel/test_kprobes.c
index 0dbab6d1acb4..6ed7be02a560 100644
--- a/kernel/test_kprobes.c
+++ b/kernel/test_kprobes.c
@@ -22,14 +22,18 @@
#define div_factor 3
+struct dummy {
+ char dummy_array[MAX_STACK_SIZE * 2];
+};
+
static u32 rand1, preh_val, posth_val, jph_val;
static int errors, handler_errors, num_tests;
-static u32 (*target)(u32 value);
+static u32 (*target)(u32 value, struct dummy d);
static u32 (*target2)(u32 value);
-static noinline u32 kprobe_target(u32 value)
+static noinline u32 kprobe_target(u32 value, struct dummy d)
{
- return (value / div_factor);
+ return (value / div_factor - d.dummy_array[0] + d.dummy_array[1]);
}
static int kp_pre_handler(struct kprobe *p, struct pt_regs *regs)
@@ -54,9 +58,11 @@ static struct kprobe kp = {
.post_handler = kp_post_handler
};
-static int test_kprobe(void)
+static int noinline test_kprobe(void)
{
int ret;
+ static struct dummy dummy;
+ memset(&dummy, 10, sizeof(dummy));
ret = register_kprobe(&kp);
if (ret < 0) {
@@ -64,7 +70,8 @@ static int test_kprobe(void)
return ret;
}
- ret = target(rand1);
+ ret = target(rand1, dummy);
+ memset(&dummy, 10, sizeof(dummy));
unregister_kprobe(&kp);
if (preh_val == 0) {
@@ -111,6 +118,8 @@ static int test_kprobes(void)
{
int ret;
struct kprobe *kps[2] = {&kp, &kp2};
+ struct dummy dummy;
+ memset(&dummy, 10, sizeof(dummy));
/* addr and flags should be cleard for reusing kprobe. */
kp.addr = NULL;
@@ -123,7 +132,7 @@ static int test_kprobes(void)
preh_val = 0;
posth_val = 0;
- ret = target(rand1);
+ ret = target(rand1, dummy);
if (preh_val == 0) {
pr_err("kprobe pre_handler not called\n");
@@ -154,7 +163,7 @@ static int test_kprobes(void)
}
-static u32 j_kprobe_target(u32 value)
+static u32 j_kprobe_target(u32 value, struct dummy d)
{
if (value != rand1) {
handler_errors++;
@@ -174,6 +183,8 @@ static struct jprobe jp = {
static int test_jprobe(void)
{
int ret;
+ struct dummy dummy;
+ memset(&dummy, 10, sizeof(dummy));
ret = register_jprobe(&jp);
if (ret < 0) {
@@ -181,7 +192,7 @@ static int test_jprobe(void)
return ret;
}
- ret = target(rand1);
+ ret = target(rand1, dummy);
unregister_jprobe(&jp);
if (jph_val == 0) {
pr_err("jprobe handler not called\n");
@@ -200,6 +211,8 @@ static int test_jprobes(void)
{
int ret;
struct jprobe *jps[2] = {&jp, &jp2};
+ struct dummy dummy;
+ memset(&dummy, 10, sizeof(dummy));
/* addr and flags should be cleard for reusing kprobe. */
jp.kp.addr = NULL;
@@ -211,7 +224,7 @@ static int test_jprobes(void)
}
jph_val = 0;
- ret = target(rand1);
+ ret = target(rand1, dummy);
if (jph_val == 0) {
pr_err("jprobe handler not called\n");
handler_errors++;
@@ -262,6 +275,8 @@ static struct kretprobe rp = {
static int test_kretprobe(void)
{
int ret;
+ struct dummy dummy;
+ memset(&dummy, 10, sizeof(dummy));
ret = register_kretprobe(&rp);
if (ret < 0) {
@@ -269,7 +284,7 @@ static int test_kretprobe(void)
return ret;
}
- ret = target(rand1);
+ ret = target(rand1, dummy);
unregister_kretprobe(&rp);
if (krph_val != rand1) {
pr_err("kretprobe handler not called\n");
@@ -306,6 +321,8 @@ static int test_kretprobes(void)
{
int ret;
struct kretprobe *rps[2] = {&rp, &rp2};
+ struct dummy dummy;
+ memset(&dummy, 10, sizeof(dummy));
/* addr and flags should be cleard for reusing kprobe. */
rp.kp.addr = NULL;
@@ -317,7 +334,7 @@ static int test_kretprobes(void)
}
krph_val = 0;
- ret = target(rand1);
+ ret = target(rand1, dummy);
if (krph_val != rand1) {
pr_err("kretprobe handler not called\n");
handler_errors++;
--
Catalin
More information about the linux-arm-kernel
mailing list