From nobody Sun Feb 8 15:29:20 2026 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id DC504C001DF for ; Mon, 31 Jul 2023 23:17:22 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231895AbjGaXRV (ORCPT ); Mon, 31 Jul 2023 19:17:21 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39980 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231776AbjGaXRQ (ORCPT ); Mon, 31 Jul 2023 19:17:16 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 72D2E172A for ; Mon, 31 Jul 2023 16:17:07 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id E8D3F61355 for ; Mon, 31 Jul 2023 23:17:06 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 4D6EDC433C9; Mon, 31 Jul 2023 23:17:06 +0000 (UTC) Received: from rostedt by gandalf with local (Exim 4.96) (envelope-from ) id 1qQc8P-003fKJ-0p; Mon, 31 Jul 2023 19:17:05 -0400 Message-ID: <20230731231705.072845526@goodmis.org> User-Agent: quilt/0.66 Date: Mon, 31 Jul 2023 19:16:35 -0400 From: Steven Rostedt To: linux-kernel@vger.kernel.org Cc: Masami Hiramatsu , Mark Rutland , Andrew Morton , Sven Schnelle , Kees Cook Subject: [for-next][PATCH 01/15] tracing: Add back FORTIFY_SOURCE logic to kernel_stack event structure References: <20230731231634.031452225@goodmis.org> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" From: "Steven Rostedt (Google)" For backward compatibility, older tooling expects to see the kernel_stack event with a "caller" field that is a fixed size array of 8 addresses. The code now supports more than 8 with an added "size" field that states the real number of entries. But the "caller" field still just looks like a fixed size to user space. Since the tracing macros that create the user space format files also creates the structures that those files represent, the kernel_stack event structure had its "caller" field a fixed size of 8, but in reality, when it is allocated on the ring buffer, it can hold more if the stack trace is bigger that 8 functions. The copying of these entries was simply done with a memcpy(): size =3D nr_entries * sizeof(unsigned long); memcpy(entry->caller, fstack->calls, size); The FORTIFY_SOURCE logic noticed at runtime that when the nr_entries was larger than 8, that the memcpy() was writing more than what the structure stated it can hold and it complained about it. This is because the FORTIFY_SOURCE code is unaware that the amount allocated is actually enough to hold the size. It does not expect that a fixed size field will hold more than the fixed size. This was originally solved by hiding the caller assignment with some pointer arithmetic. ptr =3D ring_buffer_data(); entry =3D ptr; ptr +=3D offsetof(typeof(*entry), caller); memcpy(ptr, fstack->calls, size); But it is considered bad form to hide from kernel hardening. Instead, make it work nicely with FORTIFY_SOURCE by adding a new __stack_array() macro that is specific for this one special use case. The macro will take 4 arguments: type, item, len, field (whereas the __array() macro takes just the first three). This macro will act just like the __array() macro when creating the code to deal with the format file that is exposed to user space. But for the kernel, it will turn the caller field into: type item[] __counted_by(field); or for this instance: unsigned long caller[] __counted_by(size); Now the kernel code can expose the assignment of the caller to the FORTIFY_SOURCE and everyone is happy! Link: https://lore.kernel.org/linux-trace-kernel/20230712105235.5fc441aa@ga= ndalf.local.home/ Link: https://lore.kernel.org/linux-trace-kernel/20230713092605.2ddb9788@ro= rschach.local.home Cc: Masami Hiramatsu Cc: Mark Rutland Cc: Sven Schnelle Suggested-by: Kees Cook Signed-off-by: Steven Rostedt (Google) Reviewed-by: Kees Cook --- kernel/trace/trace.c | 25 ++++--------------------- kernel/trace/trace.h | 10 ++++++++++ kernel/trace/trace_entries.h | 2 +- kernel/trace/trace_export.c | 9 +++++++++ 4 files changed, 24 insertions(+), 22 deletions(-) diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c index b8870078ef58..f6ed6b79d91f 100644 --- a/kernel/trace/trace.c +++ b/kernel/trace/trace.c @@ -3119,7 +3119,6 @@ static void __ftrace_trace_stack(struct trace_buffer = *buffer, struct ftrace_stack *fstack; struct stack_entry *entry; int stackidx; - void *ptr; =20 /* * Add one, for this function and the call to save_stack_trace() @@ -3157,32 +3156,16 @@ static void __ftrace_trace_stack(struct trace_buffe= r *buffer, nr_entries =3D stack_trace_save(fstack->calls, size, skip); } =20 - size =3D nr_entries * sizeof(unsigned long); event =3D __trace_buffer_lock_reserve(buffer, TRACE_STACK, - (sizeof(*entry) - sizeof(entry->caller)) + size, + struct_size(entry, caller, nr_entries), trace_ctx); if (!event) goto out; - ptr =3D ring_buffer_event_data(event); - entry =3D 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 +=3D offsetof(typeof(*entry), caller); - memcpy(ptr, fstack->calls, size); + entry =3D ring_buffer_event_data(event); =20 entry->size =3D nr_entries; + memcpy(&entry->caller, fstack->calls, + flex_array_size(entry, caller, nr_entries)); =20 if (!call_filter_check_discard(call, entry, buffer, event)) __buffer_unlock_commit(buffer, event); diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h index e1edc2197fc8..ba7ababb8308 100644 --- a/kernel/trace/trace.h +++ b/kernel/trace/trace.h @@ -77,6 +77,16 @@ enum trace_type { #undef __array #define __array(type, item, size) type item[size]; =20 +/* + * For backward compatibility, older user space expects to see the + * kernel_stack event with a fixed size caller field. But today the fix + * size is ignored by the kernel, and the real structure is dynamic. + * Expose to user space: "unsigned long caller[8];" but the real structure + * will be "unsigned long caller[] __counted_by(size)" + */ +#undef __stack_array +#define __stack_array(type, item, size, field) type item[] __counted_by(f= ield); + #undef __array_desc #define __array_desc(type, container, item, size) =20 diff --git a/kernel/trace/trace_entries.h b/kernel/trace/trace_entries.h index 340b2fa98218..c47422b20908 100644 --- a/kernel/trace/trace_entries.h +++ b/kernel/trace/trace_entries.h @@ -190,7 +190,7 @@ FTRACE_ENTRY(kernel_stack, stack_entry, =20 F_STRUCT( __field( int, size ) - __array( unsigned long, caller, FTRACE_STACK_ENTRIES ) + __stack_array( unsigned long, caller, FTRACE_STACK_ENTRIES, size) ), =20 F_printk("\t=3D> %ps\n\t=3D> %ps\n\t=3D> %ps\n" diff --git a/kernel/trace/trace_export.c b/kernel/trace/trace_export.c index 58f3946081e2..1698fc22afa0 100644 --- a/kernel/trace/trace_export.c +++ b/kernel/trace/trace_export.c @@ -51,6 +51,9 @@ static int ftrace_event_register(struct trace_event_call = *call, #undef __array #define __array(type, item, size) type item[size]; =20 +#undef __stack_array +#define __stack_array(type, item, size, field) __array(type, item, size) + #undef __array_desc #define __array_desc(type, container, item, size) type item[size]; =20 @@ -114,6 +117,9 @@ static void __always_unused ____ftrace_check_##name(voi= d) \ is_signed_type(_type), .filter_type =3D FILTER_OTHER, \ .len =3D _len }, =20 +#undef __stack_array +#define __stack_array(_type, _item, _len, _field) __array(_type, _item, _l= en) + #undef __array_desc #define __array_desc(_type, _container, _item, _len) __array(_type, _item,= _len) =20 @@ -149,6 +155,9 @@ static struct trace_event_fields ftrace_event_fields_##= name[] =3D { \ #undef __array #define __array(type, item, len) =20 +#undef __stack_array +#define __stack_array(type, item, len, field) + #undef __array_desc #define __array_desc(type, container, item, len) =20 --=20 2.40.1