* Ard Biesheuvel <ardb@xxxxxxxxxx> wrote:
> On Thu, 20 Mar 2025 at 17:22, Sami Tolvanen <samitolvanen@xxxxxxxxxx> wrote:
> >
> > On Thu, Mar 20, 2025 at 9:09 AM Ard Biesheuvel <ardb@xxxxxxxxxx> wrote:
> > >
> > > On Thu, 20 Mar 2025 at 15:48, Sami Tolvanen <samitolvanen@xxxxxxxxxx> wrote:
> > > >
> > > > Hi Ard,
> > > >
> > > > On Thu, Mar 20, 2025 at 12:07 AM Ard Biesheuvel <ardb+git@xxxxxxxxxx> wrote:
> > > > >
> > > > > From: Ard Biesheuvel <ardb@xxxxxxxxxx>
> > > > >
> > > > > Ensure that __GENKSYMS__ is #define'd when passing asm/asm-prototypes.h
> > > > > through the compiler to capture the exported symbols. This ensures that
> > > > > exported symbols such as __ref_stack_chk_guard on x86, which is declared
> > > > > conditionally, is visible to the tool.
> > > > >
> > > > > Otherwise, an error such as the below may be raised, breaking the build
> > > > > when CONFIG_GENDWARFKSYMS=y
> > > > >
> > > > > <stdin>:4:15: error: use of undeclared identifier '__ref_stack_chk_guard'
> > > > >
> > > > > Cc: Sami Tolvanen <samitolvanen@xxxxxxxxxx>
> > > > > Reported-by: syzbot+06fd1a3613c50d36129e@xxxxxxxxxxxxxxxxxxxxxxxxx
> > > > > Signed-off-by: Ard Biesheuvel <ardb@xxxxxxxxxx>
> > > > > ---
> > > > > scripts/Makefile.build | 1 +
> > > > > 1 file changed, 1 insertion(+)
> > > > >
> > > > > diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> > > > > index 993708d11874..7855cdc4e763 100644
> > > > > --- a/scripts/Makefile.build
> > > > > +++ b/scripts/Makefile.build
> > > > > @@ -305,6 +305,7 @@ $(obj)/%.rs: $(obj)/%.rs.S FORCE
> > > > > getasmexports = \
> > > > > { echo "\#include <linux/kernel.h>" ; \
> > > > > echo "\#include <linux/string.h>" ; \
> > > > > + echo "\#define __GENKSYMS__" ; \
> > > > > echo "\#include <asm/asm-prototypes.h>" ; \
> > > > > $(call getexportsymbols,EXPORT_SYMBOL(\1);) ; }
> > > >
> > > > This works with gendwarfksyms since __GENKSYMS__ is defined after the
> > > > EXPORT_SYMBOL() definition, but I'm now getting warnings with
> > > > genksyms:
> > > >
> > > > AS arch/x86/lib/clear_page_64.o
> > > > <stdin>:3:10: warning: "__GENKSYMS__" redefined
> > > > <command-line>: note: this is the location of the previous definition
> > > >
> > >
> > > Oops.
> > >
> > > Do you think the fix below should be sufficient?
> > >
> > > --- a/scripts/Makefile.build
> > > +++ b/scripts/Makefile.build
> > > @@ -314,7 +314,7 @@
> > > else
> > > cmd_gensymtypes_S = \
> > > $(getasmexports) | \
> > > - $(CPP) -D__GENKSYMS__ $(c_flags) -xc - | $(genksyms)
> > > + $(CPP) $(c_flags) -xc - | $(genksyms)
> > > endif # CONFIG_GENDWARFKSYMS
> > >
> > > quiet_cmd_cpp_s_S = CPP $(quiet_modtag) $@
> >
> > EXPORT_SYMBOL() has a different definition when __GENKSYMS__ is
> > defined, so I think with genksyms we actually do need this on the
> > command line. I suppose you could wrap the getasmexports definition in
> > #ifndef __GENKSYMS__ to avoid the warning, or just use
> > __GENDWARFKSYMS__ like you suggested earlier.
> >
>
> Yeah, on second thought, we could just do what Masahiro suggested, and
> drop the conditional from asm/asm-prototypes.h
>
> The issue in question only affects definitions, not declarations, and
> so having the declaration visible shouldn't be a problem.
>
> Ingo, mind dropping this patch again? We'll do the below instead (I'll
> send out the patch in a minute)
Yeah, done & thanks!
Ingo
