Re: [PATCH] selftests/ftrace: Correctly enable event in instance-event.tc

On 2023/7/11 06:37, Steven Rostedt wrote:

Shuah, can you take this?

Kindly ping and bug report :)

Hi, Steve, Ajay,

After this patch and run this testcase, I got an use-after-free report
by KASAN. Short log see [1], full logs see attach "panic.log".

And by simple bisect, I found it may be introduced by:

  27152bceea1d ("eventfs: Move tracing/events to eventfs")

Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=27152bceea1df27ffebb12ac9cd9adbf2c4c3f35

[1]
# ./ftracetest test.d/instances/instance-event.tc
=== Ftrace unit tests ===
[1] Test creation and deletion of trace instances while setting an event[   89.472397] ==================================================================
[   89.475053] BUG: KASAN: slab-use-after-free in __ftrace_event_enable_disable+0x1b/0x3a0
[   89.480039] Read of size 8 at addr ffff88814f5b2690 by task ftracetest/392
[   89.483906]
[   89.484560] CPU: 10 PID: 392 Comm: ftracetest Tainted: G        W      6.5.0+ #127
[   89.487033] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.15.0-0-g2dd4b9b3f840-prebuilt.qemu.org 04/01/2014
[   89.489751] Call Trace:
[   89.490315]  <TASK>
[   89.490817]  dump_stack_lvl+0x4b/0x80
[   89.491570]  print_report+0xd0/0x620
[   89.492296]  ? __virt_addr_valid+0xf9/0x180
[   89.493134]  ? __ftrace_event_enable_disable+0x1b/0x3a0
[   89.494151]  kasan_report+0xb6/0xf0
[   89.494881]  ? __ftrace_event_enable_disable+0x1b/0x3a0
[   89.495878]  __ftrace_event_enable_disable+0x1b/0x3a0
[   89.496805]  event_enable_write+0x109/0x170
[   89.497631]  ? __pfx_event_enable_write+0x10/0x10
[   89.498618]  ? __pfx_bpf_lsm_file_permission+0x10/0x10
[   89.499659]  ? security_file_permission+0x51/0x2d0
[   89.500604]  vfs_write+0x175/0x670
[   89.501385]  ? __pfx_vfs_write+0x10/0x10
[   89.502246]  ? __pfx__raw_spin_lock+0x10/0x10
[   89.503187]  ? expand_files+0x9b/0x330
[   89.504011]  ? __pfx_expand_files+0x10/0x10
[   89.504898]  ? set_close_on_exec+0x7b/0xe0
[   89.505782]  ? __fget_light+0xae/0x1e0
[   89.506615]  ? _raw_spin_lock+0x85/0xe0
[   89.507989]  ksys_write+0xbd/0x160
[   89.509302]  ? __pfx_ksys_write+0x10/0x10
[   89.510493]  ? dnotify_flush+0x38/0x220
[   89.511654]  ? fpregs_assert_state_consistent+0x5a/0x70
[   89.513297]  ? exit_to_user_mode_prepare+0x32/0x110
[   89.514692]  do_syscall_64+0x3f/0x90
[   89.515516]  entry_SYSCALL_64_after_hwframe+0x6e/0xd8

On Mon, 26 Jun 2023 19:12:55 +0900
Masami Hiramatsu (Google) [off-list ref] wrote:

quoted

On Mon, 26 Jun 2023 19:11:14 +0900
Masami Hiramatsu (Google) [off-list ref] wrote:

quoted

On Mon, 26 Jun 2023 08:11:44 +0800
Zheng Yejian [off-list ref] wrote:
   

quoted

Function instance_set() expects to enable event 'sched_switch', so we
should set 1 to its 'enable' file.

Testcase passed after this patch:
   # ./ftracetest test.d/instances/instance-event.tc
   === Ftrace unit tests ===
   [1] Test creation and deletion of trace instances while setting an event
   [PASS]

   # of passed:  1
   # of failed:  0
   # of unresolved:  0
   # of untested:  0
   # of unsupported:  0
   # of xfailed:  0
   # of undefined(test bug):  0
   

Good catch!

Fixes: 91e6f1ce8657 ("ftracetest: Add instance created, delete, read and enable event test")

It seems this tests internally broken but the result is same because this disabled
error checking ('set +e').

And

Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Acked-by: Steven Rostedt (Google) <rostedt@goodmis.org>

Thanks for your ack :)

-- Zheng Yejian

-- Steve

quoted

Thanks!

quoted

quoted

Signed-off-by: Zheng Yejian <redacted>
---
  .../testing/selftests/ftrace/test.d/instances/instance-event.tc | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/ftrace/test.d/instances/instance-event.tc b/tools/testing/selftests/ftrace/test.d/instances/instance-event.tc
index 0eb47fbb3f44..42422e425107 100644
--- a/tools/testing/selftests/ftrace/test.d/instances/instance-event.tc
+++ b/tools/testing/selftests/ftrace/test.d/instances/instance-event.tc

@@ -39,7 +39,7 @@ instance_read() {
  
  instance_set() {
          while :; do
-                echo 1 > foo/events/sched/sched_switch
+                echo 1 > foo/events/sched/sched_switch/enable
          done 2> /dev/null
  }
  

-- 
2.25.1
   

-- 
Masami Hiramatsu (Google) [off-list ref]