| Message ID | 20210125124533.101339-1-arnd@kernel.org |
|---|---|
| Headers | show |
| Series | kunit vs structleak | expand |
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. > > 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
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 :-)
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