Subject: [PATCH -tip ] [BUGFIX/URGENT] perf-probe: Do not add offset to uprobe address

Fix perf-probe not to add offset value to uprobe probe
address when post processing.
tevs[i].point.address is the address of symbol+offset,
but current perf-probe adjusts the point.address by
adding the offset. As a result, the probe address becomes
symbol+offset+offset. This may cause unexpected
code corruption. Urgent fix is needed.

Without this fix
---
# ./perf probe -x ./perf dso__load_vmlinux+4
# ./perf probe -l
probe_perf:dso__load_vmlinux (on 0x000000000006d2b8)
# nm ./perf.orig | grep dso__load_vmlinux\$
000000000046d0a0 T dso__load_vmlinux
---
You can see the given offset is 3 but the actual probed
address is dso__load_vmlinux+8.

With this fix
---
# ./perf probe -x ./perf dso__load_vmlinux+4
# ./perf probe -l
probe_perf:dso__load_vmlinux (on 0x000000000006d2b4)
---
Now the problem is fixed.

Note: This bug is introduced by
commit fb7345bbf7fad9bf72ef63a19c707970b9685812

Signed-off-by: Masami Hiramatsu <[email protected]>
---
tools/perf/util/probe-event.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index a8a9b6c..d8b048c 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -336,8 +336,8 @@ static int add_exec_to_probe_trace_events(struct probe_trace_event *tevs,
return ret;

for (i = 0; i < ntevs && ret >= 0; i++) {
+ /* point.address is the addres of point.symbol + point.offset */
offset = tevs[i].point.address - stext;
- offset += tevs[i].point.offset;
tevs[i].point.offset = 0;
zfree(&tevs[i].point.symbol);
ret = e_snprintf(buf, 32, "0x%lx", offset);


2014-02-06 07:48:08

by Namhyung Kim

[permalink] [raw]
Subject: Re: [PATCH -tip ] [BUGFIX/URGENT] perf-probe: Do not add offset to uprobe address

Hi Masami,

On Wed, 05 Feb 2014 05:18:58 +0000, Masami Hiramatsu wrote:
> Fix perf-probe not to add offset value to uprobe probe
> address when post processing.
> tevs[i].point.address is the address of symbol+offset,
> but current perf-probe adjusts the point.address by
> adding the offset. As a result, the probe address becomes
> symbol+offset+offset. This may cause unexpected
> code corruption. Urgent fix is needed.
>
> Without this fix
> ---
> # ./perf probe -x ./perf dso__load_vmlinux+4
> # ./perf probe -l
> probe_perf:dso__load_vmlinux (on 0x000000000006d2b8)
> # nm ./perf.orig | grep dso__load_vmlinux\$
> 000000000046d0a0 T dso__load_vmlinux

Shouldn't the original symbol address be

000000000046d2b0

?

> ---
> You can see the given offset is 3 but the actual probed

s/3/4/ ?

Thanks,
Namhyung


> address is dso__load_vmlinux+8.
>
> With this fix
> ---
> # ./perf probe -x ./perf dso__load_vmlinux+4
> # ./perf probe -l
> probe_perf:dso__load_vmlinux (on 0x000000000006d2b4)
> ---
> Now the problem is fixed.

Subject: Re: [PATCH -tip ] [BUGFIX/URGENT] perf-probe: Do not add offset to uprobe address

(2014/02/06 16:48), Namhyung Kim wrote:
> Hi Masami,
>
> On Wed, 05 Feb 2014 05:18:58 +0000, Masami Hiramatsu wrote:
>> Fix perf-probe not to add offset value to uprobe probe
>> address when post processing.
>> tevs[i].point.address is the address of symbol+offset,
>> but current perf-probe adjusts the point.address by
>> adding the offset. As a result, the probe address becomes
>> symbol+offset+offset. This may cause unexpected
>> code corruption. Urgent fix is needed.
>>
>> Without this fix
>> ---
>> # ./perf probe -x ./perf dso__load_vmlinux+4
>> # ./perf probe -l
>> probe_perf:dso__load_vmlinux (on 0x000000000006d2b8)
>> # nm ./perf.orig | grep dso__load_vmlinux\$
>> 000000000046d0a0 T dso__load_vmlinux
>
> Shouldn't the original symbol address be
>
> 000000000046d2b0

Oops, Yes, I missed my ./perf and ./perf.orig...
Since the uprobe doesn't track binary change, I usually use a
copy of original binary. Here is the correct test results;


Without this fix
---
# ./perf probe -x ./perf.orig dso__load_vmlinux+4
# ./perf probe -l
probe_perf:dso__load_vmlinux (on 0x000000000006d2b8)
# nm ./perf.orig | grep dso__load_vmlinux\$
000000000046d2b0 T dso__load_vmlinux
---
You can see the given offset is 4 but the actual probed
address is dso__load_vmlinux+8.

With this fix
---
# ./perf probe -x ./perf.orig dso__load_vmlinux+4
# ./perf probe -l
probe_perf:dso__load_vmlinux (on 0x000000000006d2b4)
---
Now the problem is fixed.


>
>> ---
>> You can see the given offset is 3 but the actual probed
>
> s/3/4/ ?

Yes, it's a typo. :(

Thank you!!



--
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]

Subject: [tip:perf/core] perf probe: Do not add offset twice to uprobe address

Commit-ID: 981a23792cd02631f8cd5dd65753208a44de5ae1
Gitweb: http://git.kernel.org/tip/981a23792cd02631f8cd5dd65753208a44de5ae1
Author: Masami Hiramatsu <[email protected]>
AuthorDate: Wed, 5 Feb 2014 05:18:58 +0000
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Mon, 10 Feb 2014 11:34:30 -0300

perf probe: Do not add offset twice to uprobe address

Fix perf-probe not to add offset value twice to uprobe probe address
when post processing.

The tevs[i].point.address struct member is the address of symbol+offset,
but current perf-probe adjusts the point.address by adding the offset.

As a result, the probe address becomes symbol+offset+offset. This may
cause unexpected code corruption. Urgent fix is needed.

Without this fix:
---
# ./perf probe -x ./perf dso__load_vmlinux+4
# ./perf probe -l
probe_perf:dso__load_vmlinux (on 0x000000000006d2b8)
# nm ./perf.orig | grep dso__load_vmlinux\$
000000000046d0a0 T dso__load_vmlinux
---

You can see the given offset is 3 but the actual probed address is
dso__load_vmlinux+8.

With this fix:
---
# ./perf probe -x ./perf dso__load_vmlinux+4
# ./perf probe -l
probe_perf:dso__load_vmlinux (on 0x000000000006d2b4)
---

Now the problem is fixed.

Note: This bug is introduced by
commit fb7345bbf7fad9bf72ef63a19c707970b9685812

Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: "David A. Long" <[email protected]>
Cc: David Ahern <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Oleg Nesterov <[email protected]>
Cc: Srikar Dronamraju <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: [email protected]
Link: http://lkml.kernel.org/r/20140205051858.6519.27314.stgit@kbuild-fedora.yrl.intra.hitachi.co.jp
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/probe-event.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index a8a9b6c..d8b048c 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -336,8 +336,8 @@ static int add_exec_to_probe_trace_events(struct probe_trace_event *tevs,
return ret;

for (i = 0; i < ntevs && ret >= 0; i++) {
+ /* point.address is the addres of point.symbol + point.offset */
offset = tevs[i].point.address - stext;
- offset += tevs[i].point.offset;
tevs[i].point.offset = 0;
zfree(&tevs[i].point.symbol);
ret = e_snprintf(buf, 32, "0x%lx", offset);