linux-trace-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] tracing: A couple of fixes for v6.18
@ 2025-10-11  3:51 Steven Rostedt
  2025-10-11  3:51 ` [PATCH 1/2] tracing: Fix tracing_mark_raw_write() to use buf and not ubuf Steven Rostedt
  2025-10-11  3:51 ` [PATCH 2/2] tracing: Stop fortify-string from warning in tracing_mark_raw_write() Steven Rostedt
  0 siblings, 2 replies; 5+ messages in thread
From: Steven Rostedt @ 2025-10-11  3:51 UTC (permalink / raw)
  To: linux-kernel, linux-trace-kernel
  Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton

[ Sorry for the dup, but I used the wrong script to send the other one.
  It hasn't gone through my tests to add the "for-linus" tag yet :-p ]

tracing fixes for v6.18:

- Fix tracing_mark_raw_write() to use per CPU buffer

  The fix to use the per CPU buffer to copy from user space was needed for
  both the trace_maker and trace_maker_raw file. The trace_maker file is
  used to write ASCII text into the trace buffer, but the trace_maker_raw is
  used to write binary structures directly into the ring buffer.

  The fix for reading from user space into per CPU buffers properly fixed
  the trace_marker write function, but the trace_marker_raw file wasn't
  fixed properly. The user space data was correctly written into the per CPU
  buffer, but the code that wrote into the ring buffer still used the user
  space pointer and not the per CPU buffer that had the user space data
  already written.

  There are several tests in the test suite to test the trace_marker file
  but it appears that there's no tests that test the trace_marker_raw file
  (this needs to be fixed), and this bug was missed.

- Stop the fortify string warning from writing into trace_marker_raw

  After converting the copy_from_user_nofault() into a memcpy(), another
  issue appeared. As writes to the trace_marker_raw expects binary data, the
  first entry is a 4 byte identifier. The entry structure is defined as:

  struct {
	struct trace_entry ent;
	int id;
	char dynamic_array[];
  };

  The size of this structure is reserved on the ring buffer and the pointer
  to the structure on the ring buffer is assigned to "entry". Then the data
  is copied via a memcpy() with:

  memcpy(&entry->id, buf, size);

  But the fortify string detects that the size is bigger than the size of
  the entry->id and produces a false positive warning.

  Hide the write from fortify string with:

  void *ptr = entry;
  ptr += offsetof(typeof(*entry), id);
  memcpy(ptr, buf, size);

  git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace.git
trace/urgent

Head SHA1: 649f416690a79861b646e304ccdee0465fec65b6


Steven Rostedt (2):
      tracing: Fix tracing_mark_raw_write() to use buf and not ubuf
      tracing: Stop fortify-string from warning in tracing_mark_raw_write()

----
 kernel/trace/trace.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

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

* [PATCH 1/2] tracing: Fix tracing_mark_raw_write() to use buf and not ubuf
  2025-10-11  3:51 [PATCH 0/2] tracing: A couple of fixes for v6.18 Steven Rostedt
@ 2025-10-11  3:51 ` Steven Rostedt
  2025-10-11  3:51 ` [PATCH 2/2] tracing: Stop fortify-string from warning in tracing_mark_raw_write() Steven Rostedt
  1 sibling, 0 replies; 5+ messages in thread
From: Steven Rostedt @ 2025-10-11  3:51 UTC (permalink / raw)
  To: linux-kernel, linux-trace-kernel
  Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
	stable, syzbot+9a2ede1643175f350105

From: Steven Rostedt <rostedt@goodmis.org>

The fix to use a per CPU buffer to read user space tested only the writes
to trace_marker. But it appears that the selftests are missing tests to
the trace_maker_raw file. The trace_maker_raw file is used by applications
that writes data structures and not strings into the file, and the tools
read the raw ring buffer to process the structures it writes.

The fix that reads the per CPU buffers passes the new per CPU buffer to
the trace_marker file writes, but the update to the trace_marker_raw write
read the data from user space into the per CPU buffer, but then still used
then passed the user space address to the function that records the data.

Pass in the per CPU buffer and not the user space address.

TODO: Add a test to better test trace_marker_raw.

Cc: stable@vger.kernel.org
Fixes: 64cf7d058a00 ("tracing: Have trace_marker use per-cpu data to read user space")
Reported-by: syzbot+9a2ede1643175f350105@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/all/68e973f5.050a0220.1186a4.0010.GAE@google.com/
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 kernel/trace/trace.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 0fd582651293..bbb89206a891 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -7497,12 +7497,12 @@ tracing_mark_raw_write(struct file *filp, const char __user *ubuf,
 	if (tr == &global_trace) {
 		guard(rcu)();
 		list_for_each_entry_rcu(tr, &marker_copies, marker_list) {
-			written = write_raw_marker_to_buffer(tr, ubuf, cnt);
+			written = write_raw_marker_to_buffer(tr, buf, cnt);
 			if (written < 0)
 				break;
 		}
 	} else {
-		written = write_raw_marker_to_buffer(tr, ubuf, cnt);
+		written = write_raw_marker_to_buffer(tr, buf, cnt);
 	}
 
 	return written;
-- 
2.51.0



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

* [PATCH 2/2] tracing: Stop fortify-string from warning in tracing_mark_raw_write()
  2025-10-11  3:51 [PATCH 0/2] tracing: A couple of fixes for v6.18 Steven Rostedt
  2025-10-11  3:51 ` [PATCH 1/2] tracing: Fix tracing_mark_raw_write() to use buf and not ubuf Steven Rostedt
@ 2025-10-11  3:51 ` Steven Rostedt
  2025-10-12  3:34   ` Masami Hiramatsu
  1 sibling, 1 reply; 5+ messages in thread
From: Steven Rostedt @ 2025-10-11  3:51 UTC (permalink / raw)
  To: linux-kernel, linux-trace-kernel
  Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
	stable, syzbot+9a2ede1643175f350105

From: Steven Rostedt <rostedt@goodmis.org>

The way tracing_mark_raw_write() records its data is that it has the
following structure:

  struct {
	struct trace_entry;
	int id;
	char dynamic_array[];
  };

But memcpy(&entry->id, buf, size) triggers the following warning when the
size is greater than the id:

 ------------[ cut here ]------------
 memcpy: detected field-spanning write (size 6) of single field "&entry->id" at kernel/trace/trace.c:7458 (size 4)
 WARNING: CPU: 7 PID: 995 at kernel/trace/trace.c:7458 write_raw_marker_to_buffer.isra.0+0x1f9/0x2e0
 Modules linked in:
 CPU: 7 UID: 0 PID: 995 Comm: bash Not tainted 6.17.0-test-00007-g60b82183e78a-dirty #211 PREEMPT(voluntary)
 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.17.0-debian-1.17.0-1 04/01/2014
 RIP: 0010:write_raw_marker_to_buffer.isra.0+0x1f9/0x2e0
 Code: 04 00 75 a7 b9 04 00 00 00 48 89 de 48 89 04 24 48 c7 c2 e0 b1 d1 b2 48 c7 c7 40 b2 d1 b2 c6 05 2d 88 6a 04 01 e8 f7 e8 bd ff <0f> 0b 48 8b 04 24 e9 76 ff ff ff 49 8d 7c 24 04 49 8d 5c 24 08 48
 RSP: 0018:ffff888104c3fc78 EFLAGS: 00010292
 RAX: 0000000000000000 RBX: 0000000000000006 RCX: 0000000000000000
 RDX: 0000000000000000 RSI: 1ffffffff6b363b4 RDI: 0000000000000001
 RBP: ffff888100058a00 R08: ffffffffb041d459 R09: ffffed1020987f40
 R10: 0000000000000007 R11: 0000000000000001 R12: ffff888100bb9010
 R13: 0000000000000000 R14: 00000000000003e3 R15: ffff888134800000
 FS:  00007fa61d286740(0000) GS:ffff888286cad000(0000) knlGS:0000000000000000
 CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
 CR2: 0000560d28d509f1 CR3: 00000001047a4006 CR4: 0000000000172ef0
 Call Trace:
  <TASK>
  tracing_mark_raw_write+0x1fe/0x290
  ? __pfx_tracing_mark_raw_write+0x10/0x10
  ? security_file_permission+0x50/0xf0
  ? rw_verify_area+0x6f/0x4b0
  vfs_write+0x1d8/0xdd0
  ? __pfx_vfs_write+0x10/0x10
  ? __pfx_css_rstat_updated+0x10/0x10
  ? count_memcg_events+0xd9/0x410
  ? fdget_pos+0x53/0x5e0
  ksys_write+0x182/0x200
  ? __pfx_ksys_write+0x10/0x10
  ? do_user_addr_fault+0x4af/0xa30
  do_syscall_64+0x63/0x350
  entry_SYSCALL_64_after_hwframe+0x76/0x7e
 RIP: 0033:0x7fa61d318687
 Code: 48 89 fa 4c 89 df e8 58 b3 00 00 8b 93 08 03 00 00 59 5e 48 83 f8 fc 74 1a 5b c3 0f 1f 84 00 00 00 00 00 48 8b 44 24 10 0f 05 <5b> c3 0f 1f 80 00 00 00 00 83 e2 39 83 fa 08 75 de e8 23 ff ff ff
 RSP: 002b:00007ffd87fe0120 EFLAGS: 00000202 ORIG_RAX: 0000000000000001
 RAX: ffffffffffffffda RBX: 00007fa61d286740 RCX: 00007fa61d318687
 RDX: 0000000000000006 RSI: 0000560d28d509f0 RDI: 0000000000000001
 RBP: 0000560d28d509f0 R08: 0000000000000000 R09: 0000000000000000
 R10: 0000000000000000 R11: 0000000000000202 R12: 0000000000000006
 R13: 00007fa61d4715c0 R14: 00007fa61d46ee80 R15: 0000000000000000
  </TASK>
 ---[ end trace 0000000000000000 ]---

This is because fortify string sees that the size of entry->id is only 4
bytes, but it is writing more than that. But this is OK as the
dynamic_array is allocated to handle that copy.

Use a void pointer and get the offset via offsetof() to keep fortify
string from warning about this copy.

Cc: stable@vger.kernel.org
Fixes: 64cf7d058a00 ("tracing: Have trace_marker use per-cpu data to read user space")
Reported-by: syzbot+9a2ede1643175f350105@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/all/68e973f5.050a0220.1186a4.0010.GAE@google.com/
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 kernel/trace/trace.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index bbb89206a891..27855fc9e0f2 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -7440,6 +7440,7 @@ static ssize_t write_raw_marker_to_buffer(struct trace_array *tr,
 	struct raw_data_entry *entry;
 	ssize_t written;
 	size_t size;
+	void *ptr;
 
 	size = sizeof(*entry) + cnt;
 
@@ -7455,7 +7456,10 @@ static ssize_t write_raw_marker_to_buffer(struct trace_array *tr,
 		return -EBADF;
 
 	entry = ring_buffer_event_data(event);
-	memcpy(&entry->id, buf, cnt);
+	/* Do not let fortify-string warn copying to &entry->id */
+	ptr = (void *)entry;
+	ptr += offsetof(typeof(*entry), id);
+	memcpy(ptr, buf, cnt);
 	written = cnt;
 
 	__buffer_unlock_commit(buffer, event);
-- 
2.51.0



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

* Re: [PATCH 2/2] tracing: Stop fortify-string from warning in tracing_mark_raw_write()
  2025-10-11  3:51 ` [PATCH 2/2] tracing: Stop fortify-string from warning in tracing_mark_raw_write() Steven Rostedt
@ 2025-10-12  3:34   ` Masami Hiramatsu
  2025-10-13 17:05     ` Steven Rostedt
  0 siblings, 1 reply; 5+ messages in thread
From: Masami Hiramatsu @ 2025-10-12  3:34 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: linux-kernel, linux-trace-kernel, Masami Hiramatsu, Mark Rutland,
	Mathieu Desnoyers, Andrew Morton, stable,
	syzbot+9a2ede1643175f350105

On Fri, 10 Oct 2025 23:51:43 -0400
Steven Rostedt <rostedt@kernel.org> wrote:

> From: Steven Rostedt <rostedt@goodmis.org>
> 
> The way tracing_mark_raw_write() records its data is that it has the
> following structure:
> 
>   struct {
> 	struct trace_entry;
> 	int id;
> 	char dynamic_array[];
>   };
> 
> But memcpy(&entry->id, buf, size) triggers the following warning when the
> size is greater than the id:
> 
>  ------------[ cut here ]------------
>  memcpy: detected field-spanning write (size 6) of single field "&entry->id" at kernel/trace/trace.c:7458 (size 4)
>  WARNING: CPU: 7 PID: 995 at kernel/trace/trace.c:7458 write_raw_marker_to_buffer.isra.0+0x1f9/0x2e0
>  Modules linked in:
>  CPU: 7 UID: 0 PID: 995 Comm: bash Not tainted 6.17.0-test-00007-g60b82183e78a-dirty #211 PREEMPT(voluntary)
>  Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.17.0-debian-1.17.0-1 04/01/2014
>  RIP: 0010:write_raw_marker_to_buffer.isra.0+0x1f9/0x2e0
>  Code: 04 00 75 a7 b9 04 00 00 00 48 89 de 48 89 04 24 48 c7 c2 e0 b1 d1 b2 48 c7 c7 40 b2 d1 b2 c6 05 2d 88 6a 04 01 e8 f7 e8 bd ff <0f> 0b 48 8b 04 24 e9 76 ff ff ff 49 8d 7c 24 04 49 8d 5c 24 08 48
>  RSP: 0018:ffff888104c3fc78 EFLAGS: 00010292
>  RAX: 0000000000000000 RBX: 0000000000000006 RCX: 0000000000000000
>  RDX: 0000000000000000 RSI: 1ffffffff6b363b4 RDI: 0000000000000001
>  RBP: ffff888100058a00 R08: ffffffffb041d459 R09: ffffed1020987f40
>  R10: 0000000000000007 R11: 0000000000000001 R12: ffff888100bb9010
>  R13: 0000000000000000 R14: 00000000000003e3 R15: ffff888134800000
>  FS:  00007fa61d286740(0000) GS:ffff888286cad000(0000) knlGS:0000000000000000
>  CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>  CR2: 0000560d28d509f1 CR3: 00000001047a4006 CR4: 0000000000172ef0
>  Call Trace:
>   <TASK>
>   tracing_mark_raw_write+0x1fe/0x290
>   ? __pfx_tracing_mark_raw_write+0x10/0x10
>   ? security_file_permission+0x50/0xf0
>   ? rw_verify_area+0x6f/0x4b0
>   vfs_write+0x1d8/0xdd0
>   ? __pfx_vfs_write+0x10/0x10
>   ? __pfx_css_rstat_updated+0x10/0x10
>   ? count_memcg_events+0xd9/0x410
>   ? fdget_pos+0x53/0x5e0
>   ksys_write+0x182/0x200
>   ? __pfx_ksys_write+0x10/0x10
>   ? do_user_addr_fault+0x4af/0xa30
>   do_syscall_64+0x63/0x350
>   entry_SYSCALL_64_after_hwframe+0x76/0x7e
>  RIP: 0033:0x7fa61d318687
>  Code: 48 89 fa 4c 89 df e8 58 b3 00 00 8b 93 08 03 00 00 59 5e 48 83 f8 fc 74 1a 5b c3 0f 1f 84 00 00 00 00 00 48 8b 44 24 10 0f 05 <5b> c3 0f 1f 80 00 00 00 00 83 e2 39 83 fa 08 75 de e8 23 ff ff ff
>  RSP: 002b:00007ffd87fe0120 EFLAGS: 00000202 ORIG_RAX: 0000000000000001
>  RAX: ffffffffffffffda RBX: 00007fa61d286740 RCX: 00007fa61d318687
>  RDX: 0000000000000006 RSI: 0000560d28d509f0 RDI: 0000000000000001
>  RBP: 0000560d28d509f0 R08: 0000000000000000 R09: 0000000000000000
>  R10: 0000000000000000 R11: 0000000000000202 R12: 0000000000000006
>  R13: 00007fa61d4715c0 R14: 00007fa61d46ee80 R15: 0000000000000000
>   </TASK>
>  ---[ end trace 0000000000000000 ]---
> 
> This is because fortify string sees that the size of entry->id is only 4
> bytes, but it is writing more than that. But this is OK as the
> dynamic_array is allocated to handle that copy.
> 
> Use a void pointer and get the offset via offsetof() to keep fortify
> string from warning about this copy.
> 
> Cc: stable@vger.kernel.org
> Fixes: 64cf7d058a00 ("tracing: Have trace_marker use per-cpu data to read user space")
> Reported-by: syzbot+9a2ede1643175f350105@syzkaller.appspotmail.com
> Closes: https://lore.kernel.org/all/68e973f5.050a0220.1186a4.0010.GAE@google.com/
> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>

Ah, fixed already.

Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>

> ---
>  kernel/trace/trace.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index bbb89206a891..27855fc9e0f2 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -7440,6 +7440,7 @@ static ssize_t write_raw_marker_to_buffer(struct trace_array *tr,
>  	struct raw_data_entry *entry;
>  	ssize_t written;
>  	size_t size;
> +	void *ptr;
>  
>  	size = sizeof(*entry) + cnt;
>  
> @@ -7455,7 +7456,10 @@ static ssize_t write_raw_marker_to_buffer(struct trace_array *tr,
>  		return -EBADF;
>  
>  	entry = ring_buffer_event_data(event);
> -	memcpy(&entry->id, buf, cnt);
> +	/* Do not let fortify-string warn copying to &entry->id */
> +	ptr = (void *)entry;
> +	ptr += offsetof(typeof(*entry), id);
> +	memcpy(ptr, buf, cnt);
>  	written = cnt;
>  
>  	__buffer_unlock_commit(buffer, event);
> -- 
> 2.51.0
> 
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

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

* Re: [PATCH 2/2] tracing: Stop fortify-string from warning in tracing_mark_raw_write()
  2025-10-12  3:34   ` Masami Hiramatsu
@ 2025-10-13 17:05     ` Steven Rostedt
  0 siblings, 0 replies; 5+ messages in thread
From: Steven Rostedt @ 2025-10-13 17:05 UTC (permalink / raw)
  To: Masami Hiramatsu (Google)
  Cc: Steven Rostedt, linux-kernel, linux-trace-kernel, Mark Rutland,
	Mathieu Desnoyers, Andrew Morton, stable,
	syzbot+9a2ede1643175f350105

On Sun, 12 Oct 2025 12:34:08 +0900
Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:

> > Use a void pointer and get the offset via offsetof() to keep fortify
> > string from warning about this copy.
> > 
> > Cc: stable@vger.kernel.org
> > Fixes: 64cf7d058a00 ("tracing: Have trace_marker use per-cpu data to read user space")
> > Reported-by: syzbot+9a2ede1643175f350105@syzkaller.appspotmail.com
> > Closes: https://lore.kernel.org/all/68e973f5.050a0220.1186a4.0010.GAE@google.com/
> > Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>  
> 
> Ah, fixed already.
> 
> Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Actually this was the wrong fix. Before submitting a pull request, I looked
to see if there was a better way to handle this and discovered "unsafe_memcpy()"
which is the proper solution:

  https://lore.kernel.org/all/20251011112032.77be18e4@gandalf.local.home/

-- Steve

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

end of thread, other threads:[~2025-10-13 17:05 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-11  3:51 [PATCH 0/2] tracing: A couple of fixes for v6.18 Steven Rostedt
2025-10-11  3:51 ` [PATCH 1/2] tracing: Fix tracing_mark_raw_write() to use buf and not ubuf Steven Rostedt
2025-10-11  3:51 ` [PATCH 2/2] tracing: Stop fortify-string from warning in tracing_mark_raw_write() Steven Rostedt
2025-10-12  3:34   ` Masami Hiramatsu
2025-10-13 17:05     ` Steven Rostedt

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).