From nobody Sat Oct 4 00:32:14 2025 Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id DFCF726B756 for ; Thu, 21 Aug 2025 15:07:27 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=216.40.44.10 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1755788850; cv=none; b=Ed1e4jzO4dAZ1XB8LzyTsNozNF9EvfLsehQi2ZrHc05djbpYYTJIci6oL1rKBnj2T98ltPRKurLvBET9XRoEL8jLf+PHLX2ftamHOX3VDzi7BilQ4ThlUVzarF6r0HwvzXXyMHd5flSVi7Ojeg+s/IGhym+aPatRWIlk817Rz6Y= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1755788850; c=relaxed/simple; bh=wLCQfXPnhvLkYuBg8hqLLP00b/P1oOwsU3cSsSqJQ1A=; h=Date:From:To:Cc:Subject:Message-ID:MIME-Version:Content-Type; b=hXn8g+eU4Xbvhj/3C0B4q0sHEnRP3p8Ek0DdMoUPC3mC6soiAw6MYxkA8PZzxtDW6kUOgMbKRBmd9WnQA7/QFUdf7xlczzhh6lIJ1bMYEXRIyzuT3sSmY1wvyinmA6IHP+00dvObi8Zt4AmiT3gnlgkCOKVPNxHJjF2Rxaxq/DM= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=goodmis.org; spf=pass smtp.mailfrom=goodmis.org; arc=none smtp.client-ip=216.40.44.10 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=goodmis.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=goodmis.org Received: from omf09.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id C3159B9730; Thu, 21 Aug 2025 15:07:20 +0000 (UTC) Received: from [HIDDEN] (Authenticated sender: rostedt@goodmis.org) by omf09.hostedemail.com (Postfix) with ESMTPA id 06A4B20028; Thu, 21 Aug 2025 15:07:18 +0000 (UTC) Date: Thu, 21 Aug 2025 11:07:23 -0400 From: Steven Rostedt To: LKML Cc: Masami Hiramatsu , Mathieu Desnoyers , Mark Rutland , Sasha Levin Subject: [for-linus][PATCH] fgraph: Copy args in intermediate storage with entry Message-ID: <20250821110723.4395212a@gandalf.local.home> X-Mailer: Claws Mail 3.20.0git84 (GTK+ 2.24.33; x86_64-pc-linux-gnu) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspamout04 X-Rspamd-Queue-Id: 06A4B20028 X-Stat-Signature: qa14phsgfncffo8hfz7d5f3xyyzqi6ge X-Session-Marker: 726F737465647440676F6F646D69732E6F7267 X-Session-ID: U2FsdGVkX1/u3cbP8boSLiSmeOy3ubL+9Hfz1EFcOjU= X-HE-Tag: 1755788838-548787 X-HE-Meta: U2FsdGVkX1+r7GON3oHQhMXsQgqYxuaDgQ5sZ0ZVYjmTM7OV2ZIYbre+wkjc6dQyjAejQPwEMS2noSKzwGpJD8SkEpLwIa5zyu5JtdO2xpLtelAr2FjBleUaJHYTYDRJHdIftbD4I5HQfPHQGp5qrmdlc2Mmrsuc3iWhGAWaPcACA/vW6IwcDZm/rgDWIc2gSqArrrO4E5/qrTiquNc4k28FJf6bG04DoCZnKUlk2EKd4JLBIyzqBqOUHcdtbPYWm0C3EAEFiwAFusC/Bp6EFC+TtY0ehRrj7tunb7bC81vFUlsjwNhw0nzWKF8AUr+GAgVbeMJXGH9IzbnF2ykUidMDxKy44vIYcTB8L7fjnA6UVIm/WGXRmlNGxdu5SssN5wsIAwcyqX5oXfjKCDpf8aaDrhrf73rJo6yprudKdetWJhywMffadXarcmPp1eqvlojK4AU12pIDLTXqBgI8ZE1+D92x74XpqE+Je6cXMNgp9EK7thnx3a+avVEpYcZ57cnEO3+lnWM= Content-Type: text/plain; charset="utf-8" git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace.git trace/fixes Head SHA1: b7b6a20aa4ab811f598793210b4ea62885b40e18 Steven Rostedt (1): fgraph: Copy args in intermediate storage with entry ---- kernel/trace/trace_functions_graph.c | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) --------------------------- commit b7b6a20aa4ab811f598793210b4ea62885b40e18 Author: Steven Rostedt Date: Wed Aug 20 19:55:22 2025 -0400 fgraph: Copy args in intermediate storage with entry =20 The output of the function graph tracer has two ways to display its entries. One way for leaf functions with no events recorded within them, and the other is for functions with events recorded inside it. As funct= ion graph has an entry and exit event, to simplify the output of leaf functions it combines the two, where as non leaf functions are separate: =20 2) | invoke_rcu_core() { 2) | raise_softirq() { 2) 0.391 us | __raise_softirq_irqoff(); 2) 1.191 us | } 2) 2.086 us | } =20 The __raise_softirq_irqoff() function above is really two events that w= ere merged into one. Otherwise it would have looked like: =20 2) | invoke_rcu_core() { 2) | raise_softirq() { 2) | __raise_softirq_irqoff() { 2) 0.391 us | } 2) 1.191 us | } 2) 2.086 us | } =20 In order to do this merge, the reading of the trace output file needs to look at the next event before printing. But since the pointer to the ev= ent is on the ring buffer, it needs to save the entry event before it looks= at the next event as the next event goes out of focus as soon as a new eve= nt is read from the ring buffer. After it reads the next event, it will pr= int the entry event with either the '{' (non leaf) or ';' and timestamps (l= eaf). =20 The iterator used to read the trace file has storage for this event. The problem happens when the function graph tracer has arguments attached to the entry event as the entry now has a variable length "args" field. Th= is field only gets set when funcargs option is used. But the args are not recorded in this temp data and garbage could be printed. The entry field is copied via: =20 data->ent =3D *curr; =20 Where "curr" is the entry field. But this method only saves the non variable length fields from the structure. =20 Add a helper structure to the iterator data that adds the max args size= to the data storage in the iterator. Then simply copy the entire entry into this storage (with size protection). =20 Cc: Masami Hiramatsu Cc: Mathieu Desnoyers Cc: Mark Rutland Link: https://lore.kernel.org/20250820195522.51d4a268@gandalf.local.home Reported-by: Sasha Levin Closes: https://lore.kernel.org/all/aJaxRVKverIjF4a6@lappy/ Fixes: ff5c9c576e75 ("ftrace: Add support for function argument to grap= h tracer") Signed-off-by: Steven Rostedt (Google) diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_func= tions_graph.c index 66e1a527cf1a..a7f4b9a47a71 100644 --- a/kernel/trace/trace_functions_graph.c +++ b/kernel/trace/trace_functions_graph.c @@ -27,14 +27,21 @@ struct fgraph_cpu_data { unsigned long enter_funcs[FTRACE_RETFUNC_DEPTH]; }; =20 +struct fgraph_ent_args { + struct ftrace_graph_ent_entry ent; + /* Force the sizeof of args[] to have FTRACE_REGS_MAX_ARGS entries */ + unsigned long args[FTRACE_REGS_MAX_ARGS]; +}; + struct fgraph_data { struct fgraph_cpu_data __percpu *cpu_data; =20 /* Place to preserve last processed entry. */ union { - struct ftrace_graph_ent_entry ent; + struct fgraph_ent_args ent; + /* TODO allow retaddr to have args */ struct fgraph_retaddr_ent_entry rent; - } ent; + }; struct ftrace_graph_ret_entry ret; int failed; int cpu; @@ -627,10 +634,13 @@ get_return_for_leaf(struct trace_iterator *iter, * Save current and next entries for later reference * if the output fails. */ - if (unlikely(curr->ent.type =3D=3D TRACE_GRAPH_RETADDR_ENT)) - data->ent.rent =3D *(struct fgraph_retaddr_ent_entry *)curr; - else - data->ent.ent =3D *curr; + if (unlikely(curr->ent.type =3D=3D TRACE_GRAPH_RETADDR_ENT)) { + data->rent =3D *(struct fgraph_retaddr_ent_entry *)curr; + } else { + int size =3D min((int)sizeof(data->ent), (int)iter->ent_size); + + memcpy(&data->ent, curr, size); + } /* * If the next event is not a return type, then * we only care about what type it is. Otherwise we can