[RFC 0/3] kunit vs structleak

[RFC 0/3] kunit vs structleak

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

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 <keescook@chromium.org>
Cc: Brendan Higgins <brendanhiggins@google.com>
Cc: Shuah Khan <skhan@linuxfoundation.org>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Alan Maguire <alan.maguire@oracle.com>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
Cc: Vitor Massaru Iha <vitor@massaru.org>
Cc: linux-hardening@vger.kernel.org
Cc: linux-kselftest@vger.kernel.org
Cc: kunit-dev@googlegroups.com
Cc: linux-kernel@vger.kernel.org
-- 
2.29.2

[RFC 1/3] bitfield: build kunit tests without structleak plugin

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

The structleak plugin causes the stack frame size to grow immensely:

lib/bitfield_kunit.c: In function 'test_bitfields_constants':
lib/bitfield_kunit.c:93:1: error: the frame size of 7440 bytes is larger than 2048 bytes [-Werror=frame-larger-than=]

Turn it off in this file.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 lib/Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/Makefile b/lib/Makefile
index b5307d3eec1a..6a7300f0f02f 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -348,6 +348,7 @@ obj-$(CONFIG_PLDMFW) += pldmfw/
 
 # KUnit tests
 obj-$(CONFIG_BITFIELD_KUNIT) += bitfield_kunit.o
+CFLAGS_REMOVE_bitfield_kunit.o += -fplugin-arg-structleak_plugin-byref -fplugin-arg-structleak_plugin-byref-all
 obj-$(CONFIG_LIST_KUNIT_TEST) += list-test.o
 obj-$(CONFIG_LINEAR_RANGES_TEST) += test_linear_ranges.o
 obj-$(CONFIG_BITS_TEST) += test_bits.o
-- 
2.29.2

[RFC 2/3] drivers/base: build kunit tests without structleak plugin

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

The structleak plugin causes the stack frame size to grow immensely:

drivers/base/test/property-entry-test.c: In function 'pe_test_reference':
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=]
  481 | }
      | ^
drivers/base/test/property-entry-test.c: In function 'pe_test_uints':
drivers/base/test/property-entry-test.c:99:1: error: the frame size of 2592 bytes is larger than 2048 bytes [-Werror=frame-larger-than=]

Turn it off in this file.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/base/test/Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/base/test/Makefile b/drivers/base/test/Makefile
index 3ca56367c84b..2f15fae8625f 100644
--- a/drivers/base/test/Makefile
+++ b/drivers/base/test/Makefile
@@ -2,3 +2,4 @@
 obj-$(CONFIG_TEST_ASYNC_DRIVER_PROBE)	+= test_async_driver_probe.o
 
 obj-$(CONFIG_KUNIT_DRIVER_PE_TEST) += property-entry-test.o
+CFLAGS_REMOVE_property-entry-test.o += -fplugin-arg-structleak_plugin-byref -fplugin-arg-structleak_plugin-byref-all
-- 
2.29.2

[RFC 3/3] thunderbolt: build kunit tests without structleak plugin

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

The structleak plugin causes the stack frame size to grow immensely:

drivers/thunderbolt/test.c:1529:1: error: the frame size of 1176 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]

Turn it off in this file.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/thunderbolt/Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/thunderbolt/Makefile b/drivers/thunderbolt/Makefile
index 7aa48f6c41d9..e571c0495a84 100644
--- a/drivers/thunderbolt/Makefile
+++ b/drivers/thunderbolt/Makefile
@@ -7,6 +7,7 @@ thunderbolt-objs += nvm.o retimer.o quirks.o
 thunderbolt-${CONFIG_ACPI} += acpi.o
 thunderbolt-$(CONFIG_DEBUG_FS) += debugfs.o
 thunderbolt-${CONFIG_USB4_KUNIT_TEST} += test.o
+CFLAGS_REMOVE_test.o += -fplugin-arg-structleak_plugin-byref -fplugin-arg-structleak_plugin-byref-all
 
 thunderbolt_dma_test-${CONFIG_USB4_DMA_TEST} += dma_test.o
 obj-$(CONFIG_USB4_DMA_TEST) += thunderbolt_dma_test.o
-- 
2.29.2

Re: [RFC 3/3] thunderbolt: build kunit tests without structleak plugin

[Mika Westerberg]

Hi Arnd,

On Mon, Jan 25, 2021 at 01:45:28PM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> The structleak plugin causes the stack frame size to grow immensely:
> 
> drivers/thunderbolt/test.c:1529:1: error: the frame size of 1176 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
> 
> Turn it off in this file.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

To me this is a reasonable work around so I can pick this up to
Thunderbolt tree if no objections.

Thanks BTW, for doing this. I got a report from buildbot some time ago
about the this but did not have time to figure out how to fix it :)

Re: [RFC 0/3] kunit vs structleak

[Kees Cook]

On Mon, Jan 25, 2021 at 01:45:25PM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> 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

Re: [RFC 0/3] kunit vs structleak

[Brendan Higgins]

On Wed, Jan 27, 2021 at 12:15 PM Kees Cook <keescook@chromium.org> wrote:
>
> On Mon, Jan 25, 2021 at 01:45:25PM +0100, Arnd Bergmann wrote:
> > From: Arnd Bergmann <arnd@arndb.de>
> >
> > 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 :-)