linux-trace-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] tracing: Fix missing errno when zero parser->idx in trace_pid_write
@ 2025-08-21  7:17 Pu Lehui
  2025-08-28  2:02 ` Pu Lehui
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Pu Lehui @ 2025-08-21  7:17 UTC (permalink / raw)
  To: rostedt, mhiramat, mathieu.desnoyers
  Cc: linux-kernel, linux-trace-kernel, pulehui

From: Pu Lehui <pulehui@huawei.com>

When trace_get_user in trace_pid_write parses an only space, the
!trace_parser_loaded branch will break with no errno, causing
tr->filtered_pids to still be assigned with pid_list, which may trigger
potential problems.

This patch will also silence the fault injection syzkaller warning in
tracepoint_add_func [0]. We can reproduce the warning by following the
steps below:
1. echo 8 >> set_event_notrace_pid. Let tr->filtered_pids owns one pid
   and register sched_switch tracepoint.
2. echo ' ' >> set_event_pid, and perform fault injection during chunk
   allocation of trace_pid_list_alloc. Let pid_list with no pid and
assign to tr->filtered_pids.
3. echo ' ' >> set_event_pid. Let pid_list is NULL and assign to
   tr->filtered_pids.
4. echo 9 >> set_event_pid, will trigger the double register
   sched_switch tracepoint warning.

Link: https://lore.kernel.org/all/67cb890e.050a0220.d8275.022e.GAE@google.com [0]
Fixes: b27f266f74fb ("tracing: Fix return value of trace_pid_write()")
Signed-off-by: Pu Lehui <pulehui@huawei.com>
---
 kernel/trace/trace.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 8d8935ed416d..feeb7eb71318 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -853,10 +853,10 @@ int trace_pid_write(struct trace_pid_list *filtered_pids,
 		ubuf += ret;
 		cnt -= ret;
 
+		ret = -EINVAL;
 		if (!trace_parser_loaded(&parser))
 			break;
 
-		ret = -EINVAL;
 		if (kstrtoul(parser.buffer, 0, &val))
 			break;
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] tracing: Fix missing errno when zero parser->idx in trace_pid_write
  2025-08-21  7:17 [PATCH] tracing: Fix missing errno when zero parser->idx in trace_pid_write Pu Lehui
@ 2025-08-28  2:02 ` Pu Lehui
  2025-08-28  2:58   ` Steven Rostedt
  2025-09-02 15:09 ` kernel test robot
  2025-09-03  4:15 ` Pu Lehui
  2 siblings, 1 reply; 5+ messages in thread
From: Pu Lehui @ 2025-08-28  2:02 UTC (permalink / raw)
  To: Pu Lehui, rostedt, mhiramat, mathieu.desnoyers
  Cc: linux-kernel, linux-trace-kernel



On 2025/8/21 15:17, Pu Lehui wrote:
> From: Pu Lehui <pulehui@huawei.com>
> 
> When trace_get_user in trace_pid_write parses an only space, the
> !trace_parser_loaded branch will break with no errno, causing
> tr->filtered_pids to still be assigned with pid_list, which may trigger
> potential problems.

Gentle ping~

Hi all, Is this commit looks proper?

> 
> This patch will also silence the fault injection syzkaller warning in
> tracepoint_add_func [0]. We can reproduce the warning by following the
> steps below:
> 1. echo 8 >> set_event_notrace_pid. Let tr->filtered_pids owns one pid
>     and register sched_switch tracepoint.
> 2. echo ' ' >> set_event_pid, and perform fault injection during chunk
>     allocation of trace_pid_list_alloc. Let pid_list with no pid and
> assign to tr->filtered_pids.
> 3. echo ' ' >> set_event_pid. Let pid_list is NULL and assign to
>     tr->filtered_pids.
> 4. echo 9 >> set_event_pid, will trigger the double register
>     sched_switch tracepoint warning.
> 
> Link: https://lore.kernel.org/all/67cb890e.050a0220.d8275.022e.GAE@google.com [0]
> Fixes: b27f266f74fb ("tracing: Fix return value of trace_pid_write()")
> Signed-off-by: Pu Lehui <pulehui@huawei.com>
> ---
>   kernel/trace/trace.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 8d8935ed416d..feeb7eb71318 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -853,10 +853,10 @@ int trace_pid_write(struct trace_pid_list *filtered_pids,
>   		ubuf += ret;
>   		cnt -= ret;
>   
> +		ret = -EINVAL;
>   		if (!trace_parser_loaded(&parser))
>   			break;
>   
> -		ret = -EINVAL;
>   		if (kstrtoul(parser.buffer, 0, &val))
>   			break;
>   

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] tracing: Fix missing errno when zero parser->idx in trace_pid_write
  2025-08-28  2:02 ` Pu Lehui
@ 2025-08-28  2:58   ` Steven Rostedt
  0 siblings, 0 replies; 5+ messages in thread
From: Steven Rostedt @ 2025-08-28  2:58 UTC (permalink / raw)
  To: Pu Lehui
  Cc: Pu Lehui, mhiramat, mathieu.desnoyers, linux-kernel,
	linux-trace-kernel

On Thu, 28 Aug 2025 10:02:54 +0800
Pu Lehui <pulehui@huawei.com> wrote:

> On 2025/8/21 15:17, Pu Lehui wrote:
> > From: Pu Lehui <pulehui@huawei.com>
> > 
> > When trace_get_user in trace_pid_write parses an only space, the
> > !trace_parser_loaded branch will break with no errno, causing
> > tr->filtered_pids to still be assigned with pid_list, which may trigger
> > potential problems.  
> 
> Gentle ping~
> 
> Hi all, Is this commit looks proper?

It's in the queue to be looked at:

  https://patchwork.kernel.org/project/linux-trace-kernel/list/

-- Steve

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] tracing: Fix missing errno when zero parser->idx in trace_pid_write
  2025-08-21  7:17 [PATCH] tracing: Fix missing errno when zero parser->idx in trace_pid_write Pu Lehui
  2025-08-28  2:02 ` Pu Lehui
@ 2025-09-02 15:09 ` kernel test robot
  2025-09-03  4:15 ` Pu Lehui
  2 siblings, 0 replies; 5+ messages in thread
From: kernel test robot @ 2025-09-02 15:09 UTC (permalink / raw)
  To: Pu Lehui
  Cc: oe-lkp, lkp, linux-kernel, linux-trace-kernel, rostedt, mhiramat,
	mathieu.desnoyers, pulehui, oliver.sang



Hello,

kernel test robot noticed "perf-sanity-tests.perf_ftrace_tests.fail" on:

commit: cebdd2c9a622becc41349f32ace1795d750beda8 ("[PATCH] tracing: Fix missing errno when zero parser->idx in trace_pid_write")
url: https://github.com/intel-lab-lkp/linux/commits/Pu-Lehui/tracing-Fix-missing-errno-when-zero-parser-idx-in-trace_pid_write/20250821-151736
base: https://git.kernel.org/cgit/linux/kernel/git/trace/linux-trace for-next
patch link: https://lore.kernel.org/all/20250821071721.3609109-1-pulehui@huaweicloud.com/
patch subject: [PATCH] tracing: Fix missing errno when zero parser->idx in trace_pid_write

in testcase: perf-sanity-tests
version: 
with following parameters:

	perf_compiler: gcc
	group: group-01


config: x86_64-rhel-9.4-bpf
compiler: gcc-12
test machine: 20 threads 1 sockets (Commet Lake) with 16G memory

(please refer to attached dmesg/kmsg for entire log/backtrace)



If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <oliver.sang@intel.com>
| Closes: https://lore.kernel.org/oe-lkp/202509022339.ae20a8bb-lkp@intel.com


The kernel config and materials to reproduce are available at:
https://download.01.org/0day-ci/archive/20250902/202509022339.ae20a8bb-lkp@intel.com


2025-08-31 21:12:04 sudo /usr/src/linux-perf-x86_64-rhel-9.4-bpf-cebdd2c9a622becc41349f32ace1795d750beda8/tools/perf/perf test 83 -v
 83: perf ftrace tests                                               : Running (1 active)
--- start ---
test child forked, pid 10325
perf ftrace list test
syscalls for sleep:
__ia32_sys_nanosleep
__x64_sys_nanosleep
__x64_sys_clock_nanosleep
__ia32_sys_clock_nanosleep
perf ftrace list test  [Success]
perf ftrace trace test
failed to reset ftrace
---- end(-1) ----
 83: perf ftrace tests                                               : FAILED!

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] tracing: Fix missing errno when zero parser->idx in trace_pid_write
  2025-08-21  7:17 [PATCH] tracing: Fix missing errno when zero parser->idx in trace_pid_write Pu Lehui
  2025-08-28  2:02 ` Pu Lehui
  2025-09-02 15:09 ` kernel test robot
@ 2025-09-03  4:15 ` Pu Lehui
  2 siblings, 0 replies; 5+ messages in thread
From: Pu Lehui @ 2025-09-03  4:15 UTC (permalink / raw)
  To: rostedt
  Cc: mhiramat, mathieu.desnoyers, Pu Lehui, linux-kernel,
	linux-trace-kernel


On 2025/8/21 15:17, Pu Lehui wrote:
> From: Pu Lehui <pulehui@huawei.com>
> 
> When trace_get_user in trace_pid_write parses an only space, the
> !trace_parser_loaded branch will break with no errno, causing
> tr->filtered_pids to still be assigned with pid_list, which may trigger
> potential problems.

Hi Steven,

Sorry, this patch will break the cleanup functionality of 
"set_ftrace_pid" as indicated in [0]. Pls ignore this patch.

Link: https://lore.kernel.org/all/202509022339.ae20a8bb-lkp@intel.com [0]

> 
> This patch will also silence the fault injection syzkaller warning in
> tracepoint_add_func [0]. We can reproduce the warning by following the
> steps below:
> 1. echo 8 >> set_event_notrace_pid. Let tr->filtered_pids owns one pid
>     and register sched_switch tracepoint.
> 2. echo ' ' >> set_event_pid, and perform fault injection during chunk
>     allocation of trace_pid_list_alloc. Let pid_list with no pid and
> assign to tr->filtered_pids.
> 3. echo ' ' >> set_event_pid. Let pid_list is NULL and assign to
>     tr->filtered_pids.
> 4. echo 9 >> set_event_pid, will trigger the double register
>     sched_switch tracepoint warning.

As for this fault injection syzkaller issue, shall we need to silence 
it? How about the below fix?

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index e6b50b416e63..c17c031e7917 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -834,7 +834,11 @@ int trace_pid_write(struct trace_pid_list 
*filtered_pids,
                 /* copy the current bits to the new max */
                 ret = trace_pid_list_first(filtered_pids, &pid);
                 while (!ret) {
-                       trace_pid_list_set(pid_list, pid);
+                       ret = trace_pid_list_set(pid_list, pid);
+                       if (ret) {
+                               trace_parser_put(&parser);
+                               return ret;
+                       }
                         ret = trace_pid_list_next(filtered_pids, pid + 
1, &pid);
                         nr_pids++;
                 }

> 
> Link: https://lore.kernel.org/all/67cb890e.050a0220.d8275.022e.GAE@google.com [0]
> Fixes: b27f266f74fb ("tracing: Fix return value of trace_pid_write()")
> Signed-off-by: Pu Lehui <pulehui@huawei.com>
> ---
>   kernel/trace/trace.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 8d8935ed416d..feeb7eb71318 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -853,10 +853,10 @@ int trace_pid_write(struct trace_pid_list *filtered_pids,
>   		ubuf += ret;
>   		cnt -= ret;
>   
> +		ret = -EINVAL;
>   		if (!trace_parser_loaded(&parser))
>   			break;
>   
> -		ret = -EINVAL;
>   		if (kstrtoul(parser.buffer, 0, &val))
>   			break;
>   

^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-09-03  4:15 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-21  7:17 [PATCH] tracing: Fix missing errno when zero parser->idx in trace_pid_write Pu Lehui
2025-08-28  2:02 ` Pu Lehui
2025-08-28  2:58   ` Steven Rostedt
2025-09-02 15:09 ` kernel test robot
2025-09-03  4:15 ` Pu Lehui

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).