linux-kernel - Re: [PATCH] tracing: fix memcpy size when copying stack entries

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20230712102621.4c588de9@gandalf.local.home>
Date:   Wed, 12 Jul 2023 10:26:21 -0400
From:   Steven Rostedt <rostedt@...dmis.org>
To:     Sven Schnelle <svens@...ux.ibm.com>
Cc:     linux-kernel@...r.kernel.org
Subject: Re: [PATCH] tracing: fix memcpy size when copying stack entries

On Wed, 12 Jul 2023 10:14:34 -0400
Steven Rostedt <rostedt@...dmis.org> wrote:

> On Wed, 12 Jul 2023 16:06:27 +0200
> Sven Schnelle <svens@...ux.ibm.com> wrote:
> 
> > > No, still getting the same warning:
> > >
> > > [    2.302776] memcpy: detected field-spanning write (size 104) of single field "stack" at kernel/trace/trace.c:3178 (size 64)    
> > 
> > BTW, i'm seeing the same error on x86 with current master when
> > CONFIG_FORTIFY_SOURCE=y and CONFIG_SCHED_TRACER=y:  
> 
> As I don't know how the fortifier works, nor what exactly it is checking,
> do you have any idea on how to quiet it?
> 
> This is a false positive, as I described before.


Hmm, maybe this would work?

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 4529e264cb86..20122eeccf97 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -3118,6 +3118,7 @@ static void __ftrace_trace_stack(struct trace_buffer *buffer,
 	struct ftrace_stack *fstack;
 	struct stack_entry *entry;
 	int stackidx;
+	void *ptr;
 
 	/*
 	 * Add one, for this function and the call to save_stack_trace()
@@ -3161,9 +3162,25 @@ static void __ftrace_trace_stack(struct trace_buffer *buffer,
 				    trace_ctx);
 	if (!event)
 		goto out;
-	entry = ring_buffer_event_data(event);
+	ptr = ring_buffer_event_data(event);
+	entry = ptr;
+
+	/*
+	 * For backward compatibility reasons, the entry->caller is an
+	 * array of 8 slots to store the stack. This is also exported
+	 * to user space. The amount allocated on the ring buffer actually
+	 * holds enough for the stack specified by nr_entries. This will
+	 * go into the location of entry->caller. Due to string fortifiers
+	 * checking the size of the destination of memcpy() it triggers
+	 * when it detects that size is greater than 8. To hide this from
+	 * the fortifiers, we use "ptr" and pointer arithmetic to assign caller.
+	 *
+	 * The below is really just:
+	 *   memcpy(&entry->caller, fstack->calls, size);
+	 */
+	ptr += offsetof(typeof(*entry), caller);
+	memcpy(ptr, fstack->calls, size);
 
-	memcpy(&entry->caller, fstack->calls, size);
 	entry->size = nr_entries;
 
 	if (!call_filter_check_discard(call, entry, buffer, event))


-- Steve