The structleak plugin causes the stack frame size to grow immensely when
used with KUnit; this is caused because KUnit allocates lots of
moderately sized structs on the stack as part of its assertion macro
implementation. For most tests with small to moderately sized tests
cases there are never enough KUnit assertions to be an issue at all;
even when a single test cases has many KUnit assertions, the compiler
should never put all these struct allocations on the stack at the same
time since the scope of the structs is so limited; however, the
structleak plugin does not seem to respect the compiler doing the right
thing and will still warn of excessive stack size in some cases.
These patches are not a permanent solution since new tests can be added
with huge test cases, but this serves as a stop gap to stop structleak
from being used on KUnit tests which will currently result in excessive
stack size.
Of the following patches, I think the thunderbolt patch may be
unnecessary since Linus already fixed that test. Additionally, I was not
able to reproduce the error on the sdhci-of-aspeed test. Nevertheless, I
included these tests cases for completeness. Please see my discussion
with Arnd for more context[1].
NOTE: Arnd did the legwork for most of these patches, but did not
actually share code for some of them, so I left his Signed-off-by off of
those patches as I don't want to misrepresent him. Arnd, please sign off
on those patches at your soonest convenience.
[1] https://lore.kernel.org/linux-arm-kernel/CAFd5g44udqkDiYBWh+VeDVJ=ELXeoXwunjv0f9frEN6HJODZng@mail.gmail.com/
Arnd Bergmann (1):
bitfield: build kunit tests without structleak plugin
Brendan Higgins (5):
gcc-plugins/structleak: add makefile var for disabling structleak
iio/test-format: build kunit tests without structleak plugin
device property: build kunit tests without structleak plugin
thunderbolt: build kunit tests without structleak plugin
mmc: sdhci-of-aspeed: build kunit tests without structleak plugin
drivers/base/test/Makefile | 2 +-
drivers/iio/test/Makefile | 1 +
drivers/mmc/host/Makefile | 1 +
drivers/thunderbolt/Makefile | 1 +
lib/Makefile | 2 +-
scripts/Makefile.gcc-plugins | 4 ++++
6 files changed, 9 insertions(+), 2 deletions(-)
base-commit: 316346243be6df12799c0b64b788e06bad97c30b
--
2.33.0.464.g1972c5931b-goog
KUnit and structleak don't play nice, so add a makefile variable for
enabling structleak when it complains.
Co-developed-by: Kees Cook <[email protected]>
Signed-off-by: Brendan Higgins <[email protected]>
---
scripts/Makefile.gcc-plugins | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/scripts/Makefile.gcc-plugins b/scripts/Makefile.gcc-plugins
index 952e46876329a..4aad284800355 100644
--- a/scripts/Makefile.gcc-plugins
+++ b/scripts/Makefile.gcc-plugins
@@ -19,6 +19,10 @@ gcc-plugin-cflags-$(CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF) \
+= -fplugin-arg-structleak_plugin-byref
gcc-plugin-cflags-$(CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL) \
+= -fplugin-arg-structleak_plugin-byref-all
+ifdef CONFIG_GCC_PLUGIN_STRUCTLEAK
+ DISABLE_STRUCTLEAK_PLUGIN += -fplugin-arg-structleak_plugin-disable
+endif
+export DISABLE_STRUCTLEAK_PLUGIN
gcc-plugin-cflags-$(CONFIG_GCC_PLUGIN_STRUCTLEAK) \
+= -DSTRUCTLEAK_PLUGIN
--
2.33.0.464.g1972c5931b-goog
The structleak plugin causes the stack frame size to grow immensely when
used with KUnit:
../drivers/iio/test/iio-test-format.c: In function ‘iio_test_iio_format_value_fixedpoint’:
../drivers/iio/test/iio-test-format.c:98:1: warning: the frame size of 2336 bytes is larger than 2048 bytes [-Wframe-larger-than=]
Turn it off in this file.
Co-developed-by: Arnd Bergmann <[email protected]>
Signed-off-by: Brendan Higgins <[email protected]>
---
drivers/iio/test/Makefile | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/iio/test/Makefile b/drivers/iio/test/Makefile
index f1099b4953014..467519a2027e5 100644
--- a/drivers/iio/test/Makefile
+++ b/drivers/iio/test/Makefile
@@ -5,3 +5,4 @@
# Keep in alphabetical order
obj-$(CONFIG_IIO_TEST_FORMAT) += iio-test-format.o
+CFLAGS_iio-test-format.o += $(DISABLE_STRUCTLEAK_PLUGIN)
--
2.33.0.464.g1972c5931b-goog
The structleak plugin causes the stack frame size to grow immensely when
used with KUnit.
Turn it off.
Co-developed-by: Arnd Bergmann <[email protected]>
Signed-off-by: Brendan Higgins <[email protected]>
---
drivers/mmc/host/Makefile | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
index 14004cc09aaad..2ab083931f8fd 100644
--- a/drivers/mmc/host/Makefile
+++ b/drivers/mmc/host/Makefile
@@ -85,6 +85,7 @@ obj-$(CONFIG_MMC_SDHCI_DOVE) += sdhci-dove.o
obj-$(CONFIG_MMC_SDHCI_TEGRA) += sdhci-tegra.o
obj-$(CONFIG_MMC_SDHCI_OF_ARASAN) += sdhci-of-arasan.o
obj-$(CONFIG_MMC_SDHCI_OF_ASPEED) += sdhci-of-aspeed.o
+CFLAGS_sdhci-of-aspeed.o += $(DISABLE_STRUCTLEAK_PLUGIN)
obj-$(CONFIG_MMC_SDHCI_OF_AT91) += sdhci-of-at91.o
obj-$(CONFIG_MMC_SDHCI_OF_ESDHC) += sdhci-of-esdhc.o
obj-$(CONFIG_MMC_SDHCI_OF_HLWD) += sdhci-of-hlwd.o
--
2.33.0.464.g1972c5931b-goog
The structleak plugin causes the stack frame size to grow immensely when
used with KUnit:
../drivers/base/test/property-entry-test.c:492:1: warning: the frame size of 2832 bytes is larger than 2048 bytes [-Wframe-larger-than=]
../drivers/base/test/property-entry-test.c:322:1: warning: the frame size of 2080 bytes is larger than 2048 bytes [-Wframe-larger-than=]
../drivers/base/test/property-entry-test.c:250:1: warning: the frame size of 4976 bytes is larger than 2048 bytes [-Wframe-larger-than=]
../drivers/base/test/property-entry-test.c:115:1: warning: the frame size of 3280 bytes is larger than 2048 bytes [-Wframe-larger-than=]
Turn it off in this file.
Co-developed-by: Arnd Bergmann <[email protected]>
Signed-off-by: Brendan Higgins <[email protected]>
---
drivers/base/test/Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/base/test/Makefile b/drivers/base/test/Makefile
index 64b2f3d744d51..7f76fee6f989d 100644
--- a/drivers/base/test/Makefile
+++ b/drivers/base/test/Makefile
@@ -2,4 +2,4 @@
obj-$(CONFIG_TEST_ASYNC_DRIVER_PROBE) += test_async_driver_probe.o
obj-$(CONFIG_DRIVER_PE_KUNIT_TEST) += property-entry-test.o
-CFLAGS_REMOVE_property-entry-test.o += -fplugin-arg-structleak_plugin-byref -fplugin-arg-structleak_plugin-byref-all
+CFLAGS_property-entry-test.o += $(DISABLE_STRUCTLEAK_PLUGIN)
--
2.33.0.464.g1972c5931b-goog
The structleak plugin causes the stack frame size to grow immensely when
used with KUnit:
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.
Linus already split up tests in this file, so this change *should* be
redundant now.
Co-developed-by: Arnd Bergmann <[email protected]>
Signed-off-by: Arnd Bergmann <[email protected]>
Signed-off-by: Brendan Higgins <[email protected]>
---
drivers/thunderbolt/Makefile | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/thunderbolt/Makefile b/drivers/thunderbolt/Makefile
index da19d7987d005..78fd365893c13 100644
--- a/drivers/thunderbolt/Makefile
+++ b/drivers/thunderbolt/Makefile
@@ -7,6 +7,7 @@ thunderbolt-objs += usb4_port.o 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_test.o += $(DISABLE_STRUCTLEAK_PLUGIN)
thunderbolt_dma_test-${CONFIG_USB4_DMA_TEST} += dma_test.o
obj-$(CONFIG_USB4_DMA_TEST) += thunderbolt_dma_test.o
--
2.33.0.464.g1972c5931b-goog
On Thu, Sep 16, 2021 at 11:11:00PM -0700, Brendan Higgins wrote:
> The structleak plugin causes the stack frame size to grow immensely when
> used with KUnit:
>
> ../drivers/iio/test/iio-test-format.c: In function ‘iio_test_iio_format_value_fixedpoint’:
> ../drivers/iio/test/iio-test-format.c:98:1: warning: the frame size of 2336 bytes is larger than 2048 bytes [-Wframe-larger-than=]
>
> Turn it off in this file.
Given that these are all for KUnit tests, is it possible there are going
to be other CONFIGs that will interact poorly (e.g. KASAN)? Maybe there
needs to be a small level of indirection with something like:
DISABLE_UNDER_KUNIT := $(DISABLE_STRUCTLEAK_PLUGIN)
export DISABLE_UNDER_KUNIT
then all of these become:
+CFLAGS_iio-test-format.o += $(DISABLE_UNDER_KUNIT)
Either way, I think these are fine to add.
Reviewed-by: Kees Cook <[email protected]>
>
> Co-developed-by: Arnd Bergmann <[email protected]>
> Signed-off-by: Brendan Higgins <[email protected]>
> ---
> drivers/iio/test/Makefile | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/iio/test/Makefile b/drivers/iio/test/Makefile
> index f1099b4953014..467519a2027e5 100644
> --- a/drivers/iio/test/Makefile
> +++ b/drivers/iio/test/Makefile
> @@ -5,3 +5,4 @@
>
> # Keep in alphabetical order
> obj-$(CONFIG_IIO_TEST_FORMAT) += iio-test-format.o
> +CFLAGS_iio-test-format.o += $(DISABLE_STRUCTLEAK_PLUGIN)
> --
> 2.33.0.464.g1972c5931b-goog
>
--
Kees Cook
On Fri, Sep 17, 2021 at 8:57 AM Kees Cook <[email protected]> wrote:
>
> This isn't a stand-alone test object, so I'm less excited about
> disabling STRUCTLEAK here.
Yeah, please don't do this for things that aren't pure tests. You're
now disabling security measures (even if I hate the gcc plugins and
hope they will go away).
Linus
On Thu, Sep 16, 2021 at 11:11:02PM -0700, Brendan Higgins wrote:
> The structleak plugin causes the stack frame size to grow immensely when
> used with KUnit:
>
> 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.
>
> Linus already split up tests in this file, so this change *should* be
> redundant now.
>
> Co-developed-by: Arnd Bergmann <[email protected]>
> Signed-off-by: Arnd Bergmann <[email protected]>
Reviewed-by: Kees Cook <[email protected]>
--
Kees Cook
On Thu, Sep 16, 2021 at 11:11:01PM -0700, Brendan Higgins wrote:
> The structleak plugin causes the stack frame size to grow immensely when
> used with KUnit:
>
> ../drivers/base/test/property-entry-test.c:492:1: warning: the frame size of 2832 bytes is larger than 2048 bytes [-Wframe-larger-than=]
> ../drivers/base/test/property-entry-test.c:322:1: warning: the frame size of 2080 bytes is larger than 2048 bytes [-Wframe-larger-than=]
> ../drivers/base/test/property-entry-test.c:250:1: warning: the frame size of 4976 bytes is larger than 2048 bytes [-Wframe-larger-than=]
> ../drivers/base/test/property-entry-test.c:115:1: warning: the frame size of 3280 bytes is larger than 2048 bytes [-Wframe-larger-than=]
>
> Turn it off in this file.
>
> Co-developed-by: Arnd Bergmann <[email protected]>
> Signed-off-by: Brendan Higgins <[email protected]>
Reviewed-by: Kees Cook <[email protected]>
--
Kees Cook
On Thu, Sep 16, 2021 at 11:11:03PM -0700, Brendan Higgins wrote:
> The structleak plugin causes the stack frame size to grow immensely when
> used with KUnit.
>
> Turn it off.
>
> Co-developed-by: Arnd Bergmann <[email protected]>
> Signed-off-by: Brendan Higgins <[email protected]>
> ---
> drivers/mmc/host/Makefile | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
> index 14004cc09aaad..2ab083931f8fd 100644
> --- a/drivers/mmc/host/Makefile
> +++ b/drivers/mmc/host/Makefile
> @@ -85,6 +85,7 @@ obj-$(CONFIG_MMC_SDHCI_DOVE) += sdhci-dove.o
> obj-$(CONFIG_MMC_SDHCI_TEGRA) += sdhci-tegra.o
> obj-$(CONFIG_MMC_SDHCI_OF_ARASAN) += sdhci-of-arasan.o
> obj-$(CONFIG_MMC_SDHCI_OF_ASPEED) += sdhci-of-aspeed.o
> +CFLAGS_sdhci-of-aspeed.o += $(DISABLE_STRUCTLEAK_PLUGIN)
This isn't a stand-alone test object, so I'm less excited about
disabling STRUCTLEAK here.
> obj-$(CONFIG_MMC_SDHCI_OF_AT91) += sdhci-of-at91.o
> obj-$(CONFIG_MMC_SDHCI_OF_ESDHC) += sdhci-of-esdhc.o
> obj-$(CONFIG_MMC_SDHCI_OF_HLWD) += sdhci-of-hlwd.o
> --
> 2.33.0.464.g1972c5931b-goog
>
--
Kees Cook
On Thu, 16 Sep 2021 23:11:00 -0700
Brendan Higgins <[email protected]> wrote:
> The structleak plugin causes the stack frame size to grow immensely when
> used with KUnit:
>
> ../drivers/iio/test/iio-test-format.c: In function ‘iio_test_iio_format_value_fixedpoint’:
> ../drivers/iio/test/iio-test-format.c:98:1: warning: the frame size of 2336 bytes is larger than 2048 bytes [-Wframe-larger-than=]
>
> Turn it off in this file.
>
> Co-developed-by: Arnd Bergmann <[email protected]>
> Signed-off-by: Brendan Higgins <[email protected]>
Acked-by: Jonathan Cameron <[email protected]>
> ---
> drivers/iio/test/Makefile | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/iio/test/Makefile b/drivers/iio/test/Makefile
> index f1099b4953014..467519a2027e5 100644
> --- a/drivers/iio/test/Makefile
> +++ b/drivers/iio/test/Makefile
> @@ -5,3 +5,4 @@
>
> # Keep in alphabetical order
> obj-$(CONFIG_IIO_TEST_FORMAT) += iio-test-format.o
> +CFLAGS_iio-test-format.o += $(DISABLE_STRUCTLEAK_PLUGIN)
On Fri, Sep 17, 2021 at 8:54 AM Kees Cook <[email protected]> wrote:
>
> On Thu, Sep 16, 2021 at 11:11:00PM -0700, Brendan Higgins wrote:
> > The structleak plugin causes the stack frame size to grow immensely when
> > used with KUnit:
> >
> > ../drivers/iio/test/iio-test-format.c: In function ‘iio_test_iio_format_value_fixedpoint’:
> > ../drivers/iio/test/iio-test-format.c:98:1: warning: the frame size of 2336 bytes is larger than 2048 bytes [-Wframe-larger-than=]
> >
> > Turn it off in this file.
>
> Given that these are all for KUnit tests, is it possible there are going
> to be other CONFIGs that will interact poorly (e.g. KASAN)? Maybe there
> needs to be a small level of indirection with something like:
I don't think so. KASAN actually uses KUnit to test it, and we have
experimented running KASAN alongside other KUnit tests for added
protection and results have looked good.
I would be surprised if there are other CONFIGs other than things
dealing with stack size that don't like KUnit.
> DISABLE_UNDER_KUNIT := $(DISABLE_STRUCTLEAK_PLUGIN)
> export DISABLE_UNDER_KUNIT
>
> then all of these become:
>
> +CFLAGS_iio-test-format.o += $(DISABLE_UNDER_KUNIT)
>
> Either way, I think these are fine to add.
I like your suggestion if we find other configs that don't like KUnit,
but I don't think we have seen any others so far, so I think I will
keep it as it is for now.
> Reviewed-by: Kees Cook <[email protected]>
>
>
> >
> > Co-developed-by: Arnd Bergmann <[email protected]>
> > Signed-off-by: Brendan Higgins <[email protected]>
> > ---
> > drivers/iio/test/Makefile | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/iio/test/Makefile b/drivers/iio/test/Makefile
> > index f1099b4953014..467519a2027e5 100644
> > --- a/drivers/iio/test/Makefile
> > +++ b/drivers/iio/test/Makefile
> > @@ -5,3 +5,4 @@
> >
> > # Keep in alphabetical order
> > obj-$(CONFIG_IIO_TEST_FORMAT) += iio-test-format.o
> > +CFLAGS_iio-test-format.o += $(DISABLE_STRUCTLEAK_PLUGIN)
> > --
> > 2.33.0.464.g1972c5931b-goog
> >
>
> --
> Kees Cook
On Fri, Sep 17, 2021 at 11:40 AM Linus Torvalds
<[email protected]> wrote:
>
> On Fri, Sep 17, 2021 at 8:57 AM Kees Cook <[email protected]> wrote:
> >
> > This isn't a stand-alone test object, so I'm less excited about
> > disabling STRUCTLEAK here.
>
> Yeah, please don't do this for things that aren't pure tests. You're
> now disabling security measures (even if I hate the gcc plugins and
> hope they will go away).
Oh, whoops, yeah, I shouldn't do that. I am just going to drop this
patch entirely, as I wasn't able to reproduce the stack frame size
issue on qemu anyway (as I mentioned on the cover letter).
Thanks for catching this.
Sorry!