[PATCH v2] crypto: ecc - Unbreak the build on arm with CONFIG_KASAN_STACK=y

[PATCH v2] crypto: ecc - Unbreak the build on arm with CONFIG_KASAN_STACK=y

[Lukas Wunner]

Andrew reports build breakage of arm allmodconfig, reproducible with gcc
14.2.0 and 15.2.0:

  crypto/ecc.c: In function 'ecc_point_mult':
  crypto/ecc.c:1380:1: error: the frame size of 1360 bytes is larger than 1280 bytes [-Werror=frame-larger-than=]

gcc aggressively inlines functions called by ecc_point_mult() (without
there being any explicit inline declarations), which pushes stack usage
close to the limit imposed by CONFIG_FRAME_WARN.  allmodconfig implies
CONFIG_KASAN_STACK=y, which increases the stack above that limit.

In the bugzilla entry linked below, gcc maintainers explain that gcc
estimates extra stack usage caused by inlining, but ASAN instrumentation
is added in post-IPA passes and thus the inlining heuristics cannot
account for it.

It could be argued that -Werror=frame-larger-than=1280 instructs the
compiler to avoid inlining beyond that limit lest the build breaks,
which would imply gcc behaves incorrectly.  But gcc maintainers reject
this notion and believe that a warning switch should never affect code
generation, even if it is promoted to an error.

One way to unbreak the build is to limit inlining via -finline-limit=100
or by explicitly declaring some functions noinline.  However while it
does keep stack usage of individual functions below the limit, *total*
stack usage increases.

A longterm solution is to refactor ecc.c for reduced stack usage.  It
currently performs ECC point multiplication with a Montgomery ladder
which uses co-Z (conjugate) addition to trade off memory for speed.
The algorithm is susceptible to timing attacks and needs to be replaced
with a constant time Montgomery ladder, which should consume less memory
and thus resolve the stack usage issue as a side effect.

In the interim, raise the limit for ecc.c, as is already done for
several other files in the source tree.

Constrain to gcc because clang 19.1.7 does not exhibit the issue.  It
makes do with a 724 bytes stack frame even though it inlines almost the
same functions as gcc.

Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=124949
Reported-by: Andrew Morton <akpm@linux-foundation.org> # off-list
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
Changes v1 -> v2:
* s/ARCH/CONFIG_ARM/, s/LLVM/CONFIG_CC_IS_GCC/ (Nathan)
* Add link to gcc bugzilla entry
* Rewrite commit message to include feedback provided by gcc maintainers
  and explain high stack usage with algorithm choice

Link to v1:
https://lore.kernel.org/r/abfaede9ab2e963d784fb70598ed74935f7f8d93.1775628469.git.lukas@wunner.de/

 crypto/Makefile | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/crypto/Makefile b/crypto/Makefile
index 1622425..c73f4d5 100644
--- a/crypto/Makefile
+++ b/crypto/Makefile
@@ -178,6 +178,11 @@ obj-$(CONFIG_CRYPTO_ZSTD) += zstd.o
 obj-$(CONFIG_CRYPTO_ECC) += ecc.o
 obj-$(CONFIG_CRYPTO_ESSIV) += essiv.o
 
+# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=124949
+ifeq ($(CONFIG_ARM)$(CONFIG_KASAN_STACK)$(CONFIG_CC_IS_GCC),yyy)
+CFLAGS_ecc.o += $(call cc-option,-Wframe-larger-than=1536)
+endif
+
 ecdh_generic-y += ecdh.o
 ecdh_generic-y += ecdh_helper.o
 obj-$(CONFIG_CRYPTO_ECDH) += ecdh_generic.o
-- 
2.51.0

Re: [PATCH v2] crypto: ecc - Unbreak the build on arm with CONFIG_KASAN_STACK=y

[Andy Shevchenko]

On Wed, May 06, 2026 at 03:27:49PM +0200, Lukas Wunner wrote:
> Andrew reports build breakage of arm allmodconfig, reproducible with gcc
> 14.2.0 and 15.2.0:
> 
>   crypto/ecc.c: In function 'ecc_point_mult':
>   crypto/ecc.c:1380:1: error: the frame size of 1360 bytes is larger than 1280 bytes [-Werror=frame-larger-than=]
> 
> gcc aggressively inlines functions called by ecc_point_mult() (without
> there being any explicit inline declarations), which pushes stack usage
> close to the limit imposed by CONFIG_FRAME_WARN.  allmodconfig implies
> CONFIG_KASAN_STACK=y, which increases the stack above that limit.
> 
> In the bugzilla entry linked below, gcc maintainers explain that gcc
> estimates extra stack usage caused by inlining, but ASAN instrumentation
> is added in post-IPA passes and thus the inlining heuristics cannot
> account for it.
> 
> It could be argued that -Werror=frame-larger-than=1280 instructs the
> compiler to avoid inlining beyond that limit lest the build breaks,
> which would imply gcc behaves incorrectly.  But gcc maintainers reject
> this notion and believe that a warning switch should never affect code
> generation, even if it is promoted to an error.
> 
> One way to unbreak the build is to limit inlining via -finline-limit=100
> or by explicitly declaring some functions noinline.  However while it
> does keep stack usage of individual functions below the limit, *total*
> stack usage increases.
> 
> A longterm solution is to refactor ecc.c for reduced stack usage.  It
> currently performs ECC point multiplication with a Montgomery ladder
> which uses co-Z (conjugate) addition to trade off memory for speed.
> The algorithm is susceptible to timing attacks and needs to be replaced
> with a constant time Montgomery ladder, which should consume less memory
> and thus resolve the stack usage issue as a side effect.
> 
> In the interim, raise the limit for ecc.c, as is already done for
> several other files in the source tree.
> 
> Constrain to gcc because clang 19.1.7 does not exhibit the issue.  It
> makes do with a 724 bytes stack frame even though it inlines almost the
> same functions as gcc.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

...

> +# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=124949

Perhaps also mention the algo change as that one sounds to me even more
critical than this issue per se.

> +ifeq ($(CONFIG_ARM)$(CONFIG_KASAN_STACK)$(CONFIG_CC_IS_GCC),yyy)
> +CFLAGS_ecc.o += $(call cc-option,-Wframe-larger-than=1536)
> +endif

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2] crypto: ecc - Unbreak the build on arm with CONFIG_KASAN_STACK=y

[Lukas Wunner]

On Wed, May 06, 2026 at 04:42:25PM +0300, Andy Shevchenko wrote:
> On Wed, May 06, 2026 at 03:27:49PM +0200, Lukas Wunner wrote:
> > A longterm solution is to refactor ecc.c for reduced stack usage.  It
> > currently performs ECC point multiplication with a Montgomery ladder
> > which uses co-Z (conjugate) addition to trade off memory for speed.
> > The algorithm is susceptible to timing attacks and needs to be replaced
> > with a constant time Montgomery ladder, which should consume less memory
> > and thus resolve the stack usage issue as a side effect.
[...]
> > +# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=124949
> 
> Perhaps also mention the algo change as that one sounds to me even more
> critical than this issue per se.

Hm, but it's already mentioned above in the commit message?

Re: [PATCH v2] crypto: ecc - Unbreak the build on arm with CONFIG_KASAN_STACK=y

[Andy Shevchenko]

On Wed, May 06, 2026 at 03:56:22PM +0200, Lukas Wunner wrote:
> On Wed, May 06, 2026 at 04:42:25PM +0300, Andy Shevchenko wrote:
> > On Wed, May 06, 2026 at 03:27:49PM +0200, Lukas Wunner wrote:
> > > A longterm solution is to refactor ecc.c for reduced stack usage.  It
> > > currently performs ECC point multiplication with a Montgomery ladder
> > > which uses co-Z (conjugate) addition to trade off memory for speed.
> > > The algorithm is susceptible to timing attacks and needs to be replaced
> > > with a constant time Montgomery ladder, which should consume less memory
> > > and thus resolve the stack usage issue as a side effect.

[...]

> > > +# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=124949
> > 
> > Perhaps also mention the algo change as that one sounds to me even more
> > critical than this issue per se.
> 
> Hm, but it's already mentioned above in the commit message?

Commit message != Makefile (or any other in-tree file).

But if you think that this is enough, I am not going to object, it would just
require a few steps to get that from the line in file.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2] crypto: ecc - Unbreak the build on arm with CONFIG_KASAN_STACK=y

[Herbert Xu]

On Wed, May 06, 2026 at 03:27:49PM +0200, Lukas Wunner wrote:
>
> Changes v1 -> v2:
> * s/ARCH/CONFIG_ARM/, s/LLVM/CONFIG_CC_IS_GCC/ (Nathan)
> * Add link to gcc bugzilla entry
> * Rewrite commit message to include feedback provided by gcc maintainers
>   and explain high stack usage with algorithm choice
> 
> Link to v1:
> https://lore.kernel.org/r/abfaede9ab2e963d784fb70598ed74935f7f8d93.1775628469.git.lukas@wunner.de/
> 
>  crypto/Makefile | 5 +++++
>  1 file changed, 5 insertions(+)

Sorry but v1 has already been applied.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH v2] crypto: ecc - Unbreak the build on arm with CONFIG_KASAN_STACK=y

[Andy Shevchenko]

On Thu, May 07, 2026 at 12:26:10PM +0800, Herbert Xu wrote:
> On Wed, May 06, 2026 at 03:27:49PM +0200, Lukas Wunner wrote:
> >
> > Changes v1 -> v2:
> > * s/ARCH/CONFIG_ARM/, s/LLVM/CONFIG_CC_IS_GCC/ (Nathan)
> > * Add link to gcc bugzilla entry
> > * Rewrite commit message to include feedback provided by gcc maintainers
> >   and explain high stack usage with algorithm choice
> > 
> > Link to v1:
> > https://lore.kernel.org/r/abfaede9ab2e963d784fb70598ed74935f7f8d93.1775628469.git.lukas@wunner.de/
> > 
> >  crypto/Makefile | 5 +++++
> >  1 file changed, 5 insertions(+)
> 
> Sorry but v1 has already been applied.

Does it make sense to revert and apply v2?

-- 
With Best Regards,
Andy Shevchenko