From: Sven Schnelle <svens@linux.ibm.com>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: linux-kernel@vger.kernel.org, keescook@chromium.org
Subject: Re: [PATCH] tracing: fix memcpy size when copying stack entries
Date: Wed, 12 Jul 2023 16:31:15 +0200 [thread overview]
Message-ID: <yt9d1qhdfbgs.fsf@linux.ibm.com> (raw)
In-Reply-To: <20230712101434.4613b3ec@gandalf.local.home> (Steven Rostedt's message of "Wed, 12 Jul 2023 10:14:34 -0400")
Hi Steven,
Steven Rostedt <rostedt@goodmis.org> writes:
> On Wed, 12 Jul 2023 16:06:27 +0200
> Sven Schnelle <svens@linux.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.
The "problem" is that struct stack_entry is
struct stack_entry {
int size;
unsigned long caller[8];
};
So, as you explained, the ringbuffer code allocates some space after the
struct for additional entries:
struct stack_entry 1;
<additional space for 1>
struct stack_entry 2;
<additional space for 2>
...
But the struct member that is passed to memcpy still has the type
information 'caller is an array with 8 members of 8 bytes', so memcpy
fortify complains. I'm not sure whether we can blame the compiler or
the fortify code here.
One (ugly and whitespace damaged) workaround is:
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 35b11f5a9519..31acd8a6b97e 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -3170,7 +3170,8 @@ static void __ftrace_trace_stack(struct trace_buffer *buffer,
goto out;
entry = ring_buffer_event_data(event);
- memcpy(&entry->caller, fstack->calls, size);
+ void *p = entry + offsetof(struct stack_entry, caller);
+ memcpy(p, fstack->calls, size);
entry->size = nr_entries;
if (!call_filter_check_discard(call, entry, buffer, event))
So with that offsetof calculation the compiler doesn't know about the 8
entries * 8 bytes limitation. Adding Kees to the thread, maybe he knows
some way.
prev parent reply other threads:[~2023-07-12 14:31 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-12 16:07 [PATCH] tracing: fix memcpy size when copying stack entries Sven Schnelle
2023-06-12 16:34 ` Steven Rostedt
2023-06-13 5:19 ` Sven Schnelle
2023-06-13 15:37 ` Steven Rostedt
2023-06-14 10:41 ` Sven Schnelle
2023-06-14 11:30 ` David Laight
2023-07-12 14:06 ` Sven Schnelle
2023-07-12 14:14 ` Steven Rostedt
2023-07-12 14:26 ` Steven Rostedt
2023-07-12 14:32 ` Sven Schnelle
2023-07-12 14:31 ` Sven Schnelle [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=yt9d1qhdfbgs.fsf@linux.ibm.com \
--to=svens@linux.ibm.com \
--cc=keescook@chromium.org \
--cc=linux-kernel@vger.kernel.org \
--cc=rostedt@goodmis.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.