All of lore.kernel.org
 help / color / mirror / Atom feed
* FAILED: patch "[PATCH] tracing: Map all PIDs to command lines" failed to apply to 4.9-stable tree
@ 2021-05-10  8:50 gregkh
  2021-05-10 16:04 ` Steven Rostedt
  0 siblings, 1 reply; 3+ messages in thread
From: gregkh @ 2021-05-10  8:50 UTC (permalink / raw)
  To: rostedt; +Cc: stable


The patch below does not apply to the 4.9-stable tree.
If someone wants it applied there, or to any other stable or longterm
tree, then please email the backport, including the original git commit
id to <stable@vger.kernel.org>.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From 785e3c0a3a870e72dc530856136ab4c8dd207128 Mon Sep 17 00:00:00 2001
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
Date: Tue, 27 Apr 2021 11:32:07 -0400
Subject: [PATCH] tracing: Map all PIDs to command lines

The default max PID is set by PID_MAX_DEFAULT, and the tracing
infrastructure uses this number to map PIDs to the comm names of the
tasks, such output of the trace can show names from the recorded PIDs in
the ring buffer. This mapping is also exported to user space via the
"saved_cmdlines" file in the tracefs directory.

But currently the mapping expects the PIDs to be less than
PID_MAX_DEFAULT, which is the default maximum and not the real maximum.
Recently, systemd will increases the maximum value of a PID on the system,
and when tasks are traced that have a PID higher than PID_MAX_DEFAULT, its
comm is not recorded. This leads to the entire trace to have "<...>" as
the comm name, which is pretty useless.

Instead, keep the array mapping the size of PID_MAX_DEFAULT, but instead
of just mapping the index to the comm, map a mask of the PID
(PID_MAX_DEFAULT - 1) to the comm, and find the full PID from the
map_cmdline_to_pid array (that already exists).

This bug goes back to the beginning of ftrace, but hasn't been an issue
until user space started increasing the maximum value of PIDs.

Link: https://lkml.kernel.org/r/20210427113207.3c601884@gandalf.local.home

Cc: stable@vger.kernel.org
Fixes: bc0c38d139ec7 ("ftrace: latency tracer infrastructure")
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 66a4ad93b5e9..e28d08905124 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -2390,14 +2390,13 @@ static void tracing_stop_tr(struct trace_array *tr)
 
 static int trace_save_cmdline(struct task_struct *tsk)
 {
-	unsigned pid, idx;
+	unsigned tpid, idx;
 
 	/* treat recording of idle task as a success */
 	if (!tsk->pid)
 		return 1;
 
-	if (unlikely(tsk->pid > PID_MAX_DEFAULT))
-		return 0;
+	tpid = tsk->pid & (PID_MAX_DEFAULT - 1);
 
 	/*
 	 * It's not the end of the world if we don't get
@@ -2408,26 +2407,15 @@ static int trace_save_cmdline(struct task_struct *tsk)
 	if (!arch_spin_trylock(&trace_cmdline_lock))
 		return 0;
 
-	idx = savedcmd->map_pid_to_cmdline[tsk->pid];
+	idx = savedcmd->map_pid_to_cmdline[tpid];
 	if (idx == NO_CMDLINE_MAP) {
 		idx = (savedcmd->cmdline_idx + 1) % savedcmd->cmdline_num;
 
-		/*
-		 * Check whether the cmdline buffer at idx has a pid
-		 * mapped. We are going to overwrite that entry so we
-		 * need to clear the map_pid_to_cmdline. Otherwise we
-		 * would read the new comm for the old pid.
-		 */
-		pid = savedcmd->map_cmdline_to_pid[idx];
-		if (pid != NO_CMDLINE_MAP)
-			savedcmd->map_pid_to_cmdline[pid] = NO_CMDLINE_MAP;
-
-		savedcmd->map_cmdline_to_pid[idx] = tsk->pid;
-		savedcmd->map_pid_to_cmdline[tsk->pid] = idx;
-
+		savedcmd->map_pid_to_cmdline[tpid] = idx;
 		savedcmd->cmdline_idx = idx;
 	}
 
+	savedcmd->map_cmdline_to_pid[idx] = tsk->pid;
 	set_cmdline(idx, tsk->comm);
 
 	arch_spin_unlock(&trace_cmdline_lock);
@@ -2438,6 +2426,7 @@ static int trace_save_cmdline(struct task_struct *tsk)
 static void __trace_find_cmdline(int pid, char comm[])
 {
 	unsigned map;
+	int tpid;
 
 	if (!pid) {
 		strcpy(comm, "<idle>");
@@ -2449,16 +2438,16 @@ static void __trace_find_cmdline(int pid, char comm[])
 		return;
 	}
 
-	if (pid > PID_MAX_DEFAULT) {
-		strcpy(comm, "<...>");
-		return;
+	tpid = pid & (PID_MAX_DEFAULT - 1);
+	map = savedcmd->map_pid_to_cmdline[tpid];
+	if (map != NO_CMDLINE_MAP) {
+		tpid = savedcmd->map_cmdline_to_pid[map];
+		if (tpid == pid) {
+			strlcpy(comm, get_saved_cmdlines(map), TASK_COMM_LEN);
+			return;
+		}
 	}
-
-	map = savedcmd->map_pid_to_cmdline[pid];
-	if (map != NO_CMDLINE_MAP)
-		strlcpy(comm, get_saved_cmdlines(map), TASK_COMM_LEN);
-	else
-		strcpy(comm, "<...>");
+	strcpy(comm, "<...>");
 }
 
 void trace_find_cmdline(int pid, char comm[])


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

* Re: FAILED: patch "[PATCH] tracing: Map all PIDs to command lines" failed to apply to 4.9-stable tree
  2021-05-10  8:50 FAILED: patch "[PATCH] tracing: Map all PIDs to command lines" failed to apply to 4.9-stable tree gregkh
@ 2021-05-10 16:04 ` Steven Rostedt
  2021-05-12  9:20   ` Greg KH
  0 siblings, 1 reply; 3+ messages in thread
From: Steven Rostedt @ 2021-05-10 16:04 UTC (permalink / raw)
  To: gregkh; +Cc: stable

On Mon, 10 May 2021 10:50:17 +0200
<gregkh@linuxfoundation.org> wrote:

> 
> The patch below does not apply to the 4.9-stable tree.
> If someone wants it applied there, or to any other stable or longterm
> tree, then please email the backport, including the original git commit
> id to <stable@vger.kernel.org>.
> 
> thanks,
> 

Looks like the following two commits also need to be backported, and then
this one should apply fine. The two commits do fix the code, so there
should be no issues backporting them:

  eaf260ac04d9b4cf9f458d5c97555bfff2da526e
  ("tracing: Treat recording comm for idle task as a success")


  e09e28671cda63e6308b31798b997639120e2a21
  ("tracing: Use strlcpy() instead of strcpy() in __trace_find_cmdline()")

After backporting the above two, I was able to apply and test this patch to
both 4.9 and 4.4.

-- Steve


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

* Re: FAILED: patch "[PATCH] tracing: Map all PIDs to command lines" failed to apply to 4.9-stable tree
  2021-05-10 16:04 ` Steven Rostedt
@ 2021-05-12  9:20   ` Greg KH
  0 siblings, 0 replies; 3+ messages in thread
From: Greg KH @ 2021-05-12  9:20 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: stable

On Mon, May 10, 2021 at 12:04:45PM -0400, Steven Rostedt wrote:
> On Mon, 10 May 2021 10:50:17 +0200
> <gregkh@linuxfoundation.org> wrote:
> 
> > 
> > The patch below does not apply to the 4.9-stable tree.
> > If someone wants it applied there, or to any other stable or longterm
> > tree, then please email the backport, including the original git commit
> > id to <stable@vger.kernel.org>.
> > 
> > thanks,
> > 
> 
> Looks like the following two commits also need to be backported, and then
> this one should apply fine. The two commits do fix the code, so there
> should be no issues backporting them:
> 
>   eaf260ac04d9b4cf9f458d5c97555bfff2da526e
>   ("tracing: Treat recording comm for idle task as a success")
> 
> 
>   e09e28671cda63e6308b31798b997639120e2a21
>   ("tracing: Use strlcpy() instead of strcpy() in __trace_find_cmdline()")
> 
> After backporting the above two, I was able to apply and test this patch to
> both 4.9 and 4.4.

That worked, thanks!

greg k-h

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

end of thread, other threads:[~2021-05-12  9:20 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-10  8:50 FAILED: patch "[PATCH] tracing: Map all PIDs to command lines" failed to apply to 4.9-stable tree gregkh
2021-05-10 16:04 ` Steven Rostedt
2021-05-12  9:20   ` Greg KH

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.