[PATCH] tracing: Fix missing errno when zero parser->idx in trace_pid_write

[PATCH] tracing: Fix missing errno when zero parser->idx in trace_pid_write

[Pu Lehui]

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

Re: [PATCH] tracing: Fix missing errno when zero parser->idx in trace_pid_write

[Pu Lehui]

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;
>   

Re: [PATCH] tracing: Fix missing errno when zero parser->idx in trace_pid_write

[Steven Rostedt]

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

Re: [PATCH] tracing: Fix missing errno when zero parser->idx in trace_pid_write

[kernel test robot]

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

Re: [PATCH] tracing: Fix missing errno when zero parser->idx in trace_pid_write

[Steven Rostedt]

On Tue, 2 Sep 2025 23:09:53 +0800
kernel test robot <oliver.sang@intel.com> wrote:

> 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

This is why I hate LKP. It's huge, complex, and I never can get it working.

I did the following:

 # lkp split-job jobs/perf-sanity-tests.yaml
 # lkp install -f ./perf-sanity-tests-defaults.yaml

[ had to fix python up a little because it wanted python3.9 where I had 3.13 ]

 # lkp run ./perf-sanity-tests-defaults.yaml
/work/git/lkp-tests.git/programs/perf-sanity-tests/run: 7: .: cannot open /work/git/lkp-tests.git/lib/tests/perf_test.sh: No such file
kill 37975 vmstat --timestamp -n 10 
kill 37973 dmesg --follow --decode 
wait for background processes: 37978 meminfo


I have no idea where this "perf_test.sh" is supposed to come from, and why
it doesn't exist :-p


-- Steve

Re: [PATCH] tracing: Fix missing errno when zero parser->idx in trace_pid_write

[Oliver Sang]

hi, Steve,

On Fri, Sep 05, 2025 at 03:03:12PM -0400, Steven Rostedt wrote:
> On Tue, 2 Sep 2025 23:09:53 +0800
> kernel test robot <oliver.sang@intel.com> wrote:
> 
> > 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
> 
> This is why I hate LKP. It's huge, complex, and I never can get it working.
> 
> I did the following:
> 
>  # lkp split-job jobs/perf-sanity-tests.yaml
>  # lkp install -f ./perf-sanity-tests-defaults.yaml
> 
> [ had to fix python up a little because it wanted python3.9 where I had 3.13 ]
> 
>  # lkp run ./perf-sanity-tests-defaults.yaml
> /work/git/lkp-tests.git/programs/perf-sanity-tests/run: 7: .: cannot open /work/git/lkp-tests.git/lib/tests/perf_test.sh: No such file
> kill 37975 vmstat --timestamp -n 10 
> kill 37973 dmesg --follow --decode 
> wait for background processes: 37978 meminfo
> 
> 
> I have no idea where this "perf_test.sh" is supposed to come from, and why
> it doesn't exist :-p

sorry for inconvenience. for this "perf_test.sh" issue, we pushed a fix
https://github.com/intel/lkp-tests/commit/3ab6496813796e537d277242d220c4571790a363

however, we found there are other issues which block reproducer to work out of
our cluster env that we can not resolve shortly.

but if you have any debug/fix patch, we'll be very glad to test/verify.

> 
> 
> -- Steve

Re: [PATCH] tracing: Fix missing errno when zero parser->idx in trace_pid_write

[Steven Rostedt]

On Thu, 11 Sep 2025 16:23:35 +0800
Oliver Sang <oliver.sang@intel.com> wrote:

> > I have no idea where this "perf_test.sh" is supposed to come from, and why
> > it doesn't exist :-p  
> 
> sorry for inconvenience. for this "perf_test.sh" issue, we pushed a fix
> https://github.com/intel/lkp-tests/commit/3ab6496813796e537d277242d220c4571790a363
> 
> however, we found there are other issues which block reproducer to work out of
> our cluster env that we can not resolve shortly.
> 
> but if you have any debug/fix patch, we'll be very glad to test/verify.

Nope, I just gave up on trying to make it work.

-- Steve

Re: [PATCH] tracing: Fix missing errno when zero parser->idx in trace_pid_write

[Pu Lehui]

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;
>   

Re: [PATCH] tracing: Fix missing errno when zero parser->idx in trace_pid_write

[Steven Rostedt]

On Wed, 3 Sep 2025 12:15:16 +0800
Pu Lehui <pulehui@huawei.com> wrote:

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

I usually don't care about fault injections because it causes "faults" that
would only happen if the system was about to crash. But anyway..

> 
> 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;
> +                       }

make it:

			if (ret < 0)
				goto out;

>                          ret = trace_pid_list_next(filtered_pids, pid + 
> 1, &pid);
>                          nr_pids++;
>                  }

And put the out label just before the trace_parser_put().

Oh, and add one space before the "out:" label. That makes diffs of patches
show the function when changes are after the label and not the label itself.

-- Steve

Re: [PATCH] tracing: Fix missing errno when zero parser->idx in trace_pid_write

[Pu Lehui]

On 2025/9/6 3:39, Steven Rostedt wrote:
> On Wed, 3 Sep 2025 12:15:16 +0800
> Pu Lehui <pulehui@huawei.com> wrote:
> 
>> As for this fault injection syzkaller issue, shall we need to silence
>> it? How about the below fix?
> 
> I usually don't care about fault injections because it causes "faults" that
> would only happen if the system was about to crash. But anyway..

agree!

> 
>>
>> 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;
>> +                       }
> 
> make it:
> 
> 			if (ret < 0)
> 				goto out;
> 
>>                           ret = trace_pid_list_next(filtered_pids, pid +
>> 1, &pid);
>>                           nr_pids++;
>>                   }
> 
> And put the out label just before the trace_parser_put().
> 
> Oh, and add one space before the "out:" label. That makes diffs of patches
> show the function when changes are after the label and not the label itself.

Looks like an interesting ops. Will do

> 
> -- Steve
>