| Message ID | 20210309123544.14040-1-msuchanek@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: make STACKPROTECTOR_PER_TASK configurable. | expand |
On Tue, 9 Mar 2021 at 13:37, Michal Suchanek <msuchanek@suse.de> wrote: > > When using dummy-tools STACKPROTECTOR_PER_TASK is unconditionally > selected. This defeats the purpose of the all-enabled tool. > What is dummy-tools and why should we care about it? > Description copied from arm > > Cc: Masahiro Yamada <masahiroy@kernel.org> > Signed-off-by: Michal Suchanek <msuchanek@suse.de> > --- > arch/arm64/Kconfig | 13 ++++++++++++- > 1 file changed, 12 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > index a8ff7cd5f096..f59d391e31a4 100644 > --- a/arch/arm64/Kconfig > +++ b/arch/arm64/Kconfig > @@ -1549,9 +1549,20 @@ config RANDOMIZE_MODULE_REGION_FULL > config CC_HAVE_STACKPROTECTOR_SYSREG > def_bool $(cc-option,-mstack-protector-guard=sysreg -mstack-protector-guard-reg=sp_el0 -mstack-protector-guard-offset=0) > > + > config STACKPROTECTOR_PER_TASK > - def_bool y > + bool "Use a unique stack canary value for each task" > depends on STACKPROTECTOR && CC_HAVE_STACKPROTECTOR_SYSREG > + default y > + help > + Due to the fact that GCC uses an ordinary symbol reference from > + which to load the value of the stack canary, this value can only > + change at reboot time on SMP systems, and all tasks running in the > + kernel's address space are forced to use the same canary value for > + the entire duration that the system is up. > + > + Enable this option to switch to a different method that uses a > + different canary value for each task. > > endmenu > > -- > 2.26.2 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Tue, Mar 9, 2021 at 9:35 PM Michal Suchanek <msuchanek@suse.de> wrote: > > When using dummy-tools STACKPROTECTOR_PER_TASK is unconditionally > selected. This defeats the purpose of the all-enabled tool. > > Description copied from arm > > Cc: Masahiro Yamada <masahiroy@kernel.org> > Signed-off-by: Michal Suchanek <msuchanek@suse.de> Could you explain what problem this patch is trying to solve? > --- > arch/arm64/Kconfig | 13 ++++++++++++- > 1 file changed, 12 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > index a8ff7cd5f096..f59d391e31a4 100644 > --- a/arch/arm64/Kconfig > +++ b/arch/arm64/Kconfig > @@ -1549,9 +1549,20 @@ config RANDOMIZE_MODULE_REGION_FULL > config CC_HAVE_STACKPROTECTOR_SYSREG > def_bool $(cc-option,-mstack-protector-guard=sysreg -mstack-protector-guard-reg=sp_el0 -mstack-protector-guard-offset=0) > > + > config STACKPROTECTOR_PER_TASK > - def_bool y > + bool "Use a unique stack canary value for each task" > depends on STACKPROTECTOR && CC_HAVE_STACKPROTECTOR_SYSREG > + default y > + help > + Due to the fact that GCC uses an ordinary symbol reference from > + which to load the value of the stack canary, this value can only > + change at reboot time on SMP systems, and all tasks running in the > + kernel's address space are forced to use the same canary value for > + the entire duration that the system is up. > + > + Enable this option to switch to a different method that uses a > + different canary value for each task. > > endmenu > > -- > 2.26.2 >
On Tue, Mar 09, 2021 at 10:22:36PM +0900, Masahiro Yamada wrote: > On Tue, Mar 9, 2021 at 9:35 PM Michal Suchanek <msuchanek@suse.de> wrote: > > > > When using dummy-tools STACKPROTECTOR_PER_TASK is unconditionally > > selected. This defeats the purpose of the all-enabled tool. > > > > Description copied from arm > > > > Cc: Masahiro Yamada <masahiroy@kernel.org> > > Signed-off-by: Michal Suchanek <msuchanek@suse.de> > > > Could you explain what problem > this patch is trying to solve? The option cannot be disabled when compiler has the required capability. Thanks Michal > > > > --- > > arch/arm64/Kconfig | 13 ++++++++++++- > > 1 file changed, 12 insertions(+), 1 deletion(-) > > > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > > index a8ff7cd5f096..f59d391e31a4 100644 > > --- a/arch/arm64/Kconfig > > +++ b/arch/arm64/Kconfig > > @@ -1549,9 +1549,20 @@ config RANDOMIZE_MODULE_REGION_FULL > > config CC_HAVE_STACKPROTECTOR_SYSREG > > def_bool $(cc-option,-mstack-protector-guard=sysreg -mstack-protector-guard-reg=sp_el0 -mstack-protector-guard-offset=0) > > > > + > > config STACKPROTECTOR_PER_TASK > > - def_bool y > > + bool "Use a unique stack canary value for each task" > > depends on STACKPROTECTOR && CC_HAVE_STACKPROTECTOR_SYSREG > > + default y > > + help > > + Due to the fact that GCC uses an ordinary symbol reference from > > + which to load the value of the stack canary, this value can only > > + change at reboot time on SMP systems, and all tasks running in the > > + kernel's address space are forced to use the same canary value for > > + the entire duration that the system is up. > > + > > + Enable this option to switch to a different method that uses a > > + different canary value for each task. > > > > endmenu > > > > -- > > 2.26.2 > > > > > -- > Best Regards > Masahiro Yamada
On Tue, Mar 9, 2021 at 10:35 PM Michal Suchánek <msuchanek@suse.de> wrote: > > On Tue, Mar 09, 2021 at 10:22:36PM +0900, Masahiro Yamada wrote: > > On Tue, Mar 9, 2021 at 9:35 PM Michal Suchanek <msuchanek@suse.de> wrote: > > > > > > When using dummy-tools STACKPROTECTOR_PER_TASK is unconditionally > > > selected. This defeats the purpose of the all-enabled tool. > > > > > > Description copied from arm > > > > > > Cc: Masahiro Yamada <masahiroy@kernel.org> > > > Signed-off-by: Michal Suchanek <msuchanek@suse.de> > > > > > > Could you explain what problem > > this patch is trying to solve? > > The option cannot be disabled when compiler has the required capability. Yes. Currently, this symbol claims "def_bool y", so there is no way to disable it. But, it comes from the nature of Kconfig in general. dummy-tools is completely unrelated here.
On Tue, Mar 09, 2021 at 11:53:21PM +0900, Masahiro Yamada wrote: > On Tue, Mar 9, 2021 at 10:35 PM Michal Suchánek <msuchanek@suse.de> wrote: > > > > On Tue, Mar 09, 2021 at 10:22:36PM +0900, Masahiro Yamada wrote: > > > On Tue, Mar 9, 2021 at 9:35 PM Michal Suchanek <msuchanek@suse.de> wrote: > > > > > > > > When using dummy-tools STACKPROTECTOR_PER_TASK is unconditionally > > > > selected. This defeats the purpose of the all-enabled tool. > > > > > > > > Description copied from arm > > > > > > > > Cc: Masahiro Yamada <masahiroy@kernel.org> > > > > Signed-off-by: Michal Suchanek <msuchanek@suse.de> > > > > > > > > > Could you explain what problem > > > this patch is trying to solve? > > > > The option cannot be disabled when compiler has the required capability. > > > Yes. > Currently, this symbol claims "def_bool y", > so there is no way to disable it. > > But, it comes from the nature of Kconfig in general. > > dummy-tools is completely unrelated here. dummy-tools makes all configuration options available in order to be able to author configuration files on system different from the one where the kernel is built. This prevents authoring a configuration file with this option disabled. Thanks Michal
On Wed, Mar 10, 2021 at 12:10 AM Michal Suchánek <msuchanek@suse.de> wrote: > > On Tue, Mar 09, 2021 at 11:53:21PM +0900, Masahiro Yamada wrote: > > On Tue, Mar 9, 2021 at 10:35 PM Michal Suchánek <msuchanek@suse.de> wrote: > > > > > > On Tue, Mar 09, 2021 at 10:22:36PM +0900, Masahiro Yamada wrote: > > > > On Tue, Mar 9, 2021 at 9:35 PM Michal Suchanek <msuchanek@suse.de> wrote: > > > > > > > > > > When using dummy-tools STACKPROTECTOR_PER_TASK is unconditionally > > > > > selected. This defeats the purpose of the all-enabled tool. > > > > > > > > > > Description copied from arm > > > > > > > > > > Cc: Masahiro Yamada <masahiroy@kernel.org> > > > > > Signed-off-by: Michal Suchanek <msuchanek@suse.de> > > > > > > > > > > > > Could you explain what problem > > > > this patch is trying to solve? > > > > > > The option cannot be disabled when compiler has the required capability. > > > > > > Yes. > > Currently, this symbol claims "def_bool y", > > so there is no way to disable it. > > > > But, it comes from the nature of Kconfig in general. > > > > dummy-tools is completely unrelated here. > > dummy-tools makes all configuration options available in order to be > able to author configuration files on system different from the one > where the kernel is built. This prevents authoring a configuration file > with this option disabled. No. dummy-tools enables as many $(cc-option, ...) and $(shell, ...) as possible. That's it. In my understanding, STACKPROTECTOR_PER_TASK should not be user-configurable. That is why 'def_bool y'. -- Best Regards Masahiro Yamada
On Wed, Mar 10, 2021 at 04:07:00AM +0900, Masahiro Yamada wrote: > On Wed, Mar 10, 2021 at 12:10 AM Michal Suchánek <msuchanek@suse.de> wrote: > > > > On Tue, Mar 09, 2021 at 11:53:21PM +0900, Masahiro Yamada wrote: > > > On Tue, Mar 9, 2021 at 10:35 PM Michal Suchánek <msuchanek@suse.de> wrote: > > > > > > > > On Tue, Mar 09, 2021 at 10:22:36PM +0900, Masahiro Yamada wrote: > > > > > On Tue, Mar 9, 2021 at 9:35 PM Michal Suchanek <msuchanek@suse.de> wrote: > > > > > > > > > > > > When using dummy-tools STACKPROTECTOR_PER_TASK is unconditionally > > > > > > selected. This defeats the purpose of the all-enabled tool. > > > > > > > > > > > > Description copied from arm > > > > > > > > > > > > Cc: Masahiro Yamada <masahiroy@kernel.org> > > > > > > Signed-off-by: Michal Suchanek <msuchanek@suse.de> > > > > > > > > > > > > > > > Could you explain what problem > > > > > this patch is trying to solve? > > > > > > > > The option cannot be disabled when compiler has the required capability. > > > > > > > > > Yes. > > > Currently, this symbol claims "def_bool y", > > > so there is no way to disable it. > > > > > > But, it comes from the nature of Kconfig in general. > > > > > > dummy-tools is completely unrelated here. > > > > dummy-tools makes all configuration options available in order to be > > able to author configuration files on system different from the one > > where the kernel is built. This prevents authoring a configuration file > > with this option disabled. > > > No. > dummy-tools enables as many $(cc-option, ...) > and $(shell, ...) as possible. That's it. > > > In my understanding, STACKPROTECTOR_PER_TASK > should not be user-configurable. > That is why 'def_bool y'. How do you author a specific configuration with dummy-tools when options are allowed to unconditionally follow these options that dummy-tools enables? Thanks Michal
On Wed, Mar 10, 2021 at 04:07:00AM +0900, Masahiro Yamada wrote: > On Wed, Mar 10, 2021 at 12:10 AM Michal Suchánek <msuchanek@suse.de> wrote: > > > > On Tue, Mar 09, 2021 at 11:53:21PM +0900, Masahiro Yamada wrote: > > > On Tue, Mar 9, 2021 at 10:35 PM Michal Suchánek <msuchanek@suse.de> wrote: > > > > > > > > On Tue, Mar 09, 2021 at 10:22:36PM +0900, Masahiro Yamada wrote: > > > > > On Tue, Mar 9, 2021 at 9:35 PM Michal Suchanek <msuchanek@suse.de> wrote: > > > > > > > > > > > > When using dummy-tools STACKPROTECTOR_PER_TASK is unconditionally > > > > > > selected. This defeats the purpose of the all-enabled tool. > > > > > > > > > > > > Description copied from arm > > > > > > > > > > > > Cc: Masahiro Yamada <masahiroy@kernel.org> > > > > > > Signed-off-by: Michal Suchanek <msuchanek@suse.de> > > > > > > > > > > > > > > > Could you explain what problem > > > > > this patch is trying to solve? > > > > > > > > The option cannot be disabled when compiler has the required capability. > > > > > > > > > Yes. > > > Currently, this symbol claims "def_bool y", > > > so there is no way to disable it. > > > > > > But, it comes from the nature of Kconfig in general. > > > > > > dummy-tools is completely unrelated here. > > > > dummy-tools makes all configuration options available in order to be > > able to author configuration files on system different from the one > > where the kernel is built. This prevents authoring a configuration file > > with this option disabled. > > > No. > dummy-tools enables as many $(cc-option, ...) > and $(shell, ...) as possible. That's it. > > > In my understanding, STACKPROTECTOR_PER_TASK > should not be user-configurable. > That is why 'def_bool y'. And that's wrong. Either it's a required copiler feature and then it should be always enabled. Or it is optional and then it should be possible to disable - one or the other. When it is optional you should be able to - (de) select in randconfig - test/bisect the old code path without crippling your toolchain - author configuration file that does not have the option enabled for build on system that uses older toolchain Thanks Michal
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index a8ff7cd5f096..f59d391e31a4 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -1549,9 +1549,20 @@ config RANDOMIZE_MODULE_REGION_FULL config CC_HAVE_STACKPROTECTOR_SYSREG def_bool $(cc-option,-mstack-protector-guard=sysreg -mstack-protector-guard-reg=sp_el0 -mstack-protector-guard-offset=0) + config STACKPROTECTOR_PER_TASK - def_bool y + bool "Use a unique stack canary value for each task" depends on STACKPROTECTOR && CC_HAVE_STACKPROTECTOR_SYSREG + default y + help + Due to the fact that GCC uses an ordinary symbol reference from + which to load the value of the stack canary, this value can only + change at reboot time on SMP systems, and all tasks running in the + kernel's address space are forced to use the same canary value for + the entire duration that the system is up. + + Enable this option to switch to a different method that uses a + different canary value for each task. endmenu
When using dummy-tools STACKPROTECTOR_PER_TASK is unconditionally selected. This defeats the purpose of the all-enabled tool. Description copied from arm Cc: Masahiro Yamada <masahiroy@kernel.org> Signed-off-by: Michal Suchanek <msuchanek@suse.de> --- arch/arm64/Kconfig | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)