2022-11-04 13:36:29

by David Wang

[permalink] [raw]
Subject: [PATCH] trace/trace_uprobe: Only invoke uprobe ebpf handler when event matches.

Only invoke uprobe ebpf handler when event matches.

uprobe ebpf handler would be called even the event dose not
match any registered perf event, following steps would be
used to generate a unregistered perf event.
1. register a uprobe event on a specified pid <pid-a>
2. <pid-a> invokes syscall `clone` (via pthread_create),
new process <pid-b> generated. (Maybe it is a bug here, the
uprobe breakpoint is inherited from <pid-a>
3. <pid-b> invokes the function which is uprobed in step 1.
4. perf event generated...

Ebpf handler would be invoked even the event happened on <pid-b>,
but the default perf event handler make further check and ignore
the event because no registered perf event match on <pid-b>.

The patch means to fix the inconsistent behavior between ebpf and the default.
Before invoke uprobe ebpf handler, make sure current event match.

Signed-off-by: David Wang <[email protected]>
--
diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index fb58e86dd117..6f13163c0c0f 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -1346,27 +1346,20 @@ static void __uprobe_perf_func(struct trace_uprobe *tu,
void *data;
int size, esize;
int rctx;
+ bool trace_event_match = true;

-#ifdef CONFIG_BPF_EVENTS
- if (bpf_prog_array_valid(call)) {
- u32 ret;

- ret = bpf_prog_run_array_sleepable(call->prog_array, regs, bpf_prog_run);
- if (!ret)
- return;
+ preempt_disable();
+ head = this_cpu_ptr(call->perf_events);
+ if (hlist_empty(head)) {
+ trace_event_match = false;
+ goto out;
}
-#endif /* CONFIG_BPF_EVENTS */

esize = SIZEOF_TRACE_ENTRY(is_ret_probe(tu));
-
size = esize + tu->tp.size + dsize;
size = ALIGN(size + sizeof(u32), sizeof(u64)) - sizeof(u32);
if (WARN_ONCE(size > PERF_MAX_TRACE_SIZE, "profile buffer not large enough"))
- return;
-
- preempt_disable();
- head = this_cpu_ptr(call->perf_events);
- if (hlist_empty(head))
goto out;

entry = perf_trace_buf_alloc(size, NULL, &rctx);
@@ -1389,11 +1382,21 @@ static void __uprobe_perf_func(struct trace_uprobe *tu,

memset(data + len, 0, size - esize - len);
}
-
perf_trace_buf_submit(entry, size, rctx, call->event.type, 1, regs,
head, NULL);
out:
preempt_enable();
+
+#ifdef CONFIG_BPF_EVENTS
+ if (trace_event_match && bpf_prog_array_valid(call)) {
+ u32 ret;
+
+ ret = bpf_prog_run_array_sleepable(call->prog_array, regs, bpf_prog_run);
+ if (!ret)
+ return;
+ }
+#endif /* CONFIG_BPF_EVENTS */
+
}

/* uprobe profile handler */
--


2022-11-04 21:52:21

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH] trace/trace_uprobe: Only invoke uprobe ebpf handler when event matches.

Hi David,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v6.1-rc3 next-20221104]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/David-Wang/trace-trace_uprobe-Only-invoke-uprobe-ebpf-handler-when-event-matches/20221104-210925
patch link: https://lore.kernel.org/r/41828b90.5798.18442c10017.Coremail.00107082%40163.com
patch subject: [PATCH] trace/trace_uprobe: Only invoke uprobe ebpf handler when event matches.
config: i386-defconfig
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
reproduce (this is a W=1 build):
# https://github.com/intel-lab-lkp/linux/commit/6c778c475c0a6c8a11c1a74705a5856681cf6cf0
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review David-Wang/trace-trace_uprobe-Only-invoke-uprobe-ebpf-handler-when-event-matches/20221104-210925
git checkout 6c778c475c0a6c8a11c1a74705a5856681cf6cf0
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash kernel/trace/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>

All warnings (new ones prefixed by >>):

kernel/trace/trace_uprobe.c: In function '__uprobe_perf_func':
>> kernel/trace/trace_uprobe.c:1349:14: warning: variable 'trace_event_match' set but not used [-Wunused-but-set-variable]
1349 | bool trace_event_match = true;
| ^~~~~~~~~~~~~~~~~


vim +/trace_event_match +1349 kernel/trace/trace_uprobe.c

1338
1339 static void __uprobe_perf_func(struct trace_uprobe *tu,
1340 unsigned long func, struct pt_regs *regs,
1341 struct uprobe_cpu_buffer *ucb, int dsize)
1342 {
1343 struct trace_event_call *call = trace_probe_event_call(&tu->tp);
1344 struct uprobe_trace_entry_head *entry;
1345 struct hlist_head *head;
1346 void *data;
1347 int size, esize;
1348 int rctx;
> 1349 bool trace_event_match = true;
1350
1351
1352 preempt_disable();
1353 head = this_cpu_ptr(call->perf_events);
1354 if (hlist_empty(head)) {
1355 trace_event_match = false;
1356 goto out;
1357 }
1358
1359 esize = SIZEOF_TRACE_ENTRY(is_ret_probe(tu));
1360 size = esize + tu->tp.size + dsize;
1361 size = ALIGN(size + sizeof(u32), sizeof(u64)) - sizeof(u32);
1362 if (WARN_ONCE(size > PERF_MAX_TRACE_SIZE, "profile buffer not large enough"))
1363 goto out;
1364
1365 entry = perf_trace_buf_alloc(size, NULL, &rctx);
1366 if (!entry)
1367 goto out;
1368
1369 if (is_ret_probe(tu)) {
1370 entry->vaddr[0] = func;
1371 entry->vaddr[1] = instruction_pointer(regs);
1372 data = DATAOF_TRACE_ENTRY(entry, true);
1373 } else {
1374 entry->vaddr[0] = instruction_pointer(regs);
1375 data = DATAOF_TRACE_ENTRY(entry, false);
1376 }
1377
1378 memcpy(data, ucb->buf, tu->tp.size + dsize);
1379
1380 if (size - esize > tu->tp.size + dsize) {
1381 int len = tu->tp.size + dsize;
1382
1383 memset(data + len, 0, size - esize - len);
1384 }
1385 perf_trace_buf_submit(entry, size, rctx, call->event.type, 1, regs,
1386 head, NULL);
1387 out:
1388 preempt_enable();
1389

--
0-DAY CI Kernel Test Service
https://01.org/lkp


Attachments:
(No filename) (3.61 kB)
config (136.38 kB)
Download all attachments

2022-11-05 07:05:15

by David Wang

[permalink] [raw]
Subject: Re:Re: [PATCH] trace/trace_uprobe: Only invoke uprobe ebpf handler when event matches.

Fix compile warning: unused variable trace_event_match
    
`trace_event_match` is used to indicate whether
current event matches any registered event. For
now, this variable is only used for ebpf handler
and CONFIG_BPF_EVENTS is needed, otherwise a compile
warning would be generated about unused variable.
    
Reported-by: kernel test robot <[email protected]>
Signed-off-by: David Wang <[email protected]>
--
diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index 6f13163c0c0f..0548c651699e 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -1346,13 +1346,17 @@ static void __uprobe_perf_func(struct trace_uprobe *tu,
void *data;
int size, esize;
int rctx;
+#ifdef CONFIG_BPF_EVENTS
bool trace_event_match = true;
+#endif /* CONFIG_BPF_EVENTS */


preempt_disable();
head = this_cpu_ptr(call->perf_events);
if (hlist_empty(head)) {
+#ifdef CONFIG_BPF_EVENTS
trace_event_match = false;
+#endif /* CONFIG_BPF_EVENTS */
goto out;
}

--

















At 2022-11-05 05:30:37, "kernel test robot" <[email protected]> wrote:
>Hi David,
>
>Thank you for the patch! Perhaps something to improve:
>
>[auto build test WARNING on linus/master]
>[also build test WARNING on v6.1-rc3 next-20221104]
>[If your patch is applied to the wrong git tree, kindly drop us a note.
>And when submitting patch, we suggest to use '--base' as documented in
>https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
>url: https://github.com/intel-lab-lkp/linux/commits/David-Wang/trace-trace_uprobe-Only-invoke-uprobe-ebpf-handler-when-event-matches/20221104-210925
>patch link: https://lore.kernel.org/r/41828b90.5798.18442c10017.Coremail.00107082%40163.com
>patch subject: [PATCH] trace/trace_uprobe: Only invoke uprobe ebpf handler when event matches.
>config: i386-defconfig
>compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
>reproduce (this is a W=1 build):
> # https://github.com/intel-lab-lkp/linux/commit/6c778c475c0a6c8a11c1a74705a5856681cf6cf0
> git remote add linux-review https://github.com/intel-lab-lkp/linux
> git fetch --no-tags linux-review David-Wang/trace-trace_uprobe-Only-invoke-uprobe-ebpf-handler-when-event-matches/20221104-210925
> git checkout 6c778c475c0a6c8a11c1a74705a5856681cf6cf0
> # save the config file
> mkdir build_dir && cp config build_dir/.config
> make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash kernel/trace/
>
>If you fix the issue, kindly add following tag where applicable
>| Reported-by: kernel test robot <[email protected]>
>
>All warnings (new ones prefixed by >>):
>
> kernel/trace/trace_uprobe.c: In function '__uprobe_perf_func':
>>> kernel/trace/trace_uprobe.c:1349:14: warning: variable 'trace_event_match' set but not used [-Wunused-but-set-variable]
> 1349 | bool trace_event_match = true;
> | ^~~~~~~~~~~~~~~~~
>
>
>vim +/trace_event_match +1349 kernel/trace/trace_uprobe.c
>
> 1338
> 1339 static void __uprobe_perf_func(struct trace_uprobe *tu,
> 1340 unsigned long func, struct pt_regs *regs,
> 1341 struct uprobe_cpu_buffer *ucb, int dsize)
> 1342 {
> 1343 struct trace_event_call *call = trace_probe_event_call(&tu->tp);
> 1344 struct uprobe_trace_entry_head *entry;
> 1345 struct hlist_head *head;
> 1346 void *data;
> 1347 int size, esize;
> 1348 int rctx;
>> 1349 bool trace_event_match = true;
> 1350
> 1351
> 1352 preempt_disable();
> 1353 head = this_cpu_ptr(call->perf_events);
> 1354 if (hlist_empty(head)) {
> 1355 trace_event_match = false;
> 1356 goto out;
> 1357 }
> 1358
> 1359 esize = SIZEOF_TRACE_ENTRY(is_ret_probe(tu));
> 1360 size = esize + tu->tp.size + dsize;
> 1361 size = ALIGN(size + sizeof(u32), sizeof(u64)) - sizeof(u32);
> 1362 if (WARN_ONCE(size > PERF_MAX_TRACE_SIZE, "profile buffer not large enough"))
> 1363 goto out;
> 1364
> 1365 entry = perf_trace_buf_alloc(size, NULL, &rctx);
> 1366 if (!entry)
> 1367 goto out;
> 1368
> 1369 if (is_ret_probe(tu)) {
> 1370 entry->vaddr[0] = func;
> 1371 entry->vaddr[1] = instruction_pointer(regs);
> 1372 data = DATAOF_TRACE_ENTRY(entry, true);
> 1373 } else {
> 1374 entry->vaddr[0] = instruction_pointer(regs);
> 1375 data = DATAOF_TRACE_ENTRY(entry, false);
> 1376 }
> 1377
> 1378 memcpy(data, ucb->buf, tu->tp.size + dsize);
> 1379
> 1380 if (size - esize > tu->tp.size + dsize) {
> 1381 int len = tu->tp.size + dsize;
> 1382
> 1383 memset(data + len, 0, size - esize - len);
> 1384 }
> 1385 perf_trace_buf_submit(entry, size, rctx, call->event.type, 1, regs,
> 1386 head, NULL);
> 1387 out:
> 1388 preempt_enable();
> 1389
>
>--
>0-DAY CI Kernel Test Service
>https://01.org/lkp