On Wed, Apr 24, 2024 at 12:26:52PM -0700, Nathan Chancellor wrote: > Hi Kees, > > On Wed, Apr 24, 2024 at 09:29:43AM -0700, Kees Cook wrote: > > When generating Runtime Calls, Clang doesn't respect the -mregparm=3 > > option used on i386. Hopefully this will be fixed correctly in Clang 19: > > https://github.com/llvm/llvm-project/pull/89707 > > but we need to fix this for earlier Clang versions today. Force the > > calling convention to use non-register arguments. > > > > Reported-by: ernsteiswuerfel > > FWIW, I think this can be > > Reported-by: Erhard Furtner <[email protected]> > > since it has been used in the kernel before, the reporter is well known > :)
Ah! Okay, thanks. I wasn't able to find an associated email address. :) > > > Closes: https://github.com/KSPP/linux/issues/350 > > Signed-off-by: Kees Cook <[email protected]> > > --- > > Cc: Marco Elver <[email protected]> > > Cc: Andrey Konovalov <[email protected]> > > Cc: Andrey Ryabinin <[email protected]> > > Cc: Nathan Chancellor <[email protected]> > > Cc: Nick Desaulniers <[email protected]> > > Cc: Bill Wendling <[email protected]> > > Cc: Justin Stitt <[email protected]> > > Cc: [email protected] > > Cc: [email protected] > > Cc: [email protected] > > --- > > lib/ubsan.h | 41 +++++++++++++++++++++++++++-------------- > > 1 file changed, 27 insertions(+), 14 deletions(-) > > > > diff --git a/lib/ubsan.h b/lib/ubsan.h > > index 50ef50811b7c..978828f6099d 100644 > > --- a/lib/ubsan.h > > +++ b/lib/ubsan.h > > @@ -124,19 +124,32 @@ typedef s64 s_max; > > typedef u64 u_max; > > #endif > > > > -void __ubsan_handle_add_overflow(void *data, void *lhs, void *rhs); > > -void __ubsan_handle_sub_overflow(void *data, void *lhs, void *rhs); > > -void __ubsan_handle_mul_overflow(void *data, void *lhs, void *rhs); > > -void __ubsan_handle_negate_overflow(void *_data, void *old_val); > > -void __ubsan_handle_divrem_overflow(void *_data, void *lhs, void *rhs); > > -void __ubsan_handle_type_mismatch(struct type_mismatch_data *data, void > > *ptr); > > -void __ubsan_handle_type_mismatch_v1(void *_data, void *ptr); > > -void __ubsan_handle_out_of_bounds(void *_data, void *index); > > -void __ubsan_handle_shift_out_of_bounds(void *_data, void *lhs, void *rhs); > > -void __ubsan_handle_builtin_unreachable(void *_data); > > -void __ubsan_handle_load_invalid_value(void *_data, void *val); > > -void __ubsan_handle_alignment_assumption(void *_data, unsigned long ptr, > > - unsigned long align, > > - unsigned long offset); > > +/* > > + * When generating Runtime Calls, Clang doesn't respect the -mregparm=3 > > + * option used on i386. Hopefully this will be fixed correctly in Clang 19: > > + * https://github.com/llvm/llvm-project/pull/89707 > > + * but we need to fix this for earlier Clang versions today. Force the > > It may be better to link to the tracking issue upstream instead of the > pull request just in case someone comes up with an alternative fix (not > that I think your change is wrong or anything but it seems like that > happens every so often). > > I also get leary of the version information in the comment, even though > I don't doubt this will be fixed in clang 19. > > > + * calling convention to use non-register arguments. > > + */ > > +#if defined(__clang__) && defined(CONFIG_X86_32) > > While __clang__ is what causes CONFIG_CC_IS_CLANG to get set and there > is some existing use of it throughout the kernel, I think > CONFIG_CC_IS_CLANG makes it easier to audit the workarounds that we > have, plus this will be presumably covered to > > CONFIG_CLANG_VERSION < 190000 Yeah, that seems much cleaner. I will adjust it... > > when the fix actually lands. This file is not expected to be used > outside of the kernel, right? That is the only thing I could think of > where this distinction would actually matter. > > > +# define ubsan_linkage asmlinkage > > Heh, clever... > > > +#else > > +# define ubsan_linkage /**/ > > Why is this defined as a comment rather than just nothing? I dunno; this is a coding style glitch of mine. :P I will drop it. Thanks for the review! -Kees > > > +#endif > > + > > +void ubsan_linkage __ubsan_handle_add_overflow(void *data, void *lhs, void > > *rhs); > > +void ubsan_linkage __ubsan_handle_sub_overflow(void *data, void *lhs, void > > *rhs); > > +void ubsan_linkage __ubsan_handle_mul_overflow(void *data, void *lhs, void > > *rhs); > > +void ubsan_linkage __ubsan_handle_negate_overflow(void *_data, void > > *old_val); > > +void ubsan_linkage __ubsan_handle_divrem_overflow(void *_data, void *lhs, > > void *rhs); > > +void ubsan_linkage __ubsan_handle_type_mismatch(struct type_mismatch_data > > *data, void *ptr); > > +void ubsan_linkage __ubsan_handle_type_mismatch_v1(void *_data, void *ptr); > > +void ubsan_linkage __ubsan_handle_out_of_bounds(void *_data, void *index); > > +void ubsan_linkage __ubsan_handle_shift_out_of_bounds(void *_data, void > > *lhs, void *rhs); > > +void ubsan_linkage __ubsan_handle_builtin_unreachable(void *_data); > > +void ubsan_linkage __ubsan_handle_load_invalid_value(void *_data, void > > *val); > > +void ubsan_linkage __ubsan_handle_alignment_assumption(void *_data, > > unsigned long ptr, > > + unsigned long align, > > + unsigned long offset); > > > > #endif > > -- > > 2.34.1 > > > > -- Kees Cook
