1. 13 Jul, 2023 1 commit
    • Zheng Yejian's avatar
      tracing: Fix memory leak of iter->temp when reading trace_pipe · d5a82189
      Zheng Yejian authored
      kmemleak reports:
        unreferenced object 0xffff88814d14e200 (size 256):
          comm "cat", pid 336, jiffies 4294871818 (age 779.490s)
          hex dump (first 32 bytes):
            04 00 01 03 00 00 00 00 08 00 00 00 00 00 00 00  ................
            0c d8 c8 9b ff ff ff ff 04 5a ca 9b ff ff ff ff  .........Z......
          backtrace:
            [<ffffffff9bdff18f>] __kmalloc+0x4f/0x140
            [<ffffffff9bc9238b>] trace_find_next_entry+0xbb/0x1d0
            [<ffffffff9bc9caef>] trace_print_lat_context+0xaf/0x4e0
            [<ffffffff9bc94490>] print_trace_line+0x3e0/0x950
            [<ffffffff9bc95499>] tracing_read_pipe+0x2d9/0x5a0
            [<ffffffff9bf03a43>] vfs_read+0x143/0x520
            [<ffffffff9bf04c2d>] ksys_read+0xbd/0x160
            [<ffffffff9d0f0edf>] do_syscall_64+0x3f/0x90
            [<ffffffff9d2000aa>] entry_SYSCALL_64_after_hwframe+0x6e/0xd8
      
      when reading file 'trace_pipe', 'iter->temp' is allocated or relocated
      in trace_find_next_entry() but not freed before 'trace_pipe' is closed.
      
      To fix it, free 'iter->temp' in tracing_release_pipe().
      
      Link: https://lore.kernel.org/linux-trace-kernel/20230713141435.1133021-1-zhengyejian1@huawei.com
      
      Cc: stable@vger.kernel.org
      Fixes: ff895103 ("tracing: Save off entry when peeking at next entry")
      Signed-off-by: default avatarZheng Yejian <zhengyejian1@huawei.com>
      Signed-off-by: default avatarSteven Rostedt (Google) <rostedt@goodmis.org>
      d5a82189
  2. 12 Jul, 2023 6 commits
    • Mohamed Khalfella's avatar
      tracing/histograms: Add histograms to hist_vars if they have referenced variables · 6018b585
      Mohamed Khalfella authored
      Hist triggers can have referenced variables without having direct
      variables fields. This can be the case if referenced variables are added
      for trigger actions. In this case the newly added references will not
      have field variables. Not taking such referenced variables into
      consideration can result in a bug where it would be possible to remove
      hist trigger with variables being refenced. This will result in a bug
      that is easily reproducable like so
      
      $ cd /sys/kernel/tracing
      $ echo 'synthetic_sys_enter char[] comm; long id' >> synthetic_events
      $ echo 'hist:keys=common_pid.execname,id.syscall:vals=hitcount:comm=common_pid.execname' >> events/raw_syscalls/sys_enter/trigger
      $ echo 'hist:keys=common_pid.execname,id.syscall:onmatch(raw_syscalls.sys_enter).synthetic_sys_enter($comm, id)' >> events/raw_syscalls/sys_enter/trigger
      $ echo '!hist:keys=common_pid.execname,id.syscall:vals=hitcount:comm=common_pid.execname' >> events/raw_syscalls/sys_enter/trigger
      
      [  100.263533] ==================================================================
      [  100.264634] BUG: KASAN: slab-use-after-free in resolve_var_refs+0xc7/0x180
      [  100.265520] Read of size 8 at addr ffff88810375d0f0 by task bash/439
      [  100.266320]
      [  100.266533] CPU: 2 PID: 439 Comm: bash Not tainted 6.5.0-rc1 #4
      [  100.267277] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.0-20220807_005459-localhost 04/01/2014
      [  100.268561] Call Trace:
      [  100.268902]  <TASK>
      [  100.269189]  dump_stack_lvl+0x4c/0x70
      [  100.269680]  print_report+0xc5/0x600
      [  100.270165]  ? resolve_var_refs+0xc7/0x180
      [  100.270697]  ? kasan_complete_mode_report_info+0x80/0x1f0
      [  100.271389]  ? resolve_var_refs+0xc7/0x180
      [  100.271913]  kasan_report+0xbd/0x100
      [  100.272380]  ? resolve_var_refs+0xc7/0x180
      [  100.272920]  __asan_load8+0x71/0xa0
      [  100.273377]  resolve_var_refs+0xc7/0x180
      [  100.273888]  event_hist_trigger+0x749/0x860
      [  100.274505]  ? kasan_save_stack+0x2a/0x50
      [  100.275024]  ? kasan_set_track+0x29/0x40
      [  100.275536]  ? __pfx_event_hist_trigger+0x10/0x10
      [  100.276138]  ? ksys_write+0xd1/0x170
      [  100.276607]  ? do_syscall_64+0x3c/0x90
      [  100.277099]  ? entry_SYSCALL_64_after_hwframe+0x6e/0xd8
      [  100.277771]  ? destroy_hist_data+0x446/0x470
      [  100.278324]  ? event_hist_trigger_parse+0xa6c/0x3860
      [  100.278962]  ? __pfx_event_hist_trigger_parse+0x10/0x10
      [  100.279627]  ? __kasan_check_write+0x18/0x20
      [  100.280177]  ? mutex_unlock+0x85/0xd0
      [  100.280660]  ? __pfx_mutex_unlock+0x10/0x10
      [  100.281200]  ? kfree+0x7b/0x120
      [  100.281619]  ? ____kasan_slab_free+0x15d/0x1d0
      [  100.282197]  ? event_trigger_write+0xac/0x100
      [  100.282764]  ? __kasan_slab_free+0x16/0x20
      [  100.283293]  ? __kmem_cache_free+0x153/0x2f0
      [  100.283844]  ? sched_mm_cid_remote_clear+0xb1/0x250
      [  100.284550]  ? __pfx_sched_mm_cid_remote_clear+0x10/0x10
      [  100.285221]  ? event_trigger_write+0xbc/0x100
      [  100.285781]  ? __kasan_check_read+0x15/0x20
      [  100.286321]  ? __bitmap_weight+0x66/0xa0
      [  100.286833]  ? _find_next_bit+0x46/0xe0
      [  100.287334]  ? task_mm_cid_work+0x37f/0x450
      [  100.287872]  event_triggers_call+0x84/0x150
      [  100.288408]  trace_event_buffer_commit+0x339/0x430
      [  100.289073]  ? ring_buffer_event_data+0x3f/0x60
      [  100.292189]  trace_event_raw_event_sys_enter+0x8b/0xe0
      [  100.295434]  syscall_trace_enter.constprop.0+0x18f/0x1b0
      [  100.298653]  syscall_enter_from_user_mode+0x32/0x40
      [  100.301808]  do_syscall_64+0x1a/0x90
      [  100.304748]  entry_SYSCALL_64_after_hwframe+0x6e/0xd8
      [  100.307775] RIP: 0033:0x7f686c75c1cb
      [  100.310617] Code: 73 01 c3 48 8b 0d 65 3c 10 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa b8 21 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 35 3c 10 00 f7 d8 64 89 01 48
      [  100.317847] RSP: 002b:00007ffc60137a38 EFLAGS: 00000246 ORIG_RAX: 0000000000000021
      [  100.321200] RAX: ffffffffffffffda RBX: 000055f566469ea0 RCX: 00007f686c75c1cb
      [  100.324631] RDX: 0000000000000001 RSI: 0000000000000001 RDI: 000000000000000a
      [  100.328104] RBP: 00007ffc60137ac0 R08: 00007f686c818460 R09: 000000000000000a
      [  100.331509] R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000009
      [  100.334992] R13: 0000000000000007 R14: 000000000000000a R15: 0000000000000007
      [  100.338381]  </TASK>
      
      We hit the bug because when second hist trigger has was created
      has_hist_vars() returned false because hist trigger did not have
      variables. As a result of that save_hist_vars() was not called to add
      the trigger to trace_array->hist_vars. Later on when we attempted to
      remove the first histogram find_any_var_ref() failed to detect it is
      being used because it did not find the second trigger in hist_vars list.
      
      With this change we wait until trigger actions are created so we can take
      into consideration if hist trigger has variable references. Also, now we
      check the return value of save_hist_vars() and fail trigger creation if
      save_hist_vars() fails.
      
      Link: https://lore.kernel.org/linux-trace-kernel/20230712223021.636335-1-mkhalfella@purestorage.com
      
      Cc: stable@vger.kernel.org
      Fixes: 067fe038 ("tracing: Add variable reference handling to hist triggers")
      Signed-off-by: default avatarMohamed Khalfella <mkhalfella@purestorage.com>
      Signed-off-by: default avatarSteven Rostedt (Google) <rostedt@goodmis.org>
      6018b585
    • Steven Rostedt (Google)'s avatar
      tracing: Stop FORTIFY_SOURCE complaining about stack trace caller · bec3c25c
      Steven Rostedt (Google) authored
      The stack_trace event is an event created by the tracing subsystem to
      store stack traces. It originally just contained a hard coded array of 8
      words to hold the stack, and a "size" to know how many entries are there.
      This is exported to user space as:
      
      name: kernel_stack
      ID: 4
      format:
      	field:unsigned short common_type;	offset:0;	size:2;	signed:0;
      	field:unsigned char common_flags;	offset:2;	size:1;	signed:0;
      	field:unsigned char common_preempt_count;	offset:3;	size:1;	signed:0;
      	field:int common_pid;	offset:4;	size:4;	signed:1;
      
      	field:int size;	offset:8;	size:4;	signed:1;
      	field:unsigned long caller[8];	offset:16;	size:64;	signed:0;
      
      print fmt: "\t=> %ps\n\t=> %ps\n\t=> %ps\n" "\t=> %ps\n\t=> %ps\n\t=> %ps\n" "\t=> %ps\n\t=> %ps\n",i
       (void *)REC->caller[0], (void *)REC->caller[1], (void *)REC->caller[2],
       (void *)REC->caller[3], (void *)REC->caller[4], (void *)REC->caller[5],
       (void *)REC->caller[6], (void *)REC->caller[7]
      
      Where the user space tracers could parse the stack. The library was
      updated for this specific event to only look at the size, and not the
      array. But some older users still look at the array (note, the older code
      still checks to make sure the array fits inside the event that it read.
      That is, if only 4 words were saved, the parser would not read the fifth
      word because it will see that it was outside of the event size).
      
      This event was changed a while ago to be more dynamic, and would save a
      full stack even if it was greater than 8 words. It does this by simply
      allocating more ring buffer to hold the extra words. Then it copies in the
      stack via:
      
      	memcpy(&entry->caller, fstack->calls, size);
      
      As the entry is struct stack_entry, that is created by a macro to both
      create the structure and export this to user space, it still had the caller
      field of entry defined as: unsigned long caller[8].
      
      When the stack is greater than 8, the FORTIFY_SOURCE code notices that the
      amount being copied is greater than the source array and complains about
      it. It has no idea that the source is pointing to the ring buffer with the
      required allocation.
      
      To hide this from the FORTIFY_SOURCE logic, pointer arithmetic is used:
      
      	ptr = ring_buffer_event_data(event);
      	entry = ptr;
      	ptr += offsetof(typeof(*entry), caller);
      	memcpy(ptr, fstack->calls, size);
      
      Link: https://lore.kernel.org/all/20230612160748.4082850-1-svens@linux.ibm.com/
      Link: https://lore.kernel.org/linux-trace-kernel/20230712105235.5fc441aa@gandalf.local.home
      
      Cc: Masami Hiramatsu <mhiramat@kernel.org>
      Cc: Mark Rutland <mark.rutland@arm.com>
      Reported-by: default avatarSven Schnelle <svens@linux.ibm.com>
      Tested-by: default avatarSven Schnelle <svens@linux.ibm.com>
      Signed-off-by: default avatarSteven Rostedt (Google) <rostedt@goodmis.org>
      bec3c25c
    • Zheng Yejian's avatar
      ftrace: Fix possible warning on checking all pages used in ftrace_process_locs() · 26efd79c
      Zheng Yejian authored
      As comments in ftrace_process_locs(), there may be NULL pointers in
      mcount_loc section:
       > Some architecture linkers will pad between
       > the different mcount_loc sections of different
       > object files to satisfy alignments.
       > Skip any NULL pointers.
      
      After commit 20e5227e ("ftrace: allow NULL pointers in mcount_loc"),
      NULL pointers will be accounted when allocating ftrace pages but skipped
      before adding into ftrace pages, this may result in some pages not being
      used. Then after commit 706c81f8 ("ftrace: Remove extra helper
      functions"), warning may occur at:
        WARN_ON(pg->next);
      
      To fix it, only warn for case that no pointers skipped but pages not used
      up, then free those unused pages after releasing ftrace_lock.
      
      Link: https://lore.kernel.org/linux-trace-kernel/20230712060452.3175675-1-zhengyejian1@huawei.com
      
      Cc: stable@vger.kernel.org
      Fixes: 706c81f8 ("ftrace: Remove extra helper functions")
      Suggested-by: default avatarSteven Rostedt <rostedt@goodmis.org>
      Signed-off-by: default avatarZheng Yejian <zhengyejian1@huawei.com>
      Signed-off-by: default avatarSteven Rostedt (Google) <rostedt@goodmis.org>
      26efd79c
    • Zheng Yejian's avatar
      ring-buffer: Fix deadloop issue on reading trace_pipe · 7e42907f
      Zheng Yejian authored
      Soft lockup occurs when reading file 'trace_pipe':
      
        watchdog: BUG: soft lockup - CPU#6 stuck for 22s! [cat:4488]
        [...]
        RIP: 0010:ring_buffer_empty_cpu+0xed/0x170
        RSP: 0018:ffff88810dd6fc48 EFLAGS: 00000246
        RAX: 0000000000000000 RBX: 0000000000000246 RCX: ffffffff93d1aaeb
        RDX: ffff88810a280040 RSI: 0000000000000008 RDI: ffff88811164b218
        RBP: ffff88811164b218 R08: 0000000000000000 R09: ffff88815156600f
        R10: ffffed102a2acc01 R11: 0000000000000001 R12: 0000000051651901
        R13: 0000000000000000 R14: ffff888115e49500 R15: 0000000000000000
        [...]
        CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
        CR2: 00007f8d853c2000 CR3: 000000010dcd8000 CR4: 00000000000006e0
        DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
        DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
        Call Trace:
         __find_next_entry+0x1a8/0x4b0
         ? peek_next_entry+0x250/0x250
         ? down_write+0xa5/0x120
         ? down_write_killable+0x130/0x130
         trace_find_next_entry_inc+0x3b/0x1d0
         tracing_read_pipe+0x423/0xae0
         ? tracing_splice_read_pipe+0xcb0/0xcb0
         vfs_read+0x16b/0x490
         ksys_read+0x105/0x210
         ? __ia32_sys_pwrite64+0x200/0x200
         ? switch_fpu_return+0x108/0x220
         do_syscall_64+0x33/0x40
         entry_SYSCALL_64_after_hwframe+0x61/0xc6
      
      Through the vmcore, I found it's because in tracing_read_pipe(),
      ring_buffer_empty_cpu() found some buffer is not empty but then it
      cannot read anything due to "rb_num_of_entries() == 0" always true,
      Then it infinitely loop the procedure due to user buffer not been
      filled, see following code path:
      
        tracing_read_pipe() {
          ... ...
          waitagain:
            tracing_wait_pipe() // 1. find non-empty buffer here
            trace_find_next_entry_inc()  // 2. loop here try to find an entry
              __find_next_entry()
                ring_buffer_empty_cpu();  // 3. find non-empty buffer
                peek_next_entry()  // 4. but peek always return NULL
                  ring_buffer_peek()
                    rb_buffer_peek()
                      rb_get_reader_page()
                        // 5. because rb_num_of_entries() == 0 always true here
                        //    then return NULL
            // 6. user buffer not been filled so goto 'waitgain'
            //    and eventually leads to an deadloop in kernel!!!
        }
      
      By some analyzing, I found that when resetting ringbuffer, the 'entries'
      of its pages are not all cleared (see rb_reset_cpu()). Then when reducing
      the ringbuffer, and if some reduced pages exist dirty 'entries' data, they
      will be added into 'cpu_buffer->overrun' (see rb_remove_pages()), which
      cause wrong 'overrun' count and eventually cause the deadloop issue.
      
      To fix it, we need to clear every pages in rb_reset_cpu().
      
      Link: https://lore.kernel.org/linux-trace-kernel/20230708225144.3785600-1-zhengyejian1@huawei.com
      
      Cc: stable@vger.kernel.org
      Fixes: a5fb8331 ("ring-buffer: Fix uninitialized read_stamp")
      Signed-off-by: default avatarZheng Yejian <zhengyejian1@huawei.com>
      Signed-off-by: default avatarSteven Rostedt (Google) <rostedt@goodmis.org>
      7e42907f
    • Arnd Bergmann's avatar
      tracing: arm64: Avoid missing-prototype warnings · 7d8b31b7
      Arnd Bergmann authored
      These are all tracing W=1 warnings in arm64 allmodconfig about missing
      prototypes:
      
      kernel/trace/trace_kprobe_selftest.c:7:5: error: no previous prototype for 'kprobe_trace_selftest_target' [-Werror=missing-pro
      totypes]
      kernel/trace/ftrace.c:329:5: error: no previous prototype for '__register_ftrace_function' [-Werror=missing-prototypes]
      kernel/trace/ftrace.c:372:5: error: no previous prototype for '__unregister_ftrace_function' [-Werror=missing-prototypes]
      kernel/trace/ftrace.c:4130:15: error: no previous prototype for 'arch_ftrace_match_adjust' [-Werror=missing-prototypes]
      kernel/trace/fgraph.c:243:15: error: no previous prototype for 'ftrace_return_to_handler' [-Werror=missing-prototypes]
      kernel/trace/fgraph.c:358:6: error: no previous prototype for 'ftrace_graph_sleep_time_control' [-Werror=missing-prototypes]
      arch/arm64/kernel/ftrace.c:460:6: error: no previous prototype for 'prepare_ftrace_return' [-Werror=missing-prototypes]
      arch/arm64/kernel/ptrace.c:2172:5: error: no previous prototype for 'syscall_trace_enter' [-Werror=missing-prototypes]
      arch/arm64/kernel/ptrace.c:2195:6: error: no previous prototype for 'syscall_trace_exit' [-Werror=missing-prototypes]
      
      Move the declarations to an appropriate header where they can be seen
      by the caller and callee, and make sure the headers are included where
      needed.
      
      Link: https://lore.kernel.org/linux-trace-kernel/20230517125215.930689-1-arnd@kernel.org
      
      Cc: Masami Hiramatsu <mhiramat@kernel.org>
      Cc: Mark Rutland <mark.rutland@arm.com>
      Cc: Will Deacon <will@kernel.org>
      Cc: Kees Cook <keescook@chromium.org>
      Cc: Florent Revest <revest@chromium.org>
      Signed-off-by: default avatarArnd Bergmann <arnd@arndb.de>
      Acked-by: default avatarCatalin Marinas <catalin.marinas@arm.com>
      [ Fixed ftrace_return_to_handler() to handle CONFIG_HAVE_FUNCTION_GRAPH_RETVAL case ]
      Signed-off-by: default avatarSteven Rostedt (Google) <rostedt@goodmis.org>
      7d8b31b7
    • Beau Belgrave's avatar
      selftests/user_events: Test struct size match cases · 769e6372
      Beau Belgrave authored
      The self tests for user_events currently does not ensure that the edge
      case for struct types work properly with size differences.
      
      Add cases for mis-matching struct names and sizes to ensure they work
      properly.
      
      Link: https://lkml.kernel.org/r/20230629235049.581-3-beaub@linux.microsoft.com
      
      Cc: Shuah Khan <skhan@linuxfoundation.org>
      Cc: linux-kselftest@vger.kernel.org
      Signed-off-by: default avatarBeau Belgrave <beaub@linux.microsoft.com>
      Signed-off-by: default avatarSteven Rostedt (Google) <rostedt@goodmis.org>
      769e6372
  3. 11 Jul, 2023 2 commits
  4. 10 Jul, 2023 2 commits
  5. 09 Jul, 2023 10 commits
  6. 08 Jul, 2023 19 commits