* [PATCH] ftrace: fix task's invalid comm of <...> when big pid
@ 2018-03-28 12:32 Wang Yu
2018-03-28 15:35 ` Steven Rostedt
0 siblings, 1 reply; 6+ messages in thread
From: Wang Yu @ 2018-03-28 12:32 UTC (permalink / raw)
To: Steven Rostedt, Ingo Molnar; +Cc: linux-kernel, Wang Yu
when pid is bigger than PID_MAX_DEFAULT, the comm of task
is <...>, it is better use pid_max to compare
Signed-off-by: Wang Yu <yuwang@linux.alibaba.com>
---
kernel/trace/trace.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
mode change 100644 => 100755 kernel/trace/trace.c
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
old mode 100644
new mode 100755
index 20a2300..0d4bc7a
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -1976,7 +1976,7 @@ static void __trace_find_cmdline(int pid, char comm[])
return;
}
- if (pid > PID_MAX_DEFAULT) {
+ if (pid > pid_max) {
strcpy(comm, "<...>");
return;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] ftrace: fix task's invalid comm of <...> when big pid
2018-03-28 12:32 [PATCH] ftrace: fix task's invalid comm of <...> when big pid Wang Yu
@ 2018-03-28 15:35 ` Steven Rostedt
2018-03-28 15:44 ` Steven Rostedt
0 siblings, 1 reply; 6+ messages in thread
From: Steven Rostedt @ 2018-03-28 15:35 UTC (permalink / raw)
To: Wang Yu; +Cc: Ingo Molnar, linux-kernel
On Wed, 28 Mar 2018 20:32:27 +0800
Wang Yu <yuwang@linux.alibaba.com> wrote:
> when pid is bigger than PID_MAX_DEFAULT, the comm of task
> is <...>, it is better use pid_max to compare
>
> Signed-off-by: Wang Yu <yuwang@linux.alibaba.com>
> ---
> kernel/trace/trace.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> mode change 100644 => 100755 kernel/trace/trace.c
>
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> old mode 100644
> new mode 100755
> index 20a2300..0d4bc7a
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -1976,7 +1976,7 @@ static void __trace_find_cmdline(int pid, char comm[])
> return;
> }
>
> - if (pid > PID_MAX_DEFAULT) {
> + if (pid > pid_max) {
Thanks! this probably should go to stable.
-- Steve
> strcpy(comm, "<...>");
> return;
> }
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ftrace: fix task's invalid comm of <...> when big pid
2018-03-28 15:35 ` Steven Rostedt
@ 2018-03-28 15:44 ` Steven Rostedt
2018-03-29 2:16 ` Wang Yu
0 siblings, 1 reply; 6+ messages in thread
From: Steven Rostedt @ 2018-03-28 15:44 UTC (permalink / raw)
To: Wang Yu; +Cc: Ingo Molnar, linux-kernel
On Wed, 28 Mar 2018 11:35:22 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:
> On Wed, 28 Mar 2018 20:32:27 +0800
> Wang Yu <yuwang@linux.alibaba.com> wrote:
>
> > when pid is bigger than PID_MAX_DEFAULT, the comm of task
> > is <...>, it is better use pid_max to compare
> >
> > Signed-off-by: Wang Yu <yuwang@linux.alibaba.com>
> > ---
> > kernel/trace/trace.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> > mode change 100644 => 100755 kernel/trace/trace.c
> >
> > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> > old mode 100644
> > new mode 100755
> > index 20a2300..0d4bc7a
> > --- a/kernel/trace/trace.c
> > +++ b/kernel/trace/trace.c
> > @@ -1976,7 +1976,7 @@ static void __trace_find_cmdline(int pid, char comm[])
> > return;
> > }
> >
> > - if (pid > PID_MAX_DEFAULT) {
> > + if (pid > pid_max) {
>
> Thanks! this probably should go to stable.
I take that back. This patch can cause a buffer overflow access.
If you looked at the line after this check, you would see:
if (pid > PID_MAX_DEFAULT) {
strcpy(comm, "<...>");
return;
}
map = savedcmd->map_pid_to_cmdline[pid];
And if you looked to see what map_pid_to_cmdline is:
struct saved_cmdlines_buffer {
unsigned map_pid_to_cmdline[PID_MAX_DEFAULT+1];
Your patch will access memory past the end of that array, and cause a
bug.
If you want to support more than PID_MAX_DEFAULT, a lot more needs to
change than this. And a change like this isn't going to go to stable.
What you can do is make that map_pid_to_cmdline array bigger.
-- Steve
>
> -- Steve
>
> > strcpy(comm, "<...>");
> > return;
> > }
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ftrace: fix task's invalid comm of <...> when big pid
2018-03-28 15:44 ` Steven Rostedt
@ 2018-03-29 2:16 ` Wang Yu
2018-03-29 14:26 ` Steven Rostedt
0 siblings, 1 reply; 6+ messages in thread
From: Wang Yu @ 2018-03-29 2:16 UTC (permalink / raw)
To: Steven Rostedt; +Cc: Ingo Molnar, linux-kernel
于 18/3/28 下午11:44, Steven Rostedt 写道:
> On Wed, 28 Mar 2018 11:35:22 -0400
> Steven Rostedt <rostedt@goodmis.org> wrote:
>
>> On Wed, 28 Mar 2018 20:32:27 +0800
>> Wang Yu <yuwang@linux.alibaba.com> wrote:
>>
>>> when pid is bigger than PID_MAX_DEFAULT, the comm of task
>>> is <...>, it is better use pid_max to compare
>>>
>>> Signed-off-by: Wang Yu <yuwang@linux.alibaba.com>
>>> ---
>>> kernel/trace/trace.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>> mode change 100644 => 100755 kernel/trace/trace.c
>>>
>>> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
>>> old mode 100644
>>> new mode 100755
>>> index 20a2300..0d4bc7a
>>> --- a/kernel/trace/trace.c
>>> +++ b/kernel/trace/trace.c
>>> @@ -1976,7 +1976,7 @@ static void __trace_find_cmdline(int pid, char comm[])
>>> return;
>>> }
>>>
>>> - if (pid > PID_MAX_DEFAULT) {
>>> + if (pid > pid_max) {
>> Thanks! this probably should go to stable.
> I take that back. This patch can cause a buffer overflow access.
>
> If you looked at the line after this check, you would see:
>
> if (pid > PID_MAX_DEFAULT) {
> strcpy(comm, "<...>");
> return;
> }
>
> map = savedcmd->map_pid_to_cmdline[pid];
>
> And if you looked to see what map_pid_to_cmdline is:
>
> struct saved_cmdlines_buffer {
> unsigned map_pid_to_cmdline[PID_MAX_DEFAULT+1];
>
> Your patch will access memory past the end of that array, and cause a
> bug.
>
> If you want to support more than PID_MAX_DEFAULT, a lot more needs to
> change than this. And a change like this isn't going to go to stable.
>
> What you can do is make that map_pid_to_cmdline array bigger.
>
> -- Steve
I am sorry about it, and as the number of cpu cores increases, the current
PID_MAX_DEFAULT is too small, our online machines set the pid_max 65536 as default, so the task
pid number bigger than PID_MAX_DEFAULT can't show the real comm (only <...>), so i want to
ajust the PID_MAX_DEFAULT upto 4x, and what do you think?
* This controls the default maximum pid allocated to a process
*/
-#define PID_MAX_DEFAULT (CONFIG_BASE_SMALL ? 0x1000 : 0x8000)
+#define PID_MAX_DEFAULT (CONFIG_BASE_SMALL ? 0x1000 : 0x20000)
>
>
>> -- Steve
>>
>>> strcpy(comm, "<...>");
>>> return;
>>> }
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ftrace: fix task's invalid comm of <...> when big pid
2018-03-29 2:16 ` Wang Yu
@ 2018-03-29 14:26 ` Steven Rostedt
2018-03-30 8:21 ` Wang Yu
0 siblings, 1 reply; 6+ messages in thread
From: Steven Rostedt @ 2018-03-29 14:26 UTC (permalink / raw)
To: Wang Yu; +Cc: Ingo Molnar, linux-kernel
On Thu, 29 Mar 2018 10:16:22 +0800
Wang Yu <yuwang@linux.alibaba.com> wrote:
> > What you can do is make that map_pid_to_cmdline array bigger.
> >
> > -- Steve
>
> I am sorry about it, and as the number of cpu cores increases, the current
>
> PID_MAX_DEFAULT is too small, our online machines set the pid_max 65536 as default, so the task
> pid number bigger than PID_MAX_DEFAULT can't show the real comm (only <...>), so i want to
> ajust the PID_MAX_DEFAULT upto 4x, and what do you think?
>
> * This controls the default maximum pid allocated to a process
> */
> -#define PID_MAX_DEFAULT (CONFIG_BASE_SMALL ? 0x1000 : 0x8000)
> +#define PID_MAX_DEFAULT (CONFIG_BASE_SMALL ? 0x1000 : 0x20000)
What I was thinking is to make the map_pid_to_cmdline array dynamic
(not static), and be set to pid_max (after pid_max is determined).
Now, pid_max can be changed at run time. Thus, the tracing code will
need to keep a separate variable for that array to store the length. It
can not rely on pid_max. But if a pid that is greater than pid_max is
found, we could kick off a work thread to increase the array.
-- Steve
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ftrace: fix task's invalid comm of <...> when big pid
2018-03-29 14:26 ` Steven Rostedt
@ 2018-03-30 8:21 ` Wang Yu
0 siblings, 0 replies; 6+ messages in thread
From: Wang Yu @ 2018-03-30 8:21 UTC (permalink / raw)
To: Steven Rostedt; +Cc: Ingo Molnar, linux-kernel
在 18/3/29 下午10:26, Steven Rostedt 写道:
> On Thu, 29 Mar 2018 10:16:22 +0800
> Wang Yu <yuwang@linux.alibaba.com> wrote:
>
>>> What you can do is make that map_pid_to_cmdline array bigger.
>>>
>>> -- Steve
>> I am sorry about it, and as the number of cpu cores increases, the current
>>
>> PID_MAX_DEFAULT is too small, our online machines set the pid_max 65536 as default, so the task
>> pid number bigger than PID_MAX_DEFAULT can't show the real comm (only <...>), so i want to
>> ajust the PID_MAX_DEFAULT upto 4x, and what do you think?
>>
>> * This controls the default maximum pid allocated to a process
>> */
>> -#define PID_MAX_DEFAULT (CONFIG_BASE_SMALL ? 0x1000 : 0x8000)
>> +#define PID_MAX_DEFAULT (CONFIG_BASE_SMALL ? 0x1000 : 0x20000)
> What I was thinking is to make the map_pid_to_cmdline array dynamic
> (not static), and be set to pid_max (after pid_max is determined).
>
> Now, pid_max can be changed at run time. Thus, the tracing code will
> need to keep a separate variable for that array to store the length. It
> can not rely on pid_max. But if a pid that is greater than pid_max is
> found, we could kick off a work thread to increase the array.
thanks for your reply, thanks, if map_pid_to_cmdline need dynamic,
saved_cmdlines_buffer
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-03-30 8:21 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-28 12:32 [PATCH] ftrace: fix task's invalid comm of <...> when big pid Wang Yu
2018-03-28 15:35 ` Steven Rostedt
2018-03-28 15:44 ` Steven Rostedt
2018-03-29 2:16 ` Wang Yu
2018-03-29 14:26 ` Steven Rostedt
2018-03-30 8:21 ` Wang Yu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.