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
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
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
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
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 :)
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.
ArndArnd 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.
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.
ArndArnd 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 :-)
linux-kselftest-mirror@lists.linaro.org
-
Arnd Bergmann -
Brendan Higgins -
Kees Cook -
Mika Westerberg