2015-08-11 12:55:39

by Luis Henriques

[permalink] [raw]
Subject: [3.16.y-ckt stable] Patch "ARC: make sure instruction_pointer() returns unsigned value" has been added to staging queue

This is a note to let you know that I have just added a patch titled

ARC: make sure instruction_pointer() returns unsigned value

to the linux-3.16.y-queue branch of the 3.16.y-ckt extended stable tree
which can be found at:

http://kernel.ubuntu.com/git/ubuntu/linux.git/log/?h=linux-3.16.y-queue

This patch is scheduled to be released in version 3.16.7-ckt16.

If you, or anyone else, feels it should not be added to this tree, please
reply to this email.

For more information about the 3.16.y-ckt tree, see
https://wiki.ubuntu.com/Kernel/Dev/ExtendedStable

Thanks.
-Luis

------

>From c7bc79956a634da40b323e3cf71650a3c08cecd3 Mon Sep 17 00:00:00 2001
From: Alexey Brodkin <[email protected]>
Date: Mon, 13 Jul 2015 10:25:17 +0300
Subject: ARC: make sure instruction_pointer() returns unsigned value

commit f51e2f1911122879eefefa4c592dea8bf794b39c upstream.

Currently instruction_pointer() returns pt_regs->ret and so return value
is of type "long", which implicitly stands for "signed long".

While that's perfectly fine when dealing with 32-bit values if return
value of instruction_pointer() gets assigned to 64-bit variable sign
extension may happen.

And at least in one real use-case it happens already.
In perf_prepare_sample() return value of perf_instruction_pointer()
(which is an alias to instruction_pointer() in case of ARC) is assigned
to (struct perf_sample_data)->ip (which type is "u64").

And what we see if instuction pointer points to user-space application
that in case of ARC lays below 0x8000_0000 "ip" gets set properly with
leading 32 zeros. But if instruction pointer points to kernel address
space that starts from 0x8000_0000 then "ip" is set with 32 leadig
"f"-s. I.e. id instruction_pointer() returns 0x8100_0000, "ip" will be
assigned with 0xffff_ffff__8100_0000. Which is obviously wrong.

In particular that issuse broke output of perf, because perf was unable
to associate addresses like 0xffff_ffff__8100_0000 with anything from
/proc/kallsyms.

That's what we used to see:
----------->8----------
6.27% ls [unknown] [k] 0xffffffff8046c5cc
2.96% ls libuClibc-0.9.34-git.so [.] memcpy
2.25% ls libuClibc-0.9.34-git.so [.] memset
1.66% ls [unknown] [k] 0xffffffff80666536
1.54% ls libuClibc-0.9.34-git.so [.] 0x000224d6
1.18% ls libuClibc-0.9.34-git.so [.] 0x00022472
----------->8----------

With that change perf output looks much better now:
----------->8----------
8.21% ls [kernel.kallsyms] [k] memset
3.52% ls libuClibc-0.9.34-git.so [.] memcpy
2.11% ls libuClibc-0.9.34-git.so [.] malloc
1.88% ls libuClibc-0.9.34-git.so [.] memset
1.64% ls [kernel.kallsyms] [k] _raw_spin_unlock_irqrestore
1.41% ls [kernel.kallsyms] [k] __d_lookup_rcu
----------->8----------

Signed-off-by: Alexey Brodkin <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Vineet Gupta <[email protected]>
Signed-off-by: Luis Henriques <[email protected]>
---
arch/arc/include/asm/ptrace.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arc/include/asm/ptrace.h b/arch/arc/include/asm/ptrace.h
index 1bfeec2c0558..2a58af7a2e3a 100644
--- a/arch/arc/include/asm/ptrace.h
+++ b/arch/arc/include/asm/ptrace.h
@@ -63,7 +63,7 @@ struct callee_regs {
long r25, r24, r23, r22, r21, r20, r19, r18, r17, r16, r15, r14, r13;
};

-#define instruction_pointer(regs) ((regs)->ret)
+#define instruction_pointer(regs) (unsigned long)((regs)->ret)
#define profile_pc(regs) instruction_pointer(regs)

/* return 1 if user mode or 0 if kernel mode */