public inbox for linux-trace-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Nathan Chancellor <nathan@kernel.org>
To: Vincent Donnefort <vdonnefort@google.com>
Cc: maz@kernel.org, rostedt@goodmis.org, arnd@arndb.de,
	linux-trace-kernel@vger.kernel.org, kvmarm@lists.linux.dev,
	kernel-team@android.com
Subject: Re: [PATCH] tracing: Generate undef symbols allowlist for simple_ring_buffer
Date: Thu, 12 Mar 2026 16:51:53 -0700	[thread overview]
Message-ID: <20260312235153.GA1147071@ax162> (raw)
In-Reply-To: <20260312182010.111013-1-vdonnefort@google.com>

Hi Vincent,

On Thu, Mar 12, 2026 at 06:20:10PM +0000, Vincent Donnefort wrote:
> Compiler and tooling-generated symbols are difficult to maintain
> across all supported architectures. Make the allowlist more robust by
> replacing the harcoded list with a mechanism that automatically detects
> these symbols.
> 
> This mechanism generates a C function designed to trigger common
> compiler-inserted symbols.

This certainly seems more robust.

> Signed-off-by: Vincent Donnefort <vdonnefort@google.com>
> 
> diff --git a/kernel/trace/Makefile b/kernel/trace/Makefile
> index beb15936829d..3b427b76434a 100644
> --- a/kernel/trace/Makefile
> +++ b/kernel/trace/Makefile
> @@ -136,17 +136,37 @@ obj-$(CONFIG_TRACE_REMOTE_TEST) += remote_test.o
>  # simple_ring_buffer is used by the pKVM hypervisor which does not have access
>  # to all kernel symbols. Fail the build if forbidden symbols are found.
>  #
> -UNDEFINED_ALLOWLIST := memset alt_cb_patch_nops __x86 __ubsan __asan __kasan __gcov __aeabi_unwind
> -UNDEFINED_ALLOWLIST += __stack_chk_fail stackleak_track_stack __ref_stack __sanitizer llvm_gcda llvm_gcov
> -UNDEFINED_ALLOWLIST += .TOC\. __clear_pages_unrolled __memmove copy_page warn_slowpath_fmt
> -UNDEFINED_ALLOWLIST += ftrace_likely_update __hwasan_load __hwasan_store __hwasan_tag_memory
> -UNDEFINED_ALLOWLIST += warn_bogus_irq_restore __stack_chk_guard
> -UNDEFINED_ALLOWLIST := $(addprefix -e , $(UNDEFINED_ALLOWLIST))
> +# undefsyms_base generates a set of compiler and tooling-generated symbols that can
> +# safely be ignored for simple_ring_buffer.
> +#
> +$(obj)/undefsyms_base.c: FORCE
> +	$(Q)echo '#include <asm/page.h>' > $@
> +	$(Q)echo '#include <asm/local.h>' >> $@
> +	$(Q)echo 'static char page[PAGE_SIZE] __aligned(PAGE_SIZE);' >> $@
> +	$(Q)echo 'void undefsyms_base(int n);' >> $@
> +	$(Q)echo 'void undefsyms_base(int n) {' >> $@
> +	$(Q)echo '	char buffer[256] = { 0 };' >> $@
> +	$(Q)echo '	u32 u = 0;' >> $@
> +	$(Q)echo '	memset((char * volatile)page, 8, PAGE_SIZE);' >> $@
> +	$(Q)echo '	memset((char * volatile)buffer, 8, sizeof(buffer));' >> $@
> +	$(Q)echo '	cmpxchg((u32 * volatile)&u, 0, 8);' >> $@
> +	$(Q)echo '	WARN_ON(n == 0xdeadbeef);' >> $@
> +	$(Q)echo '}' >> $@

This should use filechk, otherwise undefsyms_base.c will be regenerated
every build, resulting in undefsyms_base.o being rebuilt every time.

  $ make -skj"$(nproc)" ARCH=x86_64 mrproper allmodconfig kernel/trace/

  $ make -skj"$(nproc)" ARCH=x86_64 V=2 kernel/trace/
  ...
    CC      kernel/trace/undefsyms_base.o - due to: kernel/trace/undefsyms_base.c
    NM      kernel/trace/simple_ring_buffer.o - due to target missing

filechk_undefsyms_base = {                                              \
	echo '$(pound)include <asm/page.h>';                            \
	echo '$(pound)include <asm/local.h>';                           \
	echo 'static char page[PAGE_SIZE] __aligned(PAGE_SIZE);';       \
	echo 'void undefsyms_base(int n);';                             \
	echo 'void undefsyms_base(int n) {';                            \
	echo '	char buffer[256] = { 0 };';                             \
	echo '	u32 u = 0;';                                            \
	echo '	memset((char * volatile)page, 8, PAGE_SIZE);';          \
	echo '	memset((char * volatile)buffer, 8, sizeof(buffer));';   \
	echo '	cmpxchg((u32 * volatile)&u, 0, 8);';                    \
	echo '	WARN_ON(n == 0xdeadbeef);';                             \
	echo '}';                                                       \
	}

$(obj)/undefsyms_base.c: FORCE
	$(call filechk,undefsyms_base)

  $ make -skj"$(nproc)" ARCH=x86_64 mrproper allmodconfig kernel/trace/

  $ make -skj"$(nproc)" ARCH=x86_64 V=2 kernel/trace/
    GEN     Makefile - due to target is PHONY
    DESCEND objtool
    CALL    scripts/checksyscalls.sh - due to target is PHONY
    INSTALL libsubcmd_headers
    NM      kernel/trace/simple_ring_buffer.o - due to target missing

> +clean-files += undefsyms_base.c
> +targets += undefsyms_base.c

I don't think this targets addition is necessary.

> +$(obj)/undefsyms_base.o: $(obj)/undefsyms_base.c
> +
> +extra-y += undefsyms_base.o

I think this should be

  targets += undefsyms_base.o

as extra-y is deprecated per Documentation/kbuild/makefiles.rst.

> +UNDEFINED_ALLOWLIST = __asan __gcov __kasan __kcsan __hwasan __sanitizer __tsan __ubsan __x86_indirect_thunk \
> +		       $(shell $(NM) -u $(obj)/undefsyms_base.o 2>/dev/null | awk '{print $$2}')

With an allmodconfig + ThinLTO build, I still see:

  $ cat allmod.config
  CONFIG_GCOV_KERNEL=n
  CONFIG_KASAN=n
  CONFIG_LTO_CLANG_THIN=y

  $ make -skj"$(nproc)" ARCH=x86_64 KCONFIG_ALLCONFIG=1 LLVM=1 mrproper allmodconfig kernel/trace/
  Unexpected symbols in kernel/trace/simple_ring_buffer.o:
                   U __fortify_panic
                   U __write_overflow_field
                   U simple_ring_buffer_commit
                   U simple_ring_buffer_enable_tracing
                   U simple_ring_buffer_init
                   U simple_ring_buffer_reserve
                   U simple_ring_buffer_reset
                   U simple_ring_buffer_swap_reader_page
                   U simple_ring_buffer_unload

Something like:

diff --git a/kernel/trace/Makefile b/kernel/trace/Makefile
index 48c415a0c7e4..0f9a6ce9abd9 100644
--- a/kernel/trace/Makefile
+++ b/kernel/trace/Makefile
@@ -142,13 +142,15 @@ obj-$(CONFIG_TRACE_REMOTE_TEST) += remote_test.o
 filechk_undefsyms_base = {                                              \
 	echo '$(pound)include <asm/page.h>';                            \
 	echo '$(pound)include <asm/local.h>';                           \
+	echo '$(pound)include <linux/string.h>';                        \
 	echo 'static char page[PAGE_SIZE] __aligned(PAGE_SIZE);';       \
-	echo 'void undefsyms_base(int n);';                             \
-	echo 'void undefsyms_base(int n) {';                            \
+	echo 'void undefsyms_base(int n, void *ptr);';                  \
+	echo 'void undefsyms_base(int n, void *ptr) {';                 \
 	echo '	char buffer[256] = { 0 };';                             \
 	echo '	u32 u = 0;';                                            \
 	echo '	memset((char * volatile)page, 8, PAGE_SIZE);';          \
 	echo '	memset((char * volatile)buffer, 8, sizeof(buffer));';   \
+	echo '	memcpy((void* volatile)ptr, buffer, sizeof(buffer));';  \
 	echo '	cmpxchg((u32 * volatile)&u, 0, 8);';                    \
 	echo '	WARN_ON(n == 0xdeadbeef);';                             \
 	echo '}';                                                       \
--

cures the first two. The simple_ring_buffer symbols are very odd...

  $ llvm-nm kernel/trace/simple_ring_buffer.o | grep simple_ring_buffer
  ---------------- d __UNIQUE_ID_addressable_simple_ring_buffer_commit_845
  ---------------- d __UNIQUE_ID_addressable_simple_ring_buffer_enable_tracing_849
  ---------------- d __UNIQUE_ID_addressable_simple_ring_buffer_init_847
  ---------------- d __UNIQUE_ID_addressable_simple_ring_buffer_reserve_841
  ---------------- d __UNIQUE_ID_addressable_simple_ring_buffer_reset_846
  ---------------- d __UNIQUE_ID_addressable_simple_ring_buffer_swap_reader_page_837
  ---------------- d __UNIQUE_ID_addressable_simple_ring_buffer_unload_848
  ---------------- t __export_symbol_simple_ring_buffer_commit
  ---------------- t __export_symbol_simple_ring_buffer_enable_tracing
  ---------------- t __export_symbol_simple_ring_buffer_init
  ---------------- t __export_symbol_simple_ring_buffer_reserve
  ---------------- t __export_symbol_simple_ring_buffer_reset
  ---------------- t __export_symbol_simple_ring_buffer_swap_reader_page
  ---------------- t __export_symbol_simple_ring_buffer_unload
  ---------------- T simple_ring_buffer_commit
                   U simple_ring_buffer_commit
  ---------------- T simple_ring_buffer_enable_tracing
                   U simple_ring_buffer_enable_tracing
                   U simple_ring_buffer_init
  ---------------- T simple_ring_buffer_init
  ---------------- T simple_ring_buffer_init_mm
  ---------------- T simple_ring_buffer_reserve
                   U simple_ring_buffer_reserve
  ---------------- T simple_ring_buffer_reset
                   U simple_ring_buffer_reset
                   U simple_ring_buffer_swap_reader_page
  ---------------- T simple_ring_buffer_swap_reader_page
                   U simple_ring_buffer_unload
  ---------------- T simple_ring_buffer_unload
  ---------------- T simple_ring_buffer_unload_mm

This is LLVM IR bitcode at this stage, which could be messing things up.

  $ file kernel/trace/simple_ring_buffer.o
  kernel/trace/simple_ring_buffer.o: LLVM IR bitcode

Maybe not worth thinking about too much and just adding it to the
allowlist manually?

diff --git a/kernel/trace/Makefile b/kernel/trace/Makefile
index 0f9a6ce9abd9..cb1ec50a8386 100644
--- a/kernel/trace/Makefile
+++ b/kernel/trace/Makefile
@@ -166,6 +166,7 @@ $(obj)/undefsyms_base.o: $(obj)/undefsyms_base.c
 targets += undefsyms_base.o
 
 UNDEFINED_ALLOWLIST = __asan __gcov __kasan __kcsan __hwasan __sanitizer __tsan __ubsan __x86_indirect_thunk \
+		       simple_ring_buffer \
 		       $(shell $(NM) -u $(obj)/undefsyms_base.o 2>/dev/null | awk '{print $$2}')
 
 quiet_cmd_check_undefined = NM      $<
--

Cheers,
Nathan

  parent reply	other threads:[~2026-03-12 23:51 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-12 18:20 [PATCH] tracing: Generate undef symbols allowlist for simple_ring_buffer Vincent Donnefort
2026-03-12 18:43 ` Steven Rostedt
2026-03-12 23:51 ` Nathan Chancellor [this message]
2026-03-13 10:23   ` Vincent Donnefort

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=20260312235153.GA1147071@ax162 \
    --to=nathan@kernel.org \
    --cc=arnd@arndb.de \
    --cc=kernel-team@android.com \
    --cc=kvmarm@lists.linux.dev \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=maz@kernel.org \
    --cc=rostedt@goodmis.org \
    --cc=vdonnefort@google.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox