All of lore.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 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.