On Fri, Feb 23, 2024 at 07:26:46PM +0100, Sam Ravnborg wrote: > Hi Kees, > > On Fri, Feb 23, 2024 at 08:59:45AM -0800, Kees Cook wrote: > > The UBSAN instrumentation cannot work in the vDSO since it is executing > > in userspace, so disable it in the Makefile. Fixes the build failures > > such as: > > > > arch/sparc/vdso/vclock_gettime.c:217: undefined reference to `__ubsan_handle_shift_out_of_bounds' > > > > Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx> > > --- > > Cc: "David S. Miller" <davem@xxxxxxxxxxxxx> > > Cc: Andreas Larsson <andreas@xxxxxxxxxxx> > > Cc: Masahiro Yamada <masahiroy@xxxxxxxxxx> > > Cc: Sam Ravnborg <sam@xxxxxxxxxxxx> > > Cc: Helge Deller <deller@xxxxxx> > > Cc: Guo Ren <guoren@xxxxxxxxxx> > > Cc: sparclinux@xxxxxxxxxxxxxxx > > --- > > arch/sparc/vdso/Makefile | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/arch/sparc/vdso/Makefile b/arch/sparc/vdso/Makefile > > index 7f5eedf1f5e0..e8aef2c8ae99 100644 > > --- a/arch/sparc/vdso/Makefile > > +++ b/arch/sparc/vdso/Makefile > > @@ -2,6 +2,7 @@ > > # > > # Building vDSO images for sparc. > > # > > +UBSAN_SANITIZE := n > > When I read: > > config UBSAN_SANITIZE_ALL > bool "Enable instrumentation for the entire kernel" > depends on ARCH_HAS_UBSAN_SANITIZE_ALL > default y > help > This option activates instrumentation for the entire kernel. > If you don't enable this option, you have to explicitly specify > UBSAN_SANITIZE := y for the files/directories you want to check for UB. > Enabling this option will get kernel image size increased > significantly. > > > I am left with the understanding that only arch's that > selects ARCH_HAS_UBSAN_SANITIZE_ALL would need to turn off > UBSAN_SANITIZE. Ah, right. So, I removed[1] UBSAN_SANITIZE_ALL in -next (it was the only sanitizer using this logic) and this appears to be one of the impacts. :) I sent similar fixes for sh[2] and LoongArch[3]. > Are this fix papering over some other bug where we enable > UBSAN_SANITIZE_ALL for arch's that should not have it, > or something else that enable it? It's possible we should implement HAVE_ARCH_UBSAN, but in my testing everything built fine with it, so I didn't opt to do that (it looked like just additional configs for no real benefit). What do you think? -Kees [1] https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/commit/?h=for-next/kspp&id=918327e9b7ffb45321cbb4b9b86b58ec555fe6b3 [2] https://lore.kernel.org/lkml/20240130232717.work.088-kees@xxxxxxxxxx/ [3] https://lore.kernel.org/lkml/20240130233140.work.887-kees@xxxxxxxxxx/ -- Kees Cook
