On Thu, Aug 27, 2020 at 11:42 AM Kees Cook <keescook@xxxxxxxxxxxx> wrote:
>
> Do you mean you checked both gcc and clang and it was only a problem with gcc?
I didn't check with clang, but Arnd claimed it was fine.
> (If so, I can tweak the "depends" below...)
Ugh.
Instead of making the Makefile even uglier, why don't you just make
this all be done in the Kconfig.
Also, I'm not seeing the point of your patch. You didn't actually
change anything, you just made a new config variable with the same
semantics as the old one.
Add a
depends on CLANG
or something, with a comment saying that it doesn't work on gcc due to
excessive stack use.
> +ifdef CONFIG_UBSAN_OBJECT_SIZE
> + CFLAGS_UBSAN += $(call cc-option, -fsanitize=object-size)
> +endif
All of this should be thrown out, and this code should use the proper
patterns for configuration entries in the Makefile, ie just
ubsan-cflags-$(CONFIG_UBSAN_OBJECT_SIZE) += -fsanitize=object-size
and the Kconfig file is the thing that should check if that CC option
exists with
config UBSAN_OBJECT_SIZE
bool "Check for accesses beyond known object sizes"
default UBSAN
depends on CLANG # gcc makes a mess of it
depends on $(cc-option,-fsanitize-coverage=trace-pc)
and the same goes for all the other cases too:
> ifdef CONFIG_UBSAN_MISC
> CFLAGS_UBSAN += $(call cc-option, -fsanitize=shift)
> CFLAGS_UBSAN += $(call cc-option, -fsanitize=integer-divide-by-zero)
> CFLAGS_UBSAN += $(call cc-option, -fsanitize=unreachable)
> CFLAGS_UBSAN += $(call cc-option, -fsanitize=signed-integer-overflow)
> - CFLAGS_UBSAN += $(call cc-option, -fsanitize=object-size)
> CFLAGS_UBSAN += $(call cc-option, -fsanitize=bool)
> CFLAGS_UBSAN += $(call cc-option, -fsanitize=enum)
> endif
and if you don't want to ask for them (which is a good idea), you keep that
config UBSAN_MISC
bool "Misc UBSAN.."
thing, and just make all of the above have the pattern of
config UBSAN_OBJECT_SIZE
def_bool UBSAN_MISC
depends on CLANG # gcc makes a mess of it
depends on $(cc-option,-fsanitize-coverage=trace-pc)
which makes the Makefile much cleaner, and makes all our choices very
visible in the config file when they then get passed around.
We should basically strive for our Makefiles to have as little "ifdef"
etc magic as possible. We did the config work already, the Makefiles
should primarily just have those
XYZ-$(CONFIG_OPTION) += abc
kind of lines (and then you often end up having
CFLAGS_UBSAN := $(ubsan-cflags-y)
at the end).
Doesn't that all look much cleaner?
Linus
