Re: [BUG v5.2-rc1] ARM build broken

[H. Nikolaus Schaller]

Hi,

> Am 20.05.2019 um 20:53 schrieb Kees Cook <keescook@xxxxxxxxxxxx>:
> 
> [Adding Chris and Ard, who might have more compiler versions that me...]
> 
> On Mon, May 20, 2019 at 07:08:39PM +0200, H. Nikolaus Schaller wrote:
>>> Am 20.05.2019 um 17:59 schrieb Kees Cook <keescook@xxxxxxxxxxxx>:
>>> 
>>> On Mon, May 20, 2019 at 05:15:02PM +0200, H. Nikolaus Schaller wrote:
>>>> Hi,
>>>> it seems as if ARM build is broken since ARM now hard enables CONFIG_HAVE_GCC_PLUGINS
>>>> which indirectly enables CONFIG_GCC_PLUGIN_ARM_SSP_PER_TASK. Compiling this breaks
>>>> on my system (Darwin build host) due to conflicts in system headers and Linux headers.
>>>> 
>>>> So how can I turn off all these GCC_PLUGINS?
>>>> 
>>>> The offending patch seems to be
>>>> 
>>>> 	security: Create "kernel hardening" config area
>>>> 
>>>> especially the new "default y" for GCC_PLUGINS. After removing that line from
>>>> scripts/gcc-plugins/Kconfig makes my compile succeed.
>>> 
>>> The intention is to enable it _if_ the plugins are available as part of
>>> the build environment. The "default y" on GCC_PLUGINS is mediated by:
>>>       depends on HAVE_GCC_PLUGINS
>> 
>> HAVE_GCC_PLUGINS has the following description:
>> 
>> 	An arch should select this symbol if it supports building with
>>          GCC plugins.
>> 
>> So an ARCH (ARM) selects it unconditionally of the build environment.
>> 
>>>       depends on PLUGIN_HOSTCC != ""
>> 
>> Well, we have it set to "g++" for ages and it was not a problem.
>> So both conditions are true.
> 
> PLUGIN_HOSTCC should have passed the scripts/gcc-plugin.sh test, so
> that's correct. And the result (CONFIG_GCC_PLUGINS) is correct: it
> doesn't enable or disable anything itself.
> 
> What you want is to disable CONFIG_STACKPROTECTOR_PER_TASK, which
> is the knob for the feature:
> 
> config STACKPROTECTOR_PER_TASK
>        bool "Use a unique stack canary value for each task"
>        depends on GCC_PLUGINS && STACKPROTECTOR && SMP && !XIP_DEFLATED_DATA
>        select GCC_PLUGIN_ARM_SSP_PER_TASK
>        default y

Ah, disabling this makes it compile.

Unfortunately it is not explicitly disabled by e.g. omap2plus_defconfig and therefore
automatically set through the "default y". So I have to manipulate the defconfig first.

> 
>> Build error:
>> 
>> HOSTCXX -fPIC scripts/gcc-plugins/arm_ssp_per_task_plugin.o - due to: scripts/gcc-plugins/gcc-common.h
>> In file included from scripts/gcc-plugins/arm_ssp_per_task_plugin.c:3:0:
>> scripts/gcc-plugins/gcc-common.h:153:0: warning: "__unused" redefined
>> #define __unused __attribute__((__unused__))
>> ^
> 
> Does the following patch fix your build? (I assume that line is just a
> warning, but if not...)
> 
> diff --git a/scripts/gcc-plugins/gcc-common.h b/scripts/gcc-plugins/gcc-common.h
> index 552d5efd7cb7..17f06079a712 100644
> --- a/scripts/gcc-plugins/gcc-common.h
> +++ b/scripts/gcc-plugins/gcc-common.h
> @@ -150,8 +150,12 @@ void print_gimple_expr(FILE *, gimple, int, int);
> void dump_gimple_stmt(pretty_printer *, gimple, int, int);
> #endif
> 
> +#ifndef __unused
> #define __unused __attribute__((__unused__))
> +#endif
> +#ifndef __visible
> #define __visible __attribute__((visibility("default")))
> +#endif
> 
> #define DECL_NAME_POINTER(node) IDENTIFIER_POINTER(DECL_NAME(node))
> #define DECL_NAME_LENGTH(node) IDENTIFIER_LENGTH(DECL_NAME(node))

Yes, fixes this issue.

> 
>> HOSTLLD -shared scripts/gcc-plugins/arm_ssp_per_task_plugin.so - due to target missing
>> Undefined symbols for architecture x86_64:
>> "gen_reg_rtx(machine_mode)", referenced from:
>>     (anonymous namespace)::arm_pertask_ssp_rtl_pass::execute() in arm_ssp_per_task_plugin.o
> 
> However, this part sounds more like what was fixed with
> 259799ea5a9a ("gcc-plugins: arm_ssp_per_task_plugin: Fix for older GCC < 6")
> 
> And maybe some additional fixes for 4.9 are needed?

Looks similar although not a compiler but linker error. Which indicates a library search issue.

Unfortunately gcc plugins is something I didn't even know that it exists until some hours ago :)

So I have no idea where e.g. _plugin_default_version_check, _default_target_rtl or _register_callback
should be provided.

If it is part of the gcc build-from-source tree I may have a chance to add the path to the CCXFLAGS.
Or is it to be provided from the kernel tree?

> 
>> This is because CONFIG_GCC_PLUGIN_ARM_SSP_PER_TASK became automatically enabled and was never
>> before. So the compiler may lack some library search path for building this plugin (which we
>> did never miss).
> 
> Right -- maybe CONFIG_STACKPROTECTOR_PER_TASK doesn't work with old gcc
> 4.9.2? I'll see if I can find that compiler version...

I think Debian Jessie used the same compiler version as default gcc.

BR and thanks,
Nikolaus Schaller