Groups > linux.kernel > #1263187 > unrolled thread

[PATCH 0/2] perf tools: Two bugfixs related to perf probe

Started by	Wang Nan <wangnan0@huawei.com>
First post	2015-11-05 14:20 +0100
Last post	2015-11-12 07:50 +0100
Articles	19 — 7 participants

Back to article view | Back to linux.kernel

  [PATCH 0/2] perf tools: Two bugfixs related to perf probe Wang Nan <wangnan0@huawei.com> - 2015-11-05 14:20 +0100
    [PATCH 2/2] perf tools: Fix find_perf_probe_point_from_map() which incorrectly returns success Wang Nan <wangnan0@huawei.com> - 2015-11-05 14:30 +0100
      RE: [PATCH 2/2] perf tools: Fix find_perf_probe_point_from_map()  which incorrectly returns success 平松雅巳 / HIRAMATU，MASAMI   <masami.hiramatsu.pt@hitachi.com> - 2015-11-05 15:10 +0100
        Re: [PATCH 2/2] perf tools: Fix find_perf_probe_point_from_map()  which incorrectly returns success "acme@kernel.org" <acme@kernel.org> - 2015-11-05 17:10 +0100
          RE: [PATCH 2/2] perf tools: Fix find_perf_probe_point_from_map()  which incorrectly returns success 平松雅巳 / HIRAMATU，MASAMI   <masami.hiramatsu.pt@hitachi.com> - 2015-11-06 07:30 +0100
            RE: [PATCH 2/2] perf tools: Fix find_perf_probe_point_from_map()  which incorrectly returns success 平松雅巳 / HIRAMATU，MASAMI   <masami.hiramatsu.pt@hitachi.com> - 2015-11-06 08:20 +0100
              Re: [PATCH 2/2] perf tools: Fix find_perf_probe_point_from_map()  which incorrectly returns success "Wangnan (F)" <wangnan0@huawei.com> - 2015-11-06 09:40 +0100
                Re: [PATCH 2/2] perf tools: Fix find_perf_probe_point_from_map()  which incorrectly returns success "Wangnan (F)" <wangnan0@huawei.com> - 2015-11-06 10:40 +0100
                  Re: [PATCH 2/2] perf tools: Fix find_perf_probe_point_from_map()  which incorrectly returns success Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-11-06 14:50 +0100
      [tip:perf/urgent] perf tools: Fix find_perf_probe_point_from_map(  ) which incorrectly returns success tip-bot for Wang Nan <tipbot@zytor.com> - 2015-11-08 08:40 +0100
    [PATCH 1/2] perf probe: Only call probe_file__get_events() when fd is valid Wang Nan <wangnan0@huawei.com> - 2015-11-05 14:30 +0100
      RE: [PATCH 1/2] perf probe: Only call probe_file__get_events() when  fd is valid 平松雅巳 / HIRAMATU，MASAMI   <masami.hiramatsu.pt@hitachi.com> - 2015-11-05 15:30 +0100
      Re: [PATCH 1/2] perf probe: Only call probe_file__get_events() when  fd is valid Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-11-05 16:00 +0100
        RE: [PATCH 1/2] perf probe: Only call probe_file__get_events() when  fd is valid 平松雅巳 / HIRAMATU，MASAMI   <masami.hiramatsu.pt@hitachi.com> - 2015-11-05 16:10 +0100
          Re: [PATCH 1/2] perf probe: Only call probe_file__get_events() when  fd is valid 'Arnaldo Carvalho de Melo' <acme@kernel.org> - 2015-11-05 17:00 +0100
            [PATCH v2] perf probe: Verify parameters for two functions Wang Nan <wangnan0@huawei.com> - 2015-11-06 11:00 +0100
              RE: [PATCH v2] perf probe: Verify parameters for two functions 平松雅巳 / HIRAMATU，MASAMI   <masami.hiramatsu.pt@hitachi.com> - 2015-11-06 11:10 +0100
              Re: [PATCH v2] perf probe: Verify parameters for two functions "Wangnan (F)" <wangnan0@huawei.com> - 2015-11-11 08:10 +0100
              [tip:perf/urgent] perf probe: Verify parameters in two functions tip-bot for Wang Nan <tipbot@zytor.com> - 2015-11-12 07:50 +0100

#1263187 — [PATCH 0/2] perf tools: Two bugfixs related to perf probe

From	Wang Nan <wangnan0@huawei.com>
Date	2015-11-05 14:20 +0100
Subject	[PATCH 0/2] perf tools: Two bugfixs related to perf probe
Message-ID	<qrsCm-7bY-9@gated-at.bofh.it>

I found two bugs in perf probe related code. This two patches
fix them.

Wang Nan (2):
  perf probe: Only call probe_file__get_events() when fd is valid
  perf tools: Fix find_perf_probe_point_from_map() which incorrectly
    returns success

 tools/perf/builtin-probe.c    | 12 ++++++++++--
 tools/perf/util/probe-event.c |  3 +++
 2 files changed, 13 insertions(+), 2 deletions(-)

-- 
1.8.3.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1263194 — [PATCH 2/2] perf tools: Fix find_perf_probe_point_from_map() which incorrectly returns success

From	Wang Nan <wangnan0@huawei.com>
Date	2015-11-05 14:30 +0100
Subject	[PATCH 2/2] perf tools: Fix find_perf_probe_point_from_map() which incorrectly returns success
Message-ID	<qrsM1-7f5-5@gated-at.bofh.it>
In reply to	#1263187

It is possible that find_perf_probe_point_from_map() fails to find
symbol but still returns 0 because of an small error when coding:
find_perf_probe_point_from_map() set 'ret' to error code at first,
but also use it to hold return value of
kernel_get_symbol_address_by_name().

This patch resets 'ret' to error even kernel_get_symbol_address_by_name()
success, so if !sym, the whole function returns error correctly.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/probe-event.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index b51a8bf..e659c4f 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -1905,6 +1905,9 @@ static int find_perf_probe_point_from_map(struct probe_trace_point *tp,
 			sym = __find_kernel_function(addr, &map);
 		}
 	}
+
+	/* ret may has be overwritten so reset it */
+	ret = -ENOENT;
 	if (!sym)
 		goto out;
 
-- 
1.8.3.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1263236 — RE: [PATCH 2/2] perf tools: Fix find_perf_probe_point_from_map() which incorrectly returns success

From	平松雅巳 / HIRAMATU，MASAMI <masami.hiramatsu.pt@hitachi.com>
Date	2015-11-05 15:10 +0100
Subject	RE: [PATCH 2/2] perf tools: Fix find_perf_probe_point_from_map() which incorrectly returns success
Message-ID	<qrtoK-7I5-27@gated-at.bofh.it>
In reply to	#1263194

RnJvbTogV2FuZyBOYW4gW21haWx0bzp3YW5nbmFuMEBodWF3ZWkuY29tXQ0KPg0KPkl0IGlzIHBv
c3NpYmxlIHRoYXQgZmluZF9wZXJmX3Byb2JlX3BvaW50X2Zyb21fbWFwKCkgZmFpbHMgdG8gZmlu
ZA0KPnN5bWJvbCBidXQgc3RpbGwgcmV0dXJucyAwIGJlY2F1c2Ugb2YgYW4gc21hbGwgZXJyb3Ig
d2hlbiBjb2Rpbmc6DQo+ZmluZF9wZXJmX3Byb2JlX3BvaW50X2Zyb21fbWFwKCkgc2V0ICdyZXQn
IHRvIGVycm9yIGNvZGUgYXQgZmlyc3QsDQo+YnV0IGFsc28gdXNlIGl0IHRvIGhvbGQgcmV0dXJu
IHZhbHVlIG9mDQo+a2VybmVsX2dldF9zeW1ib2xfYWRkcmVzc19ieV9uYW1lKCkuDQoNCk9LLCBJ
IGRpZG4ndCBleHBlY3QgdGhhdCB0aGVyZSBpcyBhIHN5bWJvbCB3aGljaCBjYW4gYmUgZm91bmQg
YnkgDQprZXJuZWxfZ2V0X3N5bWJvbF9hZGRyZXNzX2J5X25hbWUoKSBidXQgbm90IGJ5IF9fZmlu
ZF9rZXJuZWxfZnVuY3Rpb24oKS4uLg0KV291bGQgeW91IGhhdmUgYW55IGV4YW1wbGUgb2YgdGhl
IGVycm9yPw0KDQo+DQo+VGhpcyBwYXRjaCByZXNldHMgJ3JldCcgdG8gZXJyb3IgZXZlbiBrZXJu
ZWxfZ2V0X3N5bWJvbF9hZGRyZXNzX2J5X25hbWUoKQ0KPnN1Y2Nlc3MsIHNvIGlmICFzeW0sIHRo
ZSB3aG9sZSBmdW5jdGlvbiByZXR1cm5zIGVycm9yIGNvcnJlY3RseS4NCg0KSG1tLCB0aGF0IHNv
dW5kcyB0cmlja3kuIEknZCByYXRoZXIgbGlrZSB0byBhZGQgKnBzeW0gdG8ga2VybmVsX2dldF9z
eW1ib2xfYWRkcmVzc19ieV9uYW1lKCkgDQp0byBzYXZlIHN5bWJvbCBhbmQgZG9uJ3QgdXNlIF9f
ZmluZF9rZXJuZWxfZnVuY3Rpb24oKSBpbnN0ZWFkLg0KDQpUaGFuayB5b3UsDQoNCj4NCj5TaWdu
ZWQtb2ZmLWJ5OiBXYW5nIE5hbiA8d2FuZ25hbjBAaHVhd2VpLmNvbT4NCj5DYzogQXJuYWxkbyBD
YXJ2YWxobyBkZSBNZWxvIDxhY21lQHJlZGhhdC5jb20+DQo+Q2M6IEppcmkgT2xzYSA8am9sc2FA
a2VybmVsLm9yZz4NCj5DYzogTWFzYW1pIEhpcmFtYXRzdSA8bWFzYW1pLmhpcmFtYXRzdS5wdEBo
aXRhY2hpLmNvbT4NCj5DYzogTmFtaHl1bmcgS2ltIDxuYW1oeXVuZ0BrZXJuZWwub3JnPg0KPi0t
LQ0KPiB0b29scy9wZXJmL3V0aWwvcHJvYmUtZXZlbnQuYyB8IDMgKysrDQo+IDEgZmlsZSBjaGFu
Z2VkLCAzIGluc2VydGlvbnMoKykNCj4NCj5kaWZmIC0tZ2l0IGEvdG9vbHMvcGVyZi91dGlsL3By
b2JlLWV2ZW50LmMgYi90b29scy9wZXJmL3V0aWwvcHJvYmUtZXZlbnQuYw0KPmluZGV4IGI1MWE4
YmYuLmU2NTljNGYgMTAwNjQ0DQo+LS0tIGEvdG9vbHMvcGVyZi91dGlsL3Byb2JlLWV2ZW50LmMN
Cj4rKysgYi90b29scy9wZXJmL3V0aWwvcHJvYmUtZXZlbnQuYw0KPkBAIC0xOTA1LDYgKzE5MDUs
OSBAQCBzdGF0aWMgaW50IGZpbmRfcGVyZl9wcm9iZV9wb2ludF9mcm9tX21hcChzdHJ1Y3QgcHJv
YmVfdHJhY2VfcG9pbnQgKnRwLA0KPiAJCQlzeW0gPSBfX2ZpbmRfa2VybmVsX2Z1bmN0aW9uKGFk
ZHIsICZtYXApOw0KPiAJCX0NCj4gCX0NCj4rDQo+KwkvKiByZXQgbWF5IGhhcyBiZSBvdmVyd3Jp
dHRlbiBzbyByZXNldCBpdCAqLw0KPisJcmV0ID0gLUVOT0VOVDsNCj4gCWlmICghc3ltKQ0KPiAJ
CWdvdG8gb3V0Ow0KPg0KPi0tDQo+MS44LjMuNA0KDQo=
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1263357 — Re: [PATCH 2/2] perf tools: Fix find_perf_probe_point_from_map() which incorrectly returns success

From	"acme@kernel.org" <acme@kernel.org>
Date	2015-11-05 17:10 +0100
Subject	Re: [PATCH 2/2] perf tools: Fix find_perf_probe_point_from_map() which incorrectly returns success
Message-ID	<qrvgR-tj-19@gated-at.bofh.it>
In reply to	#1263236

Em Thu, Nov 05, 2015 at 02:08:48PM +0000, 平松雅巳 / HIRAMATU，MASAMI escreveu:
> From: Wang Nan [mailto:wangnan0@huawei.com]
> >
> >It is possible that find_perf_probe_point_from_map() fails to find
> >symbol but still returns 0 because of an small error when coding:
> >find_perf_probe_point_from_map() set 'ret' to error code at first,
> >but also use it to hold return value of
> >kernel_get_symbol_address_by_name().
> 
> OK, I didn't expect that there is a symbol which can be found by 
> kernel_get_symbol_address_by_name() but not by __find_kernel_function()...

> Would you have any example of the error?
> 
> >
> >This patch resets 'ret' to error even kernel_get_symbol_address_by_name()
> >success, so if !sym, the whole function returns error correctly.
> 
> Hmm, that sounds tricky. I'd rather like to add *psym to kernel_get_symbol_address_by_name() 
> to save symbol and don't use __find_kernel_function() instead.

Tricky? I don't think so, suboptimal? possibly, but it fixes an error,
so should be processed quickly, right? I'm applying his patch and then
whatever improvement can be done on top.

- Arnaldo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1263759 — RE: [PATCH 2/2] perf tools: Fix find_perf_probe_point_from_map() which incorrectly returns success

From	平松雅巳 / HIRAMATU，MASAMI <masami.hiramatsu.pt@hitachi.com>
Date	2015-11-06 07:30 +0100
Subject	RE: [PATCH 2/2] perf tools: Fix find_perf_probe_point_from_map() which incorrectly returns success
Message-ID	<qrIH7-S8-1@gated-at.bofh.it>
In reply to	#1263357

RnJvbTogYWNtZUBrZXJuZWwub3JnIFttYWlsdG86YWNtZUBrZXJuZWwub3JnXQ0KPg0KPkVtIFRo
dSwgTm92IDA1LCAyMDE1IGF0IDAyOjA4OjQ4UE0gKzAwMDAsIOW5s+advumbheW3syAvIEhJUkFN
QVRV77yMTUFTQU1JIGVzY3JldmV1Og0KPj4gRnJvbTogV2FuZyBOYW4gW21haWx0bzp3YW5nbmFu
MEBodWF3ZWkuY29tXQ0KPj4gPg0KPj4gPkl0IGlzIHBvc3NpYmxlIHRoYXQgZmluZF9wZXJmX3By
b2JlX3BvaW50X2Zyb21fbWFwKCkgZmFpbHMgdG8gZmluZA0KPj4gPnN5bWJvbCBidXQgc3RpbGwg
cmV0dXJucyAwIGJlY2F1c2Ugb2YgYW4gc21hbGwgZXJyb3Igd2hlbiBjb2Rpbmc6DQo+PiA+Zmlu
ZF9wZXJmX3Byb2JlX3BvaW50X2Zyb21fbWFwKCkgc2V0ICdyZXQnIHRvIGVycm9yIGNvZGUgYXQg
Zmlyc3QsDQo+PiA+YnV0IGFsc28gdXNlIGl0IHRvIGhvbGQgcmV0dXJuIHZhbHVlIG9mDQo+PiA+
a2VybmVsX2dldF9zeW1ib2xfYWRkcmVzc19ieV9uYW1lKCkuDQo+Pg0KPj4gT0ssIEkgZGlkbid0
IGV4cGVjdCB0aGF0IHRoZXJlIGlzIGEgc3ltYm9sIHdoaWNoIGNhbiBiZSBmb3VuZCBieQ0KPj4g
a2VybmVsX2dldF9zeW1ib2xfYWRkcmVzc19ieV9uYW1lKCkgYnV0IG5vdCBieSBfX2ZpbmRfa2Vy
bmVsX2Z1bmN0aW9uKCkuLi4NCj4NCj4+IFdvdWxkIHlvdSBoYXZlIGFueSBleGFtcGxlIG9mIHRo
ZSBlcnJvcj8NCj4+DQo+PiA+DQo+PiA+VGhpcyBwYXRjaCByZXNldHMgJ3JldCcgdG8gZXJyb3Ig
ZXZlbiBrZXJuZWxfZ2V0X3N5bWJvbF9hZGRyZXNzX2J5X25hbWUoKQ0KPj4gPnN1Y2Nlc3MsIHNv
IGlmICFzeW0sIHRoZSB3aG9sZSBmdW5jdGlvbiByZXR1cm5zIGVycm9yIGNvcnJlY3RseS4NCj4+
DQo+PiBIbW0sIHRoYXQgc291bmRzIHRyaWNreS4gSSdkIHJhdGhlciBsaWtlIHRvIGFkZCAqcHN5
bSB0byBrZXJuZWxfZ2V0X3N5bWJvbF9hZGRyZXNzX2J5X25hbWUoKQ0KPj4gdG8gc2F2ZSBzeW1i
b2wgYW5kIGRvbid0IHVzZSBfX2ZpbmRfa2VybmVsX2Z1bmN0aW9uKCkgaW5zdGVhZC4NCj4NCj5U
cmlja3k/IEkgZG9uJ3QgdGhpbmsgc28sIHN1Ym9wdGltYWw/IHBvc3NpYmx5LCBidXQgaXQgZml4
ZXMgYW4gZXJyb3IsDQo+c28gc2hvdWxkIGJlIHByb2Nlc3NlZCBxdWlja2x5LCByaWdodD8gSSdt
IGFwcGx5aW5nIGhpcyBwYXRjaCBhbmQgdGhlbg0KPndoYXRldmVyIGltcHJvdmVtZW50IGNhbiBi
ZSBkb25lIG9uIHRvcC4NCg0KT0ssIHRoZW4gSSdsbCBzZW5kIGFuIGltcHJvdmVtZW50IHBhdGNo
Lg0KDQpUaGFua3MsDQoNCj4NCj4tIEFybmFsZG8NCg==
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1263796 — RE: [PATCH 2/2] perf tools: Fix find_perf_probe_point_from_map() which incorrectly returns success

From	平松雅巳 / HIRAMATU，MASAMI <masami.hiramatsu.pt@hitachi.com>
Date	2015-11-06 08:20 +0100
Subject	RE: [PATCH 2/2] perf tools: Fix find_perf_probe_point_from_map() which incorrectly returns success
Message-ID	<qrJtv-1pc-7@gated-at.bofh.it>
In reply to	#1263759

RnJvbTogYWNtZUBrZXJuZWwub3JnIFttYWlsdG86YWNtZUBrZXJuZWwub3JnXQ0KPj4NCj4+RW0g
VGh1LCBOb3YgMDUsIDIwMTUgYXQgMDI6MDg6NDhQTSArMDAwMCwg5bmz5p2+6ZuF5bezIC8gSElS
QU1BVFXvvIxNQVNBTUkgZXNjcmV2ZXU6DQo+Pj4gRnJvbTogV2FuZyBOYW4gW21haWx0bzp3YW5n
bmFuMEBodWF3ZWkuY29tXQ0KPj4+ID4NCj4+PiA+SXQgaXMgcG9zc2libGUgdGhhdCBmaW5kX3Bl
cmZfcHJvYmVfcG9pbnRfZnJvbV9tYXAoKSBmYWlscyB0byBmaW5kDQo+Pj4gPnN5bWJvbCBidXQg
c3RpbGwgcmV0dXJucyAwIGJlY2F1c2Ugb2YgYW4gc21hbGwgZXJyb3Igd2hlbiBjb2Rpbmc6DQo+
Pj4gPmZpbmRfcGVyZl9wcm9iZV9wb2ludF9mcm9tX21hcCgpIHNldCAncmV0JyB0byBlcnJvciBj
b2RlIGF0IGZpcnN0LA0KPj4+ID5idXQgYWxzbyB1c2UgaXQgdG8gaG9sZCByZXR1cm4gdmFsdWUg
b2YNCj4+PiA+a2VybmVsX2dldF9zeW1ib2xfYWRkcmVzc19ieV9uYW1lKCkuDQo+Pj4NCj4+PiBP
SywgSSBkaWRuJ3QgZXhwZWN0IHRoYXQgdGhlcmUgaXMgYSBzeW1ib2wgd2hpY2ggY2FuIGJlIGZv
dW5kIGJ5DQo+Pj4ga2VybmVsX2dldF9zeW1ib2xfYWRkcmVzc19ieV9uYW1lKCkgYnV0IG5vdCBi
eSBfX2ZpbmRfa2VybmVsX2Z1bmN0aW9uKCkuLi4NCj4+DQo+Pj4gV291bGQgeW91IGhhdmUgYW55
IGV4YW1wbGUgb2YgdGhlIGVycm9yPw0KPj4+DQo+Pj4gPg0KPj4+ID5UaGlzIHBhdGNoIHJlc2V0
cyAncmV0JyB0byBlcnJvciBldmVuIGtlcm5lbF9nZXRfc3ltYm9sX2FkZHJlc3NfYnlfbmFtZSgp
DQo+Pj4gPnN1Y2Nlc3MsIHNvIGlmICFzeW0sIHRoZSB3aG9sZSBmdW5jdGlvbiByZXR1cm5zIGVy
cm9yIGNvcnJlY3RseS4NCj4+Pg0KPj4+IEhtbSwgdGhhdCBzb3VuZHMgdHJpY2t5LiBJJ2QgcmF0
aGVyIGxpa2UgdG8gYWRkICpwc3ltIHRvIGtlcm5lbF9nZXRfc3ltYm9sX2FkZHJlc3NfYnlfbmFt
ZSgpDQo+Pj4gdG8gc2F2ZSBzeW1ib2wgYW5kIGRvbid0IHVzZSBfX2ZpbmRfa2VybmVsX2Z1bmN0
aW9uKCkgaW5zdGVhZC4NCj4+DQo+PlRyaWNreT8gSSBkb24ndCB0aGluayBzbywgc3Vib3B0aW1h
bD8gcG9zc2libHksIGJ1dCBpdCBmaXhlcyBhbiBlcnJvciwNCj4+c28gc2hvdWxkIGJlIHByb2Nl
c3NlZCBxdWlja2x5LCByaWdodD8gSSdtIGFwcGx5aW5nIGhpcyBwYXRjaCBhbmQgdGhlbg0KPj53
aGF0ZXZlciBpbXByb3ZlbWVudCBjYW4gYmUgZG9uZSBvbiB0b3AuDQo+DQo+T0ssIHRoZW4gSSds
bCBzZW5kIGFuIGltcHJvdmVtZW50IHBhdGNoLg0KDQpBaCwgZmluYWxseSBJIGdvdCB3aGF0IGhh
cHBlbmVkLiBJIGd1ZXNzIHRoZSBwcm9ibGVtIG1heSBoYXBwZW4gd2hlbiB3ZSBwdXQNCmEgcHJv
YmUgb24gdGhlIGtlcm5lbCBzb21ld2hlcmUgb3V0c2lkZSBvZiBhbnkgZnVuY3Rpb25zIGFuZCBy
dW4gInBlcmYgcHJvYmUgLWwiLg0KSSB0aGluayBpdCBzaG91bGQgbm90IGJlIGFsbG93ZWQgdG8g
cHV0IHRoZSBwcm9iZSBvdXRzaWRlIGFueSBzeW1ib2wuDQoNClRoZSBiYWNrZ3JvdW5kIGlzIGhl
cmUsIGF0IGZpcnN0ICJwZXJmLXByb2JlIC1hIHNvbWV3aGVyZSIgZGVmaW5lcyBhIHByb2JlIGlu
DQp0aGUga2VybmVsIGJ1dCBpdHMgYWRkcmVzcyBpcyByZWxhdGl2ZSBmcm9tICJfdGV4dCIuICh0
aHVzLCB2ZnNfcmVhZCBiZWNvbWVzICJfdGV4dCsyMzQ4MDgwIg0KIGZvciBleGFtcGxlKS4gU2lu
Y2UgaXQgaXMgbm90IHJlYWRhYmxlIGJ5IGh1bWFuLCBwZXJmIHByb2JlIC1sIHRyaWVzIHRvIGdl
dCBhbiBhcHByb3ByaWF0ZQ0Kc3ltYm9sIGZyb20gdGhlICJfdGV4dCtPRkZTRVQiLg0KRm9yIHRo
ZSBwdXJwb3NlLCB0aGUgZmlyc3Qga2VybmVsX2dldF9zeW1ib2xfYWRkcmVzc19ieV9uYW1lKCkg
aXMgZm9yIHRyYW5zbGF0aW5nIF90ZXh0IHRvDQphbiBhZGRyZXNzLCBhbmQgdGhlIHNlY29uZCAg
X19maW5kX2tlcm5lbF9mdW5jdGlvbigpIGlzIGZvciBmaW5kaW5nIGEgc3ltYm9sIGZyb20gdGhl
DQphZGRyZXNzK09GRlNFVC4NClRoZW4sIGlmIHRoZSBhZGRyZXNzK09GRlNFVCBpcyBvdXQgb2Yg
dGhlIHN5bWJvbCBtYXAsIHRoZSBzZWNvbmQgb25lIGNhbiBmYWlsLg0KVGhpcyBtZWFucyB0aGUg
Zmlyc3Qgc3ltYm9sIGFuZCB0aGUgc2Vjb25kIHN5bWJvbCBpcyBub3Qgc2FtZS4NCg0KU28sIHRo
ZSBkaXJlY3Rpb24gb2YgV2FuZyBzb2x1dGlvbiBpcyBnb29kIDopLiBKdXN0IGEgY2xlYW51cCBp
cyByZXF1aXJlZC4NCg0KVGhhbmsgeW91IQ0KDQo+DQo+VGhhbmtzLA0KPg0KPj4NCj4+LSBBcm5h
bGRvDQo=
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1263835 — Re: [PATCH 2/2] perf tools: Fix find_perf_probe_point_from_map() which incorrectly returns success

From	"Wangnan (F)" <wangnan0@huawei.com>
Date	2015-11-06 09:40 +0100
Subject	Re: [PATCH 2/2] perf tools: Fix find_perf_probe_point_from_map() which incorrectly returns success
Message-ID	<qrKIV-26Z-3@gated-at.bofh.it>
In reply to	#1263796


On 2015/11/6 15:12, 平松雅巳 / HIRAMATU，MASAMI wrote:
> From: acme@kernel.org [mailto:acme@kernel.org]
>>> Em Thu, Nov 05, 2015 at 02:08:48PM +0000, 平松雅巳 / HIRAMATU，MASAMI escreveu:
>>>> From: Wang Nan [mailto:wangnan0@huawei.com]
>>>>> It is possible that find_perf_probe_point_from_map() fails to find
>>>>> symbol but still returns 0 because of an small error when coding:
>>>>> find_perf_probe_point_from_map() set 'ret' to error code at first,
>>>>> but also use it to hold return value of
>>>>> kernel_get_symbol_address_by_name().
>>>> OK, I didn't expect that there is a symbol which can be found by
>>>> kernel_get_symbol_address_by_name() but not by __find_kernel_function()...
>>>> Would you have any example of the error?
>>>>
>>>>> This patch resets 'ret' to error even kernel_get_symbol_address_by_name()
>>>>> success, so if !sym, the whole function returns error correctly.
>>>> Hmm, that sounds tricky. I'd rather like to add *psym to kernel_get_symbol_address_by_name()
>>>> to save symbol and don't use __find_kernel_function() instead.
>>> Tricky? I don't think so, suboptimal? possibly, but it fixes an error,
>>> so should be processed quickly, right? I'm applying his patch and then
>>> whatever improvement can be done on top.
>> OK, then I'll send an improvement patch.
> Ah, finally I got what happened. I guess the problem may happen when we put
> a probe on the kernel somewhere outside of any functions and run "perf probe -l".
> I think it should not be allowed to put the probe outside any symbol.
>
> The background is here, at first "perf-probe -a somewhere" defines a probe in
> the kernel but its address is relative from "_text". (thus, vfs_read becomes "_text+2348080"
>   for example). Since it is not readable by human, perf probe -l tries to get an appropriate
> symbol from the "_text+OFFSET".
> For the purpose, the first kernel_get_symbol_address_by_name() is for translating _text to
> an address, and the second  __find_kernel_function() is for finding a symbol from the
> address+OFFSET.
> Then, if the address+OFFSET is out of the symbol map, the second one can fail.
> This means the first symbol and the second symbol is not same.
>
> So, the direction of Wang solution is good :). Just a cleanup is required.
>
> Thank you!

I also tried to finger out the problem for all day and made some 
progress. It is another
problem. It happeneds when probing an address reside in a module on 
aarch64 system.

On my aarch64 system I use kcore. Different from x86, on aarch64, 
modules address is lower
than normal kernel. For example:

On x86_64:

# readelf -a /proc/kcore

   Type           Offset             VirtAddr           PhysAddr
                  FileSiz            MemSiz              Flags  Align
   ...
   LOAD           0x00007fff81003000 0xffffffff81000000 
0x0000000000000000   <-- kernel
                  0x0000000001026000 0x0000000001026000  RWE    1000
   LOAD           0x00007fffa0003000 0xffffffffa0000000 
0x0000000000000000   <-- module
                  0x000000005f000000 0x000000005f000000  RWE    1000

On aarch64:

   Type           Offset             VirtAddr           PhysAddr
                  FileSiz            MemSiz              Flags  Align
   ...
   LOAD           0x0000000000002000 0xffffffc000000000 
0x0000000000000000    <-- kernel
                  0x000000007fc00000 0x000000007fc00000  RWE    1000
   LOAD           0xfffffffffc002000 0xffffffbffc000000 
0x0000000000000000    <-- module
                  0x0000000004000000 0x0000000004000000  RWE    1000

See? On aarch64, Offset field of module address area is negative.

Which causes a problem in dso__split_kallsyms_for_kcore(): when it 
adjusting symbols
using "pos->start -= curr_map->start - curr_map->pgoff", the relative 
order between
module functions and normal kernel function is changed.

For example:

funca at 0xffffffc00021b428 is a normal kernel function.
funcb at 0xffffffbffc000000 is a function in kernel.

During parsing /proc/kallsyms, address of funca > address of funcb.

However, after the adjusting:

funca becomes:

0xffffffc00021b428 - (0xffffffc000000000 - 0x2000) = 0x21d428

funcb becomes:

0xffffffbffc000000 - (0xffffffbffc000000 - 0xfffffffffc002000) = 
0xfffffffffc002000

address of funca < address of funcb.

Unfortunately, the rbtree is not adjusted in this case.

I hacked symbols__find:

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index b4cc766..8463b0c 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -332,12 +332,14 @@ static struct symbol *symbols__find(struct rb_root 
*symbols, u64 ip)
         while (n) {
                 struct symbol *s = rb_entry(n, struct symbol, rb_node);

-               if (ip < s->start)
+               if ((s64)ip < (s64)s->start)
                         n = n->rb_left;
-               else if (ip >= s->end)
+               else if ((s64)ip >= (s64)s->end)
                         n = n->rb_right;
-               else
+               else {
+                       pr_debug("found %p\n", (void *)ip);
                         return s;
+               }
         }

         return NULL;

and get correct result:

try to find information at 3ffc000000 in kernel_module
Failed to find module kernel_module.
Failed to find the path for kernel_module: [kernel_module]
Failed to find corresponding probes from debuginfo.
found 0xfffffffffc002000

However, what we really need is adjusting rbtree in this case.

Could you please give me some hint for fixing this problem? I'm not 
familiar with
this part of code.

Thank you.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1263869 — Re: [PATCH 2/2] perf tools: Fix find_perf_probe_point_from_map() which incorrectly returns success

From	"Wangnan (F)" <wangnan0@huawei.com>
Date	2015-11-06 10:40 +0100
Subject	Re: [PATCH 2/2] perf tools: Fix find_perf_probe_point_from_map() which incorrectly returns success
Message-ID	<qrLF0-2Hw-21@gated-at.bofh.it>
In reply to	#1263835


On 2015/11/6 16:30, Wangnan (F) wrote:
>
>
> On 2015/11/6 15:12, 平松雅巳 / HIRAMATU，MASAMI wrote:
>> From: acme@kernel.org [mailto:acme@kernel.org]
>>>> Em Thu, Nov 05, 2015 at 02:08:48PM +0000, 平松雅巳 / 
>>>> HIRAMATU，MASAMI escreveu:
>>>>> From: Wang Nan [mailto:wangnan0@huawei.com]

[SNIP]
>> Ah, finally I got what happened. I guess the problem may happen when 
>> we put
>> a probe on the kernel somewhere outside of any functions and run 
>> "perf probe -l".
>> I think it should not be allowed to put the probe outside any symbol.
>>
>> The background is here, at first "perf-probe -a somewhere" defines a 
>> probe in
>> the kernel but its address is relative from "_text". (thus, vfs_read 
>> becomes "_text+2348080"
>>   for example). Since it is not readable by human, perf probe -l 
>> tries to get an appropriate
>> symbol from the "_text+OFFSET".
>> For the purpose, the first kernel_get_symbol_address_by_name() is for 
>> translating _text to
>> an address, and the second  __find_kernel_function() is for finding a 
>> symbol from the
>> address+OFFSET.
>> Then, if the address+OFFSET is out of the symbol map, the second one 
>> can fail.
>> This means the first symbol and the second symbol is not same.
>>
>> So, the direction of Wang solution is good :). Just a cleanup is 
>> required.
>>
>> Thank you!
>
> I also tried to finger out the problem for all day and made some 
> progress. It is another
> problem. It happeneds when probing an address reside in a module on 
> aarch64 system.
>
> On my aarch64 system I use kcore. Different from x86, on aarch64, 
> modules address is lower
> than normal kernel. For example:
>
> On x86_64:
>
> # readelf -a /proc/kcore
>
>   Type           Offset             VirtAddr           PhysAddr
>                  FileSiz            MemSiz              Flags Align
>   ...
>   LOAD           0x00007fff81003000 0xffffffff81000000 
> 0x0000000000000000   <-- kernel
>                  0x0000000001026000 0x0000000001026000  RWE 1000
>   LOAD           0x00007fffa0003000 0xffffffffa0000000 
> 0x0000000000000000   <-- module
>                  0x000000005f000000 0x000000005f000000  RWE 1000
>
> On aarch64:
>
>   Type           Offset             VirtAddr           PhysAddr
>                  FileSiz            MemSiz              Flags Align
>   ...
>   LOAD           0x0000000000002000 0xffffffc000000000 
> 0x0000000000000000    <-- kernel
>                  0x000000007fc00000 0x000000007fc00000  RWE 1000
>   LOAD           0xfffffffffc002000 0xffffffbffc000000 
> 0x0000000000000000    <-- module
>                  0x0000000004000000 0x0000000004000000  RWE 1000
>
> See? On aarch64, Offset field of module address area is negative.
>

One thing should be noticed that, even if normal kernel code and modules 
use different
'struct map', they share a same dso. Please see dso__load_kcore, notice 
how it initialize
parameters (md) before calling file__read_maps().

> Which causes a problem in dso__split_kallsyms_for_kcore(): when it 
> adjusting symbols
> using "pos->start -= curr_map->start - curr_map->pgoff", the relative 
> order between
> module functions and normal kernel function is changed.
>
> For example:
>
> funca at 0xffffffc00021b428 is a normal kernel function.
> funcb at 0xffffffbffc000000 is a function in kernel.
>
> During parsing /proc/kallsyms, address of funca > address of funcb.
>
> However, after the adjusting:
>
> funca becomes:
>
> 0xffffffc00021b428 - (0xffffffc000000000 - 0x2000) = 0x21d428
>
> funcb becomes:
>
> 0xffffffbffc000000 - (0xffffffbffc000000 - 0xfffffffffc002000) = 
> 0xfffffffffc002000
>
> address of funca < address of funcb.
>
> Unfortunately, the rbtree is not adjusted in this case.
>

Even if they are in different maps, they share a same dso here, so a 
same rbtree.

Thank you.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1264029 — Re: [PATCH 2/2] perf tools: Fix find_perf_probe_point_from_map() which incorrectly returns success

From	Arnaldo Carvalho de Melo <acme@kernel.org>
Date	2015-11-06 14:50 +0100
Subject	Re: [PATCH 2/2] perf tools: Fix find_perf_probe_point_from_map() which incorrectly returns success
Message-ID	<qrPyW-59d-19@gated-at.bofh.it>
In reply to	#1263869

Em Fri, Nov 06, 2015 at 05:27:06PM +0800, Wangnan (F) escreveu:
> On 2015/11/6 16:30, Wangnan (F) wrote:
> >On 2015/11/6 15:12, 平松雅巳 / HIRAMATU，MASAMI wrote:
> >>From: acme@kernel.org [mailto:acme@kernel.org]
> >>>>Em Thu, Nov 05, 2015 at 02:08:48PM +0000, 平松雅巳 / HIRAMATU，MASAMI escreveu:
> >>>>>From: Wang Nan [mailto:wangnan0@huawei.com]
 
> [SNIP]
> >>Ah, finally I got what happened. I guess the problem may happen when
> >>we put a probe on the kernel somewhere outside of any functions and
> >>run "perf probe -l".

> >>I think it should not be allowed to put the probe outside any symbol.

> >>The background is here, at first "perf-probe -a somewhere" defines a
> >>probe in the kernel but its address is relative from "_text". (thus,
> >>vfs_read becomes "_text+2348080"

> >>  for example). Since it is not readable by human, perf probe -l tries
> >>to get an appropriate
> >>symbol from the "_text+OFFSET".
> >>For the purpose, the first kernel_get_symbol_address_by_name() is for
> >>translating _text to
> >>an address, and the second  __find_kernel_function() is for finding a
> >>symbol from the
> >>address+OFFSET.

> >>Then, if the address+OFFSET is out of the symbol map, the second one can
> >>fail.

> >>This means the first symbol and the second symbol is not same.

> >>So, the direction of Wang solution is good :). Just a cleanup is
> >>required.

> >I also tried to finger out the problem for all day and made some progress.
> >It is another
> >problem. It happeneds when probing an address reside in a module on
> >aarch64 system.
> >
> >On my aarch64 system I use kcore. Different from x86, on aarch64, modules
> >address is lower
> >than normal kernel. For example:
> >
> >On x86_64:
> >
> ># readelf -a /proc/kcore
> >
> >  Type           Offset             VirtAddr           PhysAddr
> >                 FileSiz            MemSiz              Flags Align
> >  ...
> >  LOAD           0x00007fff81003000 0xffffffff81000000 0x0000000000000000
> ><-- kernel
> >                 0x0000000001026000 0x0000000001026000  RWE 1000
> >  LOAD           0x00007fffa0003000 0xffffffffa0000000 0x0000000000000000
> ><-- module
> >                 0x000000005f000000 0x000000005f000000  RWE 1000
> >
> >On aarch64:
> >
> >  Type           Offset             VirtAddr           PhysAddr
> >                 FileSiz            MemSiz              Flags Align
> >  ...
> >  LOAD           0x0000000000002000 0xffffffc000000000 0x0000000000000000
> ><-- kernel
> >                 0x000000007fc00000 0x000000007fc00000  RWE 1000
> >  LOAD           0xfffffffffc002000 0xffffffbffc000000 0x0000000000000000
> ><-- module
> >                 0x0000000004000000 0x0000000004000000  RWE 1000
> >
> >See? On aarch64, Offset field of module address area is negative.
> >
> 
> One thing should be noticed that, even if normal kernel code and modules use
> different
> 'struct map', they share a same dso. Please see dso__load_kcore, notice how
> it initialize
> parameters (md) before calling file__read_maps().
> 
> >Which causes a problem in dso__split_kallsyms_for_kcore(): when it
> >adjusting symbols
> >using "pos->start -= curr_map->start - curr_map->pgoff", the relative
> >order between
> >module functions and normal kernel function is changed.
> >
> >For example:
> >
> >funca at 0xffffffc00021b428 is a normal kernel function.
> >funcb at 0xffffffbffc000000 is a function in kernel.
> >
> >During parsing /proc/kallsyms, address of funca > address of funcb.

> >However, after the adjusting:

> >funca becomes:

> >0xffffffc00021b428 - (0xffffffc000000000 - 0x2000) = 0x21d428

> >funcb becomes:

> >0xffffffbffc000000 - (0xffffffbffc000000 - 0xfffffffffc002000) =
> >0xfffffffffc002000

> >address of funca < address of funcb.

> >Unfortunately, the rbtree is not adjusted in this case.
 
> Even if they are in different maps, they share a same dso here, so a same
> rbtree.

Yeah, see the answer to the patch you sent, we can't change the symbols
in a DSO, as it may be shared by multiple maps (think about glibc and
prelink, even without prelink) the same applies for kernel modules, that
we represent in the same way, and in at least one case, i.e. split
kallsyms for modules, core kernel, etc we share the same dso by multiple
maps, so any adjustment that needs to be done should be done to the map
members, not to the dso ones.

CCing Adrian, that originally wrote the kcore code, but IIRC there are
other places that touch sym-> (thus dso internal state) instead of
adjusting map members :-\

- Arnaldo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1265023 — [tip:perf/urgent] perf tools: Fix find_perf_probe_point_from_map( ) which incorrectly returns success

From	tip-bot for Wang Nan <tipbot@zytor.com>
Date	2015-11-08 08:40 +0100
Subject	[tip:perf/urgent] perf tools: Fix find_perf_probe_point_from_map( ) which incorrectly returns success
Message-ID	<qssJX-5As-19@gated-at.bofh.it>
In reply to	#1263194

Commit-ID:  98d3b258ede2cdac31a2728543f652964e597e79
Gitweb:     http://git.kernel.org/tip/98d3b258ede2cdac31a2728543f652964e597e79
Author:     Wang Nan <wangnan0@huawei.com>
AuthorDate: Thu, 5 Nov 2015 13:19:25 +0000
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 5 Nov 2015 12:47:52 -0300

perf tools: Fix find_perf_probe_point_from_map() which incorrectly returns success

It is possible that find_perf_probe_point_from_map() fails to find a
symbol but still returns 0 because of an small error when coding:
find_perf_probe_point_from_map() set 'ret' to error code at first, but
also use it to hold return value of kernel_get_symbol_address_by_name().

This patch resets 'ret' to error even kernel_get_symbol_address_by_name()
success, so if !sym, the whole function returns error correctly.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Zefan Li <lizefan@huawei.com>
Cc: pi3orama@163.com
Link: http://lkml.kernel.org/r/1446729565-27592-3-git-send-email-wangnan0@huawei.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/probe-event.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index b51a8bf..e659c4f 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -1905,6 +1905,9 @@ static int find_perf_probe_point_from_map(struct probe_trace_point *tp,
 			sym = __find_kernel_function(addr, &map);
 		}
 	}
+
+	/* ret may has be overwritten so reset it */
+	ret = -ENOENT;
 	if (!sym)
 		goto out;
 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1263201 — [PATCH 1/2] perf probe: Only call probe_file__get_events() when fd is valid

From	Wang Nan <wangnan0@huawei.com>
Date	2015-11-05 14:30 +0100
Subject	[PATCH 1/2] perf probe: Only call probe_file__get_events() when fd is valid
Message-ID	<qrsM2-7f5-27@gated-at.bofh.it>
In reply to	#1263187

In system with kprobe enabled but uprobe turned off, 'perf probe -d'
causes segfault because it calls probe_file__get_events() with a
negative fd (when deleting uprobe events).

This patch validates fds before calling probe_file__get_events().

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-probe.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index 132afc9..861aa89 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -384,7 +384,11 @@ static int perf_del_probe_events(struct strfilter *filter)
 		goto out;
 	}
 
-	ret = probe_file__get_events(kfd, filter, klist);
+	if (kfd < 0)
+		ret = -ENOENT;
+	else
+		ret = probe_file__get_events(kfd, filter, klist);
+
 	if (ret == 0) {
 		strlist__for_each(ent, klist)
 			pr_info("Removed event: %s\n", ent->s);
@@ -394,7 +398,11 @@ static int perf_del_probe_events(struct strfilter *filter)
 			goto error;
 	}
 
-	ret2 = probe_file__get_events(ufd, filter, ulist);
+	if (ufd < 0)
+		ret2 = -ENOENT;
+	else
+		ret2 = probe_file__get_events(ufd, filter, ulist);
+
 	if (ret2 == 0) {
 		strlist__for_each(ent, ulist)
 			pr_info("Removed event: %s\n", ent->s);
-- 
1.8.3.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1263245 — RE: [PATCH 1/2] perf probe: Only call probe_file__get_events() when fd is valid

From	平松雅巳 / HIRAMATU，MASAMI <masami.hiramatsu.pt@hitachi.com>
Date	2015-11-05 15:30 +0100
Subject	RE: [PATCH 1/2] perf probe: Only call probe_file__get_events() when fd is valid
Message-ID	<qrtI6-7PQ-19@gated-at.bofh.it>
In reply to	#1263201

RnJvbTogV2FuZyBOYW4gW21haWx0bzp3YW5nbmFuMEBodWF3ZWkuY29tXQ0KPg0KPkluIHN5c3Rl
bSB3aXRoIGtwcm9iZSBlbmFibGVkIGJ1dCB1cHJvYmUgdHVybmVkIG9mZiwgJ3BlcmYgcHJvYmUg
LWQnDQo+Y2F1c2VzIHNlZ2ZhdWx0IGJlY2F1c2UgaXQgY2FsbHMgcHJvYmVfZmlsZV9fZ2V0X2V2
ZW50cygpIHdpdGggYQ0KPm5lZ2F0aXZlIGZkICh3aGVuIGRlbGV0aW5nIHVwcm9iZSBldmVudHMp
Lg0KDQpIbW0sIE9LLiBUaGlzIG1heSBoYXBwZW4gaWYgdXNlciBydW5zIHBlcmYgcHJvYmUgb24g
dGhlIGtlcm5lbA0Kd2hpY2ggb25seSBlbmFibGVzIGVpdGhlciBDT05GSUdfS1BST0JFX0VWRU5U
UyBvciBDT05GSUdfVVBST0JFX0VWRU5UUy4NCg0KPg0KPlRoaXMgcGF0Y2ggdmFsaWRhdGVzIGZk
cyBiZWZvcmUgY2FsbGluZyBwcm9iZV9maWxlX19nZXRfZXZlbnRzKCkuDQoNCkhtbSwgY291bGQg
eW91IGltcHJvdmUgcHJvYmVfZmlsZV9fZ2V0X2V2ZW50cygpIHRvIGNoZWNrIHRoZSBmZCBpbnN0
ZWFkDQpvZiBjaGVja2luZyBpdCBhdCBjYWxsLXNpdGU/IEkgdGhpbmsgdGhhdCBpcyBtb3JlIGdl
bmVyaWMgZml4dXAuDQoNClRoYW5rIHlvdSwNCg0KPg0KPlNpZ25lZC1vZmYtYnk6IFdhbmcgTmFu
IDx3YW5nbmFuMEBodWF3ZWkuY29tPg0KPkNjOiBBcm5hbGRvIENhcnZhbGhvIGRlIE1lbG8gPGFj
bWVAcmVkaGF0LmNvbT4NCj5DYzogSmlyaSBPbHNhIDxqb2xzYUBrZXJuZWwub3JnPg0KPkNjOiBN
YXNhbWkgSGlyYW1hdHN1IDxtYXNhbWkuaGlyYW1hdHN1LnB0QGhpdGFjaGkuY29tPg0KPkNjOiBO
YW1oeXVuZyBLaW0gPG5hbWh5dW5nQGtlcm5lbC5vcmc+DQo+LS0tDQo+IHRvb2xzL3BlcmYvYnVp
bHRpbi1wcm9iZS5jIHwgMTIgKysrKysrKysrKy0tDQo+IDEgZmlsZSBjaGFuZ2VkLCAxMCBpbnNl
cnRpb25zKCspLCAyIGRlbGV0aW9ucygtKQ0KPg0KPmRpZmYgLS1naXQgYS90b29scy9wZXJmL2J1
aWx0aW4tcHJvYmUuYyBiL3Rvb2xzL3BlcmYvYnVpbHRpbi1wcm9iZS5jDQo+aW5kZXggMTMyYWZj
OS4uODYxYWE4OSAxMDA2NDQNCj4tLS0gYS90b29scy9wZXJmL2J1aWx0aW4tcHJvYmUuYw0KPisr
KyBiL3Rvb2xzL3BlcmYvYnVpbHRpbi1wcm9iZS5jDQo+QEAgLTM4NCw3ICszODQsMTEgQEAgc3Rh
dGljIGludCBwZXJmX2RlbF9wcm9iZV9ldmVudHMoc3RydWN0IHN0cmZpbHRlciAqZmlsdGVyKQ0K
PiAJCWdvdG8gb3V0Ow0KPiAJfQ0KPg0KPi0JcmV0ID0gcHJvYmVfZmlsZV9fZ2V0X2V2ZW50cyhr
ZmQsIGZpbHRlciwga2xpc3QpOw0KPisJaWYgKGtmZCA8IDApDQo+KwkJcmV0ID0gLUVOT0VOVDsN
Cj4rCWVsc2UNCj4rCQlyZXQgPSBwcm9iZV9maWxlX19nZXRfZXZlbnRzKGtmZCwgZmlsdGVyLCBr
bGlzdCk7DQo+Kw0KPiAJaWYgKHJldCA9PSAwKSB7DQo+IAkJc3RybGlzdF9fZm9yX2VhY2goZW50
LCBrbGlzdCkNCj4gCQkJcHJfaW5mbygiUmVtb3ZlZCBldmVudDogJXNcbiIsIGVudC0+cyk7DQo+
QEAgLTM5NCw3ICszOTgsMTEgQEAgc3RhdGljIGludCBwZXJmX2RlbF9wcm9iZV9ldmVudHMoc3Ry
dWN0IHN0cmZpbHRlciAqZmlsdGVyKQ0KPiAJCQlnb3RvIGVycm9yOw0KPiAJfQ0KPg0KPi0JcmV0
MiA9IHByb2JlX2ZpbGVfX2dldF9ldmVudHModWZkLCBmaWx0ZXIsIHVsaXN0KTsNCj4rCWlmICh1
ZmQgPCAwKQ0KPisJCXJldDIgPSAtRU5PRU5UOw0KPisJZWxzZQ0KPisJCXJldDIgPSBwcm9iZV9m
aWxlX19nZXRfZXZlbnRzKHVmZCwgZmlsdGVyLCB1bGlzdCk7DQo+Kw0KPiAJaWYgKHJldDIgPT0g
MCkgew0KPiAJCXN0cmxpc3RfX2Zvcl9lYWNoKGVudCwgdWxpc3QpDQo+IAkJCXByX2luZm8oIlJl
bW92ZWQgZXZlbnQ6ICVzXG4iLCBlbnQtPnMpOw0KPi0tDQo+MS44LjMuNA0KDQo=
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1263308 — Re: [PATCH 1/2] perf probe: Only call probe_file__get_events() when fd is valid

From	Arnaldo Carvalho de Melo <acme@kernel.org>
Date	2015-11-05 16:00 +0100
Subject	Re: [PATCH 1/2] perf probe: Only call probe_file__get_events() when fd is valid
Message-ID	<qrub8-81X-31@gated-at.bofh.it>
In reply to	#1263201

Em Thu, Nov 05, 2015 at 01:19:24PM +0000, Wang Nan escreveu:
> In system with kprobe enabled but uprobe turned off, 'perf probe -d'
> causes segfault because it calls probe_file__get_events() with a
> negative fd (when deleting uprobe events).
> 
> This patch validates fds before calling probe_file__get_events().

Wouldn't this shorter patch be more robust by deferring the validation
to just before using the 'fd' value?

The end result is that probe_file__get_events() will return -ENOENT in
both calls, so ret and ret2 will both be set to -ENOENT, as in your
patch.

Masami?

- Arnaldo

diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
index 89dbeb92c68e..f04a8318a1a7 100644
--- a/tools/perf/util/probe-file.c
+++ b/tools/perf/util/probe-file.c
@@ -138,6 +138,9 @@ struct strlist *probe_file__get_rawlist(int fd)
 	char *p;
 	struct strlist *sl;
 
+	if (fd < 0)
+		return NULL;
+
 	sl = strlist__new(NULL, NULL);
 
 	fp = fdopen(dup(fd), "r");


diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
index 89dbeb92c68e..e5dc8e62f0f1 100644
--- a/tools/perf/util/probe-file.c
+++ b/tools/perf/util/probe-file.c
@@ -169,6 +169,9 @@ static struct strlist *__probe_file__get_namelist(int fd, bool include_group)
 	struct probe_trace_event tev;
 	int ret = 0;
 
+	if (fd < 0)
+		return NULL;
+
 	memset(&tev, 0, sizeof(tev));
 	rawlist = probe_file__get_rawlist(fd);
 	if (!rawlist)

 
> Signed-off-by: Wang Nan <wangnan0@huawei.com>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/builtin-probe.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
> index 132afc9..861aa89 100644
> --- a/tools/perf/builtin-probe.c
> +++ b/tools/perf/builtin-probe.c
> @@ -384,7 +384,11 @@ static int perf_del_probe_events(struct strfilter *filter)
>  		goto out;
>  	}
>  
> -	ret = probe_file__get_events(kfd, filter, klist);
> +	if (kfd < 0)
> +		ret = -ENOENT;
> +	else
> +		ret = probe_file__get_events(kfd, filter, klist);
> +
>  	if (ret == 0) {
>  		strlist__for_each(ent, klist)
>  			pr_info("Removed event: %s\n", ent->s);
> @@ -394,7 +398,11 @@ static int perf_del_probe_events(struct strfilter *filter)
>  			goto error;
>  	}
>  
> -	ret2 = probe_file__get_events(ufd, filter, ulist);
> +	if (ufd < 0)
> +		ret2 = -ENOENT;
> +	else
> +		ret2 = probe_file__get_events(ufd, filter, ulist);
> +
>  	if (ret2 == 0) {
>  		strlist__for_each(ent, ulist)
>  			pr_info("Removed event: %s\n", ent->s);
> -- 
> 1.8.3.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1263316 — RE: [PATCH 1/2] perf probe: Only call probe_file__get_events() when fd is valid

From	平松雅巳 / HIRAMATU，MASAMI <masami.hiramatsu.pt@hitachi.com>
Date	2015-11-05 16:10 +0100
Subject	RE: [PATCH 1/2] perf probe: Only call probe_file__get_events() when fd is valid
Message-ID	<qrukP-8kv-29@gated-at.bofh.it>
In reply to	#1263308

From: Arnaldo Carvalho de Melo [mailto:acme@kernel.org]
>Em Thu, Nov 05, 2015 at 01:19:24PM +0000, Wang Nan escreveu:
>> In system with kprobe enabled but uprobe turned off, 'perf probe -d'
>> causes segfault because it calls probe_file__get_events() with a
>> negative fd (when deleting uprobe events).
>>
>> This patch validates fds before calling probe_file__get_events().
>
>Wouldn't this shorter patch be more robust by deferring the validation
>to just before using the 'fd' value?
>
>The end result is that probe_file__get_events() will return -ENOENT in
>both calls, so ret and ret2 will both be set to -ENOENT, as in your
>patch.
>
>Masami?

Yes, I've suggested so :)
https://lkml.org/lkml/2015/11/5/353

Thanks!



>
>- Arnaldo
>
>diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
>index 89dbeb92c68e..f04a8318a1a7 100644
>--- a/tools/perf/util/probe-file.c
>+++ b/tools/perf/util/probe-file.c
>@@ -138,6 +138,9 @@ struct strlist *probe_file__get_rawlist(int fd)
> 	char *p;
> 	struct strlist *sl;
>
>+	if (fd < 0)
>+		return NULL;
>+
> 	sl = strlist__new(NULL, NULL);
>
> 	fp = fdopen(dup(fd), "r");
>
>
>diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
>index 89dbeb92c68e..e5dc8e62f0f1 100644
>--- a/tools/perf/util/probe-file.c
>+++ b/tools/perf/util/probe-file.c
>@@ -169,6 +169,9 @@ static struct strlist *__probe_file__get_namelist(int fd, bool include_group)
> 	struct probe_trace_event tev;
> 	int ret = 0;
>
>+	if (fd < 0)
>+		return NULL;
>+
> 	memset(&tev, 0, sizeof(tev));
> 	rawlist = probe_file__get_rawlist(fd);
> 	if (!rawlist)
>
>
>> Signed-off-by: Wang Nan <wangnan0@huawei.com>
>> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
>> Cc: Jiri Olsa <jolsa@kernel.org>
>> Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
>> Cc: Namhyung Kim <namhyung@kernel.org>
>> ---
>>  tools/perf/builtin-probe.c | 12 ++++++++++--
>>  1 file changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
>> index 132afc9..861aa89 100644
>> --- a/tools/perf/builtin-probe.c
>> +++ b/tools/perf/builtin-probe.c
>> @@ -384,7 +384,11 @@ static int perf_del_probe_events(struct strfilter *filter)
>>  		goto out;
>>  	}
>>
>> -	ret = probe_file__get_events(kfd, filter, klist);
>> +	if (kfd < 0)
>> +		ret = -ENOENT;
>> +	else
>> +		ret = probe_file__get_events(kfd, filter, klist);
>> +
>>  	if (ret == 0) {
>>  		strlist__for_each(ent, klist)
>>  			pr_info("Removed event: %s\n", ent->s);
>> @@ -394,7 +398,11 @@ static int perf_del_probe_events(struct strfilter *filter)
>>  			goto error;
>>  	}
>>
>> -	ret2 = probe_file__get_events(ufd, filter, ulist);
>> +	if (ufd < 0)
>> +		ret2 = -ENOENT;
>> +	else
>> +		ret2 = probe_file__get_events(ufd, filter, ulist);
>> +
>>  	if (ret2 == 0) {
>>  		strlist__for_each(ent, ulist)
>>  			pr_info("Removed event: %s\n", ent->s);
>> --
>> 1.8.3.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1263345 — Re: [PATCH 1/2] perf probe: Only call probe_file__get_events() when fd is valid

From	'Arnaldo Carvalho de Melo' <acme@kernel.org>
Date	2015-11-05 17:00 +0100
Subject	Re: [PATCH 1/2] perf probe: Only call probe_file__get_events() when fd is valid
Message-ID	<qrv7d-9F-25@gated-at.bofh.it>
In reply to	#1263316

Em Thu, Nov 05, 2015 at 03:07:23PM +0000, 平松雅巳 / HIRAMATU，MASAMI escreveu:
> From: Arnaldo Carvalho de Melo [mailto:acme@kernel.org]
> >Em Thu, Nov 05, 2015 at 01:19:24PM +0000, Wang Nan escreveu:
> >> In system with kprobe enabled but uprobe turned off, 'perf probe -d'
> >> causes segfault because it calls probe_file__get_events() with a
> >> negative fd (when deleting uprobe events).
> >>
> >> This patch validates fds before calling probe_file__get_events().
> >
> >Wouldn't this shorter patch be more robust by deferring the validation
> >to just before using the 'fd' value?
> >
> >The end result is that probe_file__get_events() will return -ENOENT in
> >both calls, so ret and ret2 will both be set to -ENOENT, as in your
> >patch.
> >
> >Masami?
> 
> Yes, I've suggested so :)

Kinda, you suggested to check at probe_file__get_events() , while I am
suggesting to check even deeper, just before the actual use of fd, in
probe_file__get_rawlist().

- Arnaldo

> https://lkml.org/lkml/2015/11/5/353
> 
> Thanks!
> 
> 
> 
> >
> >- Arnaldo
> >
> >diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
> >index 89dbeb92c68e..f04a8318a1a7 100644
> >--- a/tools/perf/util/probe-file.c
> >+++ b/tools/perf/util/probe-file.c
> >@@ -138,6 +138,9 @@ struct strlist *probe_file__get_rawlist(int fd)
> > 	char *p;
> > 	struct strlist *sl;
> >
> >+	if (fd < 0)
> >+		return NULL;
> >+
> > 	sl = strlist__new(NULL, NULL);
> >
> > 	fp = fdopen(dup(fd), "r");
> >
> >
> >diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
> >index 89dbeb92c68e..e5dc8e62f0f1 100644
> >--- a/tools/perf/util/probe-file.c
> >+++ b/tools/perf/util/probe-file.c
> >@@ -169,6 +169,9 @@ static struct strlist *__probe_file__get_namelist(int fd, bool include_group)
> > 	struct probe_trace_event tev;
> > 	int ret = 0;
> >
> >+	if (fd < 0)
> >+		return NULL;
> >+
> > 	memset(&tev, 0, sizeof(tev));
> > 	rawlist = probe_file__get_rawlist(fd);
> > 	if (!rawlist)
> >
> >
> >> Signed-off-by: Wang Nan <wangnan0@huawei.com>
> >> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> >> Cc: Jiri Olsa <jolsa@kernel.org>
> >> Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> >> Cc: Namhyung Kim <namhyung@kernel.org>
> >> ---
> >>  tools/perf/builtin-probe.c | 12 ++++++++++--
> >>  1 file changed, 10 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
> >> index 132afc9..861aa89 100644
> >> --- a/tools/perf/builtin-probe.c
> >> +++ b/tools/perf/builtin-probe.c
> >> @@ -384,7 +384,11 @@ static int perf_del_probe_events(struct strfilter *filter)
> >>  		goto out;
> >>  	}
> >>
> >> -	ret = probe_file__get_events(kfd, filter, klist);
> >> +	if (kfd < 0)
> >> +		ret = -ENOENT;
> >> +	else
> >> +		ret = probe_file__get_events(kfd, filter, klist);
> >> +
> >>  	if (ret == 0) {
> >>  		strlist__for_each(ent, klist)
> >>  			pr_info("Removed event: %s\n", ent->s);
> >> @@ -394,7 +398,11 @@ static int perf_del_probe_events(struct strfilter *filter)
> >>  			goto error;
> >>  	}
> >>
> >> -	ret2 = probe_file__get_events(ufd, filter, ulist);
> >> +	if (ufd < 0)
> >> +		ret2 = -ENOENT;
> >> +	else
> >> +		ret2 = probe_file__get_events(ufd, filter, ulist);
> >> +
> >>  	if (ret2 == 0) {
> >>  		strlist__for_each(ent, ulist)
> >>  			pr_info("Removed event: %s\n", ent->s);
> >> --
> >> 1.8.3.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1263879 — [PATCH v2] perf probe: Verify parameters for two functions

From	Wang Nan <wangnan0@huawei.com>
Date	2015-11-06 11:00 +0100
Subject	[PATCH v2] perf probe: Verify parameters for two functions
Message-ID	<qrLYm-2Oc-27@gated-at.bofh.it>
In reply to	#1263345

On kernel with only one of CONFIG_KPROBE_EVENTS and
CONFIG_UPROBE_EVENTS enabled, 'perf probe -d' causes segfault because
perf_del_probe_events() calls probe_file__get_events() with a negative
fd.

This patch fixes it by add parameter validation at the entry of
probe_file__get_events() and probe_file__get_rawlist(). Since they are
both non-static public functions (in .h file), parameter verifying
is required.

v1 -> v2: Verify fd at the head of probe_file__get_rawlist() instead of
          checking at call site (suggested by Masami and Arnaldo at [1,2]).

[1] http://lkml.kernel.org/r/50399556C9727B4D88A595C8584AAB37526048E3@GSjpTKYDCembx32.service.hitachi.net
[2] http://lkml.kernel.org/r/20151105155830.GV13236@kernel.org

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/probe-file.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
index 89dbeb9..e3b3b92 100644
--- a/tools/perf/util/probe-file.c
+++ b/tools/perf/util/probe-file.c
@@ -138,6 +138,9 @@ struct strlist *probe_file__get_rawlist(int fd)
 	char *p;
 	struct strlist *sl;
 
+	if (fd < 0)
+		return NULL;
+
 	sl = strlist__new(NULL, NULL);
 
 	fp = fdopen(dup(fd), "r");
@@ -271,6 +274,9 @@ int probe_file__get_events(int fd, struct strfilter *filter,
 	const char *p;
 	int ret = -ENOENT;
 
+	if (!plist)
+		return -EINVAL;
+
 	namelist = __probe_file__get_namelist(fd, true);
 	if (!namelist)
 		return -ENOENT;
-- 
1.8.3.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1263889 — RE: [PATCH v2] perf probe: Verify parameters for two functions

From	平松雅巳 / HIRAMATU，MASAMI <masami.hiramatsu.pt@hitachi.com>
Date	2015-11-06 11:10 +0100
Subject	RE: [PATCH v2] perf probe: Verify parameters for two functions
Message-ID	<qrM82-36S-37@gated-at.bofh.it>
In reply to	#1263879

RnJvbTogV2FuZyBOYW4gW21haWx0bzp3YW5nbmFuMEBodWF3ZWkuY29tXQ0KPg0KPk9uIGtlcm5l
bCB3aXRoIG9ubHkgb25lIG9mIENPTkZJR19LUFJPQkVfRVZFTlRTIGFuZA0KPkNPTkZJR19VUFJP
QkVfRVZFTlRTIGVuYWJsZWQsICdwZXJmIHByb2JlIC1kJyBjYXVzZXMgc2VnZmF1bHQgYmVjYXVz
ZQ0KPnBlcmZfZGVsX3Byb2JlX2V2ZW50cygpIGNhbGxzIHByb2JlX2ZpbGVfX2dldF9ldmVudHMo
KSB3aXRoIGEgbmVnYXRpdmUNCj5mZC4NCj4NCj5UaGlzIHBhdGNoIGZpeGVzIGl0IGJ5IGFkZCBw
YXJhbWV0ZXIgdmFsaWRhdGlvbiBhdCB0aGUgZW50cnkgb2YNCj5wcm9iZV9maWxlX19nZXRfZXZl
bnRzKCkgYW5kIHByb2JlX2ZpbGVfX2dldF9yYXdsaXN0KCkuIFNpbmNlIHRoZXkgYXJlDQo+Ym90
aCBub24tc3RhdGljIHB1YmxpYyBmdW5jdGlvbnMgKGluIC5oIGZpbGUpLCBwYXJhbWV0ZXIgdmVy
aWZ5aW5nDQo+aXMgcmVxdWlyZWQuDQoNCkxvb2tzIGdvb2QgdG8gbWUgISA6KQ0KDQpBY2tlZC1i
eTogTWFzYW1pIEhpcmFtYXRzdSA8bWFzYW1pLmhpcmFtYXRzdS5wdEBoaXRhY2hpLmNvbT4NCg0K
VGhhbmsgeW91IQ0KDQo+DQo+djEgLT4gdjI6IFZlcmlmeSBmZCBhdCB0aGUgaGVhZCBvZiBwcm9i
ZV9maWxlX19nZXRfcmF3bGlzdCgpIGluc3RlYWQgb2YNCj4gICAgICAgICAgY2hlY2tpbmcgYXQg
Y2FsbCBzaXRlIChzdWdnZXN0ZWQgYnkgTWFzYW1pIGFuZCBBcm5hbGRvIGF0IFsxLDJdKS4NCj4N
Cj5bMV0gaHR0cDovL2xrbWwua2VybmVsLm9yZy9yLzUwMzk5NTU2Qzk3MjdCNEQ4OEE1OTVDODU4
NEFBQjM3NTI2MDQ4RTNAR1NqcFRLWURDZW1ieDMyLnNlcnZpY2UuaGl0YWNoaS5uZXQNCj5bMl0g
aHR0cDovL2xrbWwua2VybmVsLm9yZy9yLzIwMTUxMTA1MTU1ODMwLkdWMTMyMzZAa2VybmVsLm9y
Zw0KPg0KPlNpZ25lZC1vZmYtYnk6IFdhbmcgTmFuIDx3YW5nbmFuMEBodWF3ZWkuY29tPg0KPkNj
OiBBcm5hbGRvIENhcnZhbGhvIGRlIE1lbG8gPGFjbWVAcmVkaGF0LmNvbT4NCj5DYzogSmlyaSBP
bHNhIDxqb2xzYUBrZXJuZWwub3JnPg0KPkNjOiBNYXNhbWkgSGlyYW1hdHN1IDxtYXNhbWkuaGly
YW1hdHN1LnB0QGhpdGFjaGkuY29tPg0KPkNjOiBOYW1oeXVuZyBLaW0gPG5hbWh5dW5nQGtlcm5l
bC5vcmc+DQo+LS0tDQo+IHRvb2xzL3BlcmYvdXRpbC9wcm9iZS1maWxlLmMgfCA2ICsrKysrKw0K
PiAxIGZpbGUgY2hhbmdlZCwgNiBpbnNlcnRpb25zKCspDQo+DQo+ZGlmZiAtLWdpdCBhL3Rvb2xz
L3BlcmYvdXRpbC9wcm9iZS1maWxlLmMgYi90b29scy9wZXJmL3V0aWwvcHJvYmUtZmlsZS5jDQo+
aW5kZXggODlkYmViOS4uZTNiM2I5MiAxMDA2NDQNCj4tLS0gYS90b29scy9wZXJmL3V0aWwvcHJv
YmUtZmlsZS5jDQo+KysrIGIvdG9vbHMvcGVyZi91dGlsL3Byb2JlLWZpbGUuYw0KPkBAIC0xMzgs
NiArMTM4LDkgQEAgc3RydWN0IHN0cmxpc3QgKnByb2JlX2ZpbGVfX2dldF9yYXdsaXN0KGludCBm
ZCkNCj4gCWNoYXIgKnA7DQo+IAlzdHJ1Y3Qgc3RybGlzdCAqc2w7DQo+DQo+KwlpZiAoZmQgPCAw
KQ0KPisJCXJldHVybiBOVUxMOw0KPisNCj4gCXNsID0gc3RybGlzdF9fbmV3KE5VTEwsIE5VTEwp
Ow0KPg0KPiAJZnAgPSBmZG9wZW4oZHVwKGZkKSwgInIiKTsNCj5AQCAtMjcxLDYgKzI3NCw5IEBA
IGludCBwcm9iZV9maWxlX19nZXRfZXZlbnRzKGludCBmZCwgc3RydWN0IHN0cmZpbHRlciAqZmls
dGVyLA0KPiAJY29uc3QgY2hhciAqcDsNCj4gCWludCByZXQgPSAtRU5PRU5UOw0KPg0KPisJaWYg
KCFwbGlzdCkNCj4rCQlyZXR1cm4gLUVJTlZBTDsNCj4rDQo+IAluYW1lbGlzdCA9IF9fcHJvYmVf
ZmlsZV9fZ2V0X25hbWVsaXN0KGZkLCB0cnVlKTsNCj4gCWlmICghbmFtZWxpc3QpDQo+IAkJcmV0
dXJuIC1FTk9FTlQ7DQo+LS0NCj4xLjguMy40DQoNCg==
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1266993 — Re: [PATCH v2] perf probe: Verify parameters for two functions

From	"Wangnan (F)" <wangnan0@huawei.com>
Date	2015-11-11 08:10 +0100
Subject	Re: [PATCH v2] perf probe: Verify parameters for two functions
Message-ID	<qtxHA-80B-1@gated-at.bofh.it>
In reply to	#1263879

Hi Arnaldo,

Could you please collect this patch to your tree? It fixes a segfault
when only one of kprobe and uprobe is enabled.

Thank you.

On 2015/11/6 17:50, Wang Nan wrote:
> On kernel with only one of CONFIG_KPROBE_EVENTS and
> CONFIG_UPROBE_EVENTS enabled, 'perf probe -d' causes segfault because
> perf_del_probe_events() calls probe_file__get_events() with a negative
> fd.
>
> This patch fixes it by add parameter validation at the entry of
> probe_file__get_events() and probe_file__get_rawlist(). Since they are
> both non-static public functions (in .h file), parameter verifying
> is required.
>
> v1 -> v2: Verify fd at the head of probe_file__get_rawlist() instead of
>            checking at call site (suggested by Masami and Arnaldo at [1,2]).
>
> [1] http://lkml.kernel.org/r/50399556C9727B4D88A595C8584AAB37526048E3@GSjpTKYDCembx32.service.hitachi.net
> [2] http://lkml.kernel.org/r/20151105155830.GV13236@kernel.org
>
> Signed-off-by: Wang Nan <wangnan0@huawei.com>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> ---
>   tools/perf/util/probe-file.c | 6 ++++++
>   1 file changed, 6 insertions(+)
>
> diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
> index 89dbeb9..e3b3b92 100644
> --- a/tools/perf/util/probe-file.c
> +++ b/tools/perf/util/probe-file.c
> @@ -138,6 +138,9 @@ struct strlist *probe_file__get_rawlist(int fd)
>   	char *p;
>   	struct strlist *sl;
>   
> +	if (fd < 0)
> +		return NULL;
> +
>   	sl = strlist__new(NULL, NULL);
>   
>   	fp = fdopen(dup(fd), "r");
> @@ -271,6 +274,9 @@ int probe_file__get_events(int fd, struct strfilter *filter,
>   	const char *p;
>   	int ret = -ENOENT;
>   
> +	if (!plist)
> +		return -EINVAL;
> +
>   	namelist = __probe_file__get_namelist(fd, true);
>   	if (!namelist)
>   		return -ENOENT;


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1267656 — [tip:perf/urgent] perf probe: Verify parameters in two functions

From	tip-bot for Wang Nan <tipbot@zytor.com>
Date	2015-11-12 07:50 +0100
Subject	[tip:perf/urgent] perf probe: Verify parameters in two functions
Message-ID	<qtTRL-5pg-7@gated-at.bofh.it>
In reply to	#1263879

Commit-ID:  421fd0845eaeecce6b3806f7f0c0d67d1f9ad108
Gitweb:     http://git.kernel.org/tip/421fd0845eaeecce6b3806f7f0c0d67d1f9ad108
Author:     Wang Nan <wangnan0@huawei.com>
AuthorDate: Fri, 6 Nov 2015 09:50:15 +0000
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 11 Nov 2015 18:41:32 -0300

perf probe: Verify parameters in two functions

On kernel with only one out of CONFIG_KPROBE_EVENTS and
CONFIG_UPROBE_EVENTS enabled, 'perf probe -d' causes a segfault because
perf_del_probe_events() calls probe_file__get_events() with a negative
fd.

This patch fixes it by adding parameter validation at the entry of
probe_file__get_events() and probe_file__get_rawlist(). Since they are
both non-static public functions (in .h file), parameter verifying is
required.

v1 -> v2: Verify fd at the head of probe_file__get_rawlist() instead of
          checking at call site (suggested by Masami and Arnaldo at [1,2]).

[1] http://lkml.kernel.org/r/50399556C9727B4D88A595C8584AAB37526048E3@GSjpTKYDCembx32.service.hitachi.net
[2] http://lkml.kernel.org/r/20151105155830.GV13236@kernel.org

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Zefan Li <lizefan@huawei.com>
Cc: pi3orama@163.com
Link: http://lkml.kernel.org/r/1446803415-83382-1-git-send-email-wangnan0@huawei.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/probe-file.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
index 89dbeb9..e3b3b92 100644
--- a/tools/perf/util/probe-file.c
+++ b/tools/perf/util/probe-file.c
@@ -138,6 +138,9 @@ struct strlist *probe_file__get_rawlist(int fd)
 	char *p;
 	struct strlist *sl;
 
+	if (fd < 0)
+		return NULL;
+
 	sl = strlist__new(NULL, NULL);
 
 	fp = fdopen(dup(fd), "r");
@@ -271,6 +274,9 @@ int probe_file__get_events(int fd, struct strfilter *filter,
 	const char *p;
 	int ret = -ENOENT;
 
+	if (!plist)
+		return -EINVAL;
+
 	namelist = __probe_file__get_namelist(fd, true);
 	if (!namelist)
 		return -ENOENT;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

csiph-web

[PATCH 0/2] perf tools: Two bugfixs related to perf probe

Contents

#1263187 — [PATCH 0/2] perf tools: Two bugfixs related to perf probe

#1263194 — [PATCH 2/2] perf tools: Fix find_perf_probe_point_from_map() which incorrectly returns success

#1263236 — RE: [PATCH 2/2] perf tools: Fix find_perf_probe_point_from_map() which incorrectly returns success

#1263357 — Re: [PATCH 2/2] perf tools: Fix find_perf_probe_point_from_map() which incorrectly returns success

#1263759 — RE: [PATCH 2/2] perf tools: Fix find_perf_probe_point_from_map() which incorrectly returns success

#1263796 — RE: [PATCH 2/2] perf tools: Fix find_perf_probe_point_from_map() which incorrectly returns success

#1263835 — Re: [PATCH 2/2] perf tools: Fix find_perf_probe_point_from_map() which incorrectly returns success

#1263869 — Re: [PATCH 2/2] perf tools: Fix find_perf_probe_point_from_map() which incorrectly returns success

#1264029 — Re: [PATCH 2/2] perf tools: Fix find_perf_probe_point_from_map() which incorrectly returns success

#1265023 — [tip:perf/urgent] perf tools: Fix find_perf_probe_point_from_map( ) which incorrectly returns success

#1263201 — [PATCH 1/2] perf probe: Only call probe_file__get_events() when fd is valid

#1263245 — RE: [PATCH 1/2] perf probe: Only call probe_file__get_events() when fd is valid

#1263308 — Re: [PATCH 1/2] perf probe: Only call probe_file__get_events() when fd is valid

#1263316 — RE: [PATCH 1/2] perf probe: Only call probe_file__get_events() when fd is valid

#1263345 — Re: [PATCH 1/2] perf probe: Only call probe_file__get_events() when fd is valid

#1263879 — [PATCH v2] perf probe: Verify parameters for two functions

#1263889 — RE: [PATCH v2] perf probe: Verify parameters for two functions

#1266993 — Re: [PATCH v2] perf probe: Verify parameters for two functions

#1267656 — [tip:perf/urgent] perf probe: Verify parameters in two functions