Message ID | 20240712113656.30422-1-flyingpeng@tencent.com |
---|---|
State | New |
Headers | show |
Series | crypto/ecc: increase frame warning limit | expand |
(cc Arnd, Nathan) On Fri, 12 Jul 2024 at 13:37, <flyingpenghao@gmail.com> wrote: > > From: Peng Hao <flyingpeng@tencent.com> > > When building kernel with clang, which will typically > have sanitizers enabled, there is a warning about a large stack frame. > > crypto/ecc.c:1129:13: error: stack frame size (2136) exceeds limit (2048) in 'ecc_point_double_jacobian' [-Werror,-Wframe-larger-than] > static void ecc_point_double_jacobian(u64 *x1, u64 *y1, u64 *z1, > ^ > > Since many arrays are defined in ecc_point_double_jacobian, they occupy a > lot of stack space, but are difficult to adjust. just increase the limit > for configurations that have KASAN or KCSAN enabled. > > Signed-off-by: Peng Hao <flyingpeng@tencent.com> > --- > crypto/Makefile | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/crypto/Makefile b/crypto/Makefile > index edbbaa3ffef5..ab7bebaa7218 100644 > --- a/crypto/Makefile > +++ b/crypto/Makefile > @@ -190,6 +190,12 @@ obj-$(CONFIG_CRYPTO_ECC) += ecc.o > obj-$(CONFIG_CRYPTO_ESSIV) += essiv.o > obj-$(CONFIG_CRYPTO_CURVE25519) += curve25519-generic.o > > +ifneq ($(CONFIG_FRAME_WARN),0) > +ifeq ($(filter y,$(CONFIG_KASAN)$(CONFIG_KCSAN)),y) > +CFLAGS_ecc.o = -Wframe-larger-than=2776 > +endif > +endif > + I don't think this is an acceptable workaround - this applies to all functions in the file, which call into each other as well. It would be better if we could figure out why the stack blows up like this on clang, which could be related to inlining behavior or other compiler heuristics that prevent stack allocations from being reused. Arnd, Nathan: any insights?
Hi Ard, On Sat, Jul 13, 2024 at 12:23:24PM +0200, Ard Biesheuvel wrote: > On Fri, 12 Jul 2024 at 13:37, <flyingpenghao@gmail.com> wrote: > > > > From: Peng Hao <flyingpeng@tencent.com> > > > > When building kernel with clang, which will typically > > have sanitizers enabled, there is a warning about a large stack frame. > > > > crypto/ecc.c:1129:13: error: stack frame size (2136) exceeds limit (2048) in 'ecc_point_double_jacobian' [-Werror,-Wframe-larger-than] > > static void ecc_point_double_jacobian(u64 *x1, u64 *y1, u64 *z1, > > ^ > > > > Since many arrays are defined in ecc_point_double_jacobian, they occupy a > > lot of stack space, but are difficult to adjust. just increase the limit > > for configurations that have KASAN or KCSAN enabled. > > > > Signed-off-by: Peng Hao <flyingpeng@tencent.com> > > --- > > crypto/Makefile | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/crypto/Makefile b/crypto/Makefile > > index edbbaa3ffef5..ab7bebaa7218 100644 > > --- a/crypto/Makefile > > +++ b/crypto/Makefile > > @@ -190,6 +190,12 @@ obj-$(CONFIG_CRYPTO_ECC) += ecc.o > > obj-$(CONFIG_CRYPTO_ESSIV) += essiv.o > > obj-$(CONFIG_CRYPTO_CURVE25519) += curve25519-generic.o > > > > +ifneq ($(CONFIG_FRAME_WARN),0) > > +ifeq ($(filter y,$(CONFIG_KASAN)$(CONFIG_KCSAN)),y) > > +CFLAGS_ecc.o = -Wframe-larger-than=2776 > > +endif > > +endif > > + > > I don't think this is an acceptable workaround - this applies to all > functions in the file, which call into each other as well. Agreed, as I stated on a review of another patch this same submitter sent for a similar warning in infiniband: https://lore.kernel.org/20240709212608.GA1649561@thelio-3990X/ Unfortunately, this appears to be getting copy and pasted from AMDGPU, which needed it for their horrible DCN code that comes from some other upstream source. > It would be better if we could figure out why the stack blows up like > this on clang, which could be related to inlining behavior or other > compiler heuristics that prevent stack allocations from being reused. I asked for further details like configuration in my second message in that thread and it never materialized: https://lore.kernel.org/20240709212608.GA1649561@thelio-3990X/ I suspect this might just be a KASAN_STACK warning, which is known to generate high stack usage warnings (as noted in the Kconfig text),we will have to wait for confirmation: https://github.com/ClangBuiltLinux/linux/issues/39 Cheers, Nathan
diff --git a/crypto/Makefile b/crypto/Makefile index edbbaa3ffef5..ab7bebaa7218 100644 --- a/crypto/Makefile +++ b/crypto/Makefile @@ -190,6 +190,12 @@ obj-$(CONFIG_CRYPTO_ECC) += ecc.o obj-$(CONFIG_CRYPTO_ESSIV) += essiv.o obj-$(CONFIG_CRYPTO_CURVE25519) += curve25519-generic.o +ifneq ($(CONFIG_FRAME_WARN),0) +ifeq ($(filter y,$(CONFIG_KASAN)$(CONFIG_KCSAN)),y) +CFLAGS_ecc.o = -Wframe-larger-than=2776 +endif +endif + ecdh_generic-y += ecdh.o ecdh_generic-y += ecdh_helper.o obj-$(CONFIG_CRYPTO_ECDH) += ecdh_generic.o