2021-01-27 03:37:12

by Arnd Bergmann

[permalink] [raw]
Subject: [RFC 0/3] kunit vs structleak

From: Arnd Bergmann <[email protected]>

I ran into a couple of problems with kunit tests taking too much stack
space, sometimes dangerously so. These the the three instances that
cause an increase over the warning limit of some architectures:

lib/bitfield_kunit.c:93:1: error: the frame size of 7440 bytes is larger than 2048 bytes [-Werror=frame-larger-than=]
drivers/base/test/property-entry-test.c:481:1: error: the frame size of 2640 bytes is larger than 2048 bytes [-Werror=frame-larger-than=]
drivers/thunderbolt/test.c:1529:1: error: the frame size of 1176 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]

Ideally there should be a way to rewrite the kunit infrastructure
that avoids the explosion of stack data when the structleak plugin
is used.

A rather drastic measure would be to use Kconfig logic to make
the two options mutually exclusive. This would clearly work, but
is probably not needed.

As a simpler workaround, this disables the plugin for the three
files in which the excessive stack usage was observed.

Arnd

Arnd Bergmann (3):
bitfield: build kunit tests without structleak plugin
drivers/base: build kunit tests without structleak plugin
thunderbolt: build kunit tests without structleak plugin

drivers/base/test/Makefile | 1 +
drivers/thunderbolt/Makefile | 1 +
lib/Makefile | 1 +
3 files changed, 3 insertions(+)

Cc: Kees Cook <[email protected]>
Cc: Brendan Higgins <[email protected]>
Cc: Shuah Khan <[email protected]>
Cc: Geert Uytterhoeven <[email protected]>
Cc: Alan Maguire <[email protected]>
Cc: Dmitry Torokhov <[email protected]>
Cc: Mika Westerberg <[email protected]>
Cc: Vitor Massaru Iha <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
--
2.29.2


2021-01-28 00:24:38

by Kees Cook

[permalink] [raw]
Subject: Re: [RFC 0/3] kunit vs structleak

On Mon, Jan 25, 2021 at 01:45:25PM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann <[email protected]>
>
> I ran into a couple of problems with kunit tests taking too much stack
> space, sometimes dangerously so. These the the three instances that
> cause an increase over the warning limit of some architectures:
>
> lib/bitfield_kunit.c:93:1: error: the frame size of 7440 bytes is larger than 2048 bytes [-Werror=frame-larger-than=]
> drivers/base/test/property-entry-test.c:481:1: error: the frame size of 2640 bytes is larger than 2048 bytes [-Werror=frame-larger-than=]
> drivers/thunderbolt/test.c:1529:1: error: the frame size of 1176 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
>
> Ideally there should be a way to rewrite the kunit infrastructure
> that avoids the explosion of stack data when the structleak plugin
> is used.
>
> A rather drastic measure would be to use Kconfig logic to make
> the two options mutually exclusive. This would clearly work, but
> is probably not needed.
>
> As a simpler workaround, this disables the plugin for the three
> files in which the excessive stack usage was observed.
>
> Arnd
>
> Arnd Bergmann (3):
> bitfield: build kunit tests without structleak plugin
> drivers/base: build kunit tests without structleak plugin
> thunderbolt: build kunit tests without structleak plugin
>
> drivers/base/test/Makefile | 1 +
> drivers/thunderbolt/Makefile | 1 +
> lib/Makefile | 1 +
> 3 files changed, 3 insertions(+)

I think I'd prefer centralizing the disabling, as done with the other
plugins, instead of sprinkling "open coded" command-line options around
the kernel's Makefiles. :)

For example:


diff --git a/scripts/Makefile.gcc-plugins b/scripts/Makefile.gcc-plugins
index 952e46876329..2d5009e3b593 100644
--- a/scripts/Makefile.gcc-plugins
+++ b/scripts/Makefile.gcc-plugins
@@ -21,6 +21,10 @@ gcc-plugin-cflags-$(CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL) \
+= -fplugin-arg-structleak_plugin-byref-all
gcc-plugin-cflags-$(CONFIG_GCC_PLUGIN_STRUCTLEAK) \
+= -DSTRUCTLEAK_PLUGIN
+ifdef CONFIG_GCC_PLUGIN_STRUCTLEAK
+ DISABLE_STRUCTLEAK_PLUGIN += -fplugin-arg-structleak_plugin-disable
+endif
+export DISABLE_STRUCTLEAK_PLUGIN

gcc-plugin-$(CONFIG_GCC_PLUGIN_RANDSTRUCT) += randomize_layout_plugin.so
gcc-plugin-cflags-$(CONFIG_GCC_PLUGIN_RANDSTRUCT) \


And then use DISABLE_STRUCTLEAK_PLUGIN.

--
Kees Cook

2021-01-29 21:31:21

by Brendan Higgins

[permalink] [raw]
Subject: Re: [RFC 0/3] kunit vs structleak

On Wed, Jan 27, 2021 at 12:15 PM Kees Cook <[email protected]> wrote:
>
> On Mon, Jan 25, 2021 at 01:45:25PM +0100, Arnd Bergmann wrote:
> > From: Arnd Bergmann <[email protected]>
> >
> > I ran into a couple of problems with kunit tests taking too much stack
> > space, sometimes dangerously so. These the the three instances that
> > cause an increase over the warning limit of some architectures:
> >
> > lib/bitfield_kunit.c:93:1: error: the frame size of 7440 bytes is larger than 2048 bytes [-Werror=frame-larger-than=]
> > drivers/base/test/property-entry-test.c:481:1: error: the frame size of 2640 bytes is larger than 2048 bytes [-Werror=frame-larger-than=]
> > drivers/thunderbolt/test.c:1529:1: error: the frame size of 1176 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
> >
> > Ideally there should be a way to rewrite the kunit infrastructure
> > that avoids the explosion of stack data when the structleak plugin
> > is used.
> >
> > A rather drastic measure would be to use Kconfig logic to make
> > the two options mutually exclusive. This would clearly work, but
> > is probably not needed.
> >
> > As a simpler workaround, this disables the plugin for the three
> > files in which the excessive stack usage was observed.
> >
> > Arnd
> >
> > Arnd Bergmann (3):
> > bitfield: build kunit tests without structleak plugin
> > drivers/base: build kunit tests without structleak plugin
> > thunderbolt: build kunit tests without structleak plugin
> >
> > drivers/base/test/Makefile | 1 +
> > drivers/thunderbolt/Makefile | 1 +
> > lib/Makefile | 1 +
> > 3 files changed, 3 insertions(+)
>
> I think I'd prefer centralizing the disabling, as done with the other
> plugins, instead of sprinkling "open coded" command-line options around
> the kernel's Makefiles. :)
>
> For example:
>
>
> diff --git a/scripts/Makefile.gcc-plugins b/scripts/Makefile.gcc-plugins
> index 952e46876329..2d5009e3b593 100644
> --- a/scripts/Makefile.gcc-plugins
> +++ b/scripts/Makefile.gcc-plugins
> @@ -21,6 +21,10 @@ gcc-plugin-cflags-$(CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL) \
> += -fplugin-arg-structleak_plugin-byref-all
> gcc-plugin-cflags-$(CONFIG_GCC_PLUGIN_STRUCTLEAK) \
> += -DSTRUCTLEAK_PLUGIN
> +ifdef CONFIG_GCC_PLUGIN_STRUCTLEAK
> + DISABLE_STRUCTLEAK_PLUGIN += -fplugin-arg-structleak_plugin-disable
> +endif
> +export DISABLE_STRUCTLEAK_PLUGIN
>
> gcc-plugin-$(CONFIG_GCC_PLUGIN_RANDSTRUCT) += randomize_layout_plugin.so
> gcc-plugin-cflags-$(CONFIG_GCC_PLUGIN_RANDSTRUCT) \
>
>
> And then use DISABLE_STRUCTLEAK_PLUGIN.

This looks fine to me. Does somebody want me to send this out as a
patch? Don't want to steal anyone's thunder :-)