* lib/crypto/chacha.c:65:1: warning: the frame size of 1604 bytes is larger than 1024 bytes
@ 2020-08-27 3:52 kernel test robot
2020-08-27 8:05 ` Herbert Xu
0 siblings, 1 reply; 23+ messages in thread
From: kernel test robot @ 2020-08-27 3:52 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: kbuild-all, linux-kernel, Herbert Xu
[-- Attachment #1: Type: text/plain, Size: 6629 bytes --]
Hi Ard,
First bad commit (maybe != root cause):
tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
head: 15bc20c6af4ceee97a1f90b43c0e386643c071b4
commit: 5fb8ef25803ef33e2eb60b626435828b937bed75 crypto: chacha - move existing library code into lib/crypto
date: 9 months ago
config: i386-randconfig-r015-20200827 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce (this is a W=1 build):
git checkout 5fb8ef25803ef33e2eb60b626435828b937bed75
# save the attached .config to linux build tree
make W=1 ARCH=i386
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
lib/crypto/chacha.c: In function 'chacha_permute':
>> lib/crypto/chacha.c:65:1: warning: the frame size of 1604 bytes is larger than 1024 bytes [-Wframe-larger-than=]
65 | }
| ^
# https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5fb8ef25803ef33e2eb60b626435828b937bed75
git remote add linus https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
git fetch --no-tags linus master
git checkout 5fb8ef25803ef33e2eb60b626435828b937bed75
vim +65 lib/crypto/chacha.c
e192be9d9a3055 lib/chacha20.c Theodore Ts'o 2016-06-12 16
1ca1b917940c24 lib/chacha.c Eric Biggers 2018-11-16 17 static void chacha_permute(u32 *x, int nrounds)
e192be9d9a3055 lib/chacha20.c Theodore Ts'o 2016-06-12 18 {
e192be9d9a3055 lib/chacha20.c Theodore Ts'o 2016-06-12 19 int i;
e192be9d9a3055 lib/chacha20.c Theodore Ts'o 2016-06-12 20
1ca1b917940c24 lib/chacha.c Eric Biggers 2018-11-16 21 /* whitelist the allowed round counts */
aa7624093cb7fb lib/chacha.c Eric Biggers 2018-11-16 22 WARN_ON_ONCE(nrounds != 20 && nrounds != 12);
1ca1b917940c24 lib/chacha.c Eric Biggers 2018-11-16 23
1ca1b917940c24 lib/chacha.c Eric Biggers 2018-11-16 24 for (i = 0; i < nrounds; i += 2) {
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 25 x[0] += x[4]; x[12] = rol32(x[12] ^ x[0], 16);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 26 x[1] += x[5]; x[13] = rol32(x[13] ^ x[1], 16);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 27 x[2] += x[6]; x[14] = rol32(x[14] ^ x[2], 16);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 28 x[3] += x[7]; x[15] = rol32(x[15] ^ x[3], 16);
e192be9d9a3055 lib/chacha20.c Theodore Ts'o 2016-06-12 29
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 30 x[8] += x[12]; x[4] = rol32(x[4] ^ x[8], 12);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 31 x[9] += x[13]; x[5] = rol32(x[5] ^ x[9], 12);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 32 x[10] += x[14]; x[6] = rol32(x[6] ^ x[10], 12);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 33 x[11] += x[15]; x[7] = rol32(x[7] ^ x[11], 12);
e192be9d9a3055 lib/chacha20.c Theodore Ts'o 2016-06-12 34
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 35 x[0] += x[4]; x[12] = rol32(x[12] ^ x[0], 8);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 36 x[1] += x[5]; x[13] = rol32(x[13] ^ x[1], 8);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 37 x[2] += x[6]; x[14] = rol32(x[14] ^ x[2], 8);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 38 x[3] += x[7]; x[15] = rol32(x[15] ^ x[3], 8);
e192be9d9a3055 lib/chacha20.c Theodore Ts'o 2016-06-12 39
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 40 x[8] += x[12]; x[4] = rol32(x[4] ^ x[8], 7);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 41 x[9] += x[13]; x[5] = rol32(x[5] ^ x[9], 7);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 42 x[10] += x[14]; x[6] = rol32(x[6] ^ x[10], 7);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 43 x[11] += x[15]; x[7] = rol32(x[7] ^ x[11], 7);
e192be9d9a3055 lib/chacha20.c Theodore Ts'o 2016-06-12 44
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 45 x[0] += x[5]; x[15] = rol32(x[15] ^ x[0], 16);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 46 x[1] += x[6]; x[12] = rol32(x[12] ^ x[1], 16);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 47 x[2] += x[7]; x[13] = rol32(x[13] ^ x[2], 16);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 48 x[3] += x[4]; x[14] = rol32(x[14] ^ x[3], 16);
e192be9d9a3055 lib/chacha20.c Theodore Ts'o 2016-06-12 49
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 50 x[10] += x[15]; x[5] = rol32(x[5] ^ x[10], 12);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 51 x[11] += x[12]; x[6] = rol32(x[6] ^ x[11], 12);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 52 x[8] += x[13]; x[7] = rol32(x[7] ^ x[8], 12);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 53 x[9] += x[14]; x[4] = rol32(x[4] ^ x[9], 12);
e192be9d9a3055 lib/chacha20.c Theodore Ts'o 2016-06-12 54
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 55 x[0] += x[5]; x[15] = rol32(x[15] ^ x[0], 8);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 56 x[1] += x[6]; x[12] = rol32(x[12] ^ x[1], 8);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 57 x[2] += x[7]; x[13] = rol32(x[13] ^ x[2], 8);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 58 x[3] += x[4]; x[14] = rol32(x[14] ^ x[3], 8);
e192be9d9a3055 lib/chacha20.c Theodore Ts'o 2016-06-12 59
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 60 x[10] += x[15]; x[5] = rol32(x[5] ^ x[10], 7);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 61 x[11] += x[12]; x[6] = rol32(x[6] ^ x[11], 7);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 62 x[8] += x[13]; x[7] = rol32(x[7] ^ x[8], 7);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 63 x[9] += x[14]; x[4] = rol32(x[4] ^ x[9], 7);
e192be9d9a3055 lib/chacha20.c Theodore Ts'o 2016-06-12 64 }
dd333449d0fb66 lib/chacha20.c Eric Biggers 2018-11-16 @65 }
dd333449d0fb66 lib/chacha20.c Eric Biggers 2018-11-16 66
:::::: The code at line 65 was first introduced by commit
:::::: dd333449d0fb667c5250c42488a7e90470e16c77 crypto: chacha20-generic - add HChaCha20 library function
:::::: TO: Eric Biggers <ebiggers@google.com>
:::::: CC: Herbert Xu <herbert@gondor.apana.org.au>
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 34663 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: lib/crypto/chacha.c:65:1: warning: the frame size of 1604 bytes is larger than 1024 bytes
2020-08-27 3:52 lib/crypto/chacha.c:65:1: warning: the frame size of 1604 bytes is larger than 1024 bytes kernel test robot
@ 2020-08-27 8:05 ` Herbert Xu
2020-08-27 8:10 ` Ard Biesheuvel
0 siblings, 1 reply; 23+ messages in thread
From: Herbert Xu @ 2020-08-27 8:05 UTC (permalink / raw)
To: kernel test robot, Peter Oberparleiter, Kees Cook,
Andrey Ryabinin, Linus Torvalds
Cc: Ard Biesheuvel, kbuild-all, linux-kernel, Linux Crypto Mailing List
On Thu, Aug 27, 2020 at 11:52:50AM +0800, kernel test robot wrote:
>
> First bad commit (maybe != root cause):
>
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> head: 15bc20c6af4ceee97a1f90b43c0e386643c071b4
> commit: 5fb8ef25803ef33e2eb60b626435828b937bed75 crypto: chacha - move existing library code into lib/crypto
> date: 9 months ago
> config: i386-randconfig-r015-20200827 (attached as .config)
> compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
> reproduce (this is a W=1 build):
> git checkout 5fb8ef25803ef33e2eb60b626435828b937bed75
> # save the attached .config to linux build tree
> make W=1 ARCH=i386
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
>
> All warnings (new ones prefixed by >>):
>
> lib/crypto/chacha.c: In function 'chacha_permute':
> >> lib/crypto/chacha.c:65:1: warning: the frame size of 1604 bytes is larger than 1024 bytes [-Wframe-larger-than=]
> 65 | }
> | ^
This doesn't happen with a normal configuration. To recreate
this warning, you need to enable both GCOV_KERNEL and UBSAN.
This is the minimal gcc command-line to recreate it:
gcc -Wframe-larger-than=1024 -fprofile-arcs -fsanitize=object-size -c -O2 chacha.c
If you take away either profile-arcs or sanitize=object-size then
the problem goes away.
Any suggestions?
Thanks,
--
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: lib/crypto/chacha.c:65:1: warning: the frame size of 1604 bytes is larger than 1024 bytes
2020-08-27 8:05 ` Herbert Xu
@ 2020-08-27 8:10 ` Ard Biesheuvel
2020-08-27 8:24 ` Herbert Xu
2020-08-27 8:33 ` Arnd Bergmann
0 siblings, 2 replies; 23+ messages in thread
From: Ard Biesheuvel @ 2020-08-27 8:10 UTC (permalink / raw)
To: Herbert Xu, Arnd Bergmann
Cc: kernel test robot, Peter Oberparleiter, Kees Cook,
Andrey Ryabinin, Linus Torvalds, kbuild-all,
Linux Kernel Mailing List, Linux Crypto Mailing List
(+ Arnd)
On Thu, 27 Aug 2020 at 10:06, Herbert Xu <herbert@gondor.apana.org.au> wrote:
>
> On Thu, Aug 27, 2020 at 11:52:50AM +0800, kernel test robot wrote:
> >
> > First bad commit (maybe != root cause):
> >
> > tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> > head: 15bc20c6af4ceee97a1f90b43c0e386643c071b4
> > commit: 5fb8ef25803ef33e2eb60b626435828b937bed75 crypto: chacha - move existing library code into lib/crypto
> > date: 9 months ago
> > config: i386-randconfig-r015-20200827 (attached as .config)
> > compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
> > reproduce (this is a W=1 build):
> > git checkout 5fb8ef25803ef33e2eb60b626435828b937bed75
> > # save the attached .config to linux build tree
> > make W=1 ARCH=i386
> >
> > If you fix the issue, kindly add following tag as appropriate
> > Reported-by: kernel test robot <lkp@intel.com>
> >
> > All warnings (new ones prefixed by >>):
> >
> > lib/crypto/chacha.c: In function 'chacha_permute':
> > >> lib/crypto/chacha.c:65:1: warning: the frame size of 1604 bytes is larger than 1024 bytes [-Wframe-larger-than=]
> > 65 | }
> > | ^
>
> This doesn't happen with a normal configuration. To recreate
> this warning, you need to enable both GCOV_KERNEL and UBSAN.
>
> This is the minimal gcc command-line to recreate it:
>
> gcc -Wframe-larger-than=1024 -fprofile-arcs -fsanitize=object-size -c -O2 chacha.c
>
> If you take away either profile-arcs or sanitize=object-size then
> the problem goes away.
>
> Any suggestions?
>
Is it really worth it to obsess about this? Special compiler
instrumentation simply leads to a larger stack footprint in many
cases, which is why we use a larger stack to begin with (at least we
do so for Kasan, so if we don't for Ubsan, we should consider it)
Past experience also shows that this is highly dependent on the exact
compiler version, so issues like these are often moving targets.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: lib/crypto/chacha.c:65:1: warning: the frame size of 1604 bytes is larger than 1024 bytes
2020-08-27 8:10 ` Ard Biesheuvel
@ 2020-08-27 8:24 ` Herbert Xu
2020-08-27 17:34 ` Linus Torvalds
2020-08-27 8:33 ` Arnd Bergmann
1 sibling, 1 reply; 23+ messages in thread
From: Herbert Xu @ 2020-08-27 8:24 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Arnd Bergmann, kernel test robot, Peter Oberparleiter, Kees Cook,
Andrey Ryabinin, Linus Torvalds, kbuild-all,
Linux Kernel Mailing List, Linux Crypto Mailing List
On Thu, Aug 27, 2020 at 10:10:19AM +0200, Ard Biesheuvel wrote:
>
> Is it really worth it to obsess about this? Special compiler
> instrumentation simply leads to a larger stack footprint in many
> cases, which is why we use a larger stack to begin with (at least we
> do so for Kasan, so if we don't for Ubsan, we should consider it)
Perhaps the stack frame warning should be disabled if both GCOV and
UBSAN are on.
> Past experience also shows that this is highly dependent on the exact
> compiler version, so issues like these are often moving targets.
Interestingly this particular file fails with those options on
gcc 8, 9 and 10.
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: lib/crypto/chacha.c:65:1: warning: the frame size of 1604 bytes is larger than 1024 bytes
2020-08-27 8:24 ` Herbert Xu
@ 2020-08-27 17:34 ` Linus Torvalds
2020-08-27 17:55 ` Linus Torvalds
0 siblings, 1 reply; 23+ messages in thread
From: Linus Torvalds @ 2020-08-27 17:34 UTC (permalink / raw)
To: Herbert Xu
Cc: Ard Biesheuvel, Arnd Bergmann, kernel test robot,
Peter Oberparleiter, Kees Cook, Andrey Ryabinin, kbuild-all,
Linux Kernel Mailing List, Linux Crypto Mailing List
On Thu, Aug 27, 2020 at 1:25 AM Herbert Xu <herbert@gondor.apana.org.au> wrote:
>
> Interestingly this particular file fails with those options on
> gcc 8, 9 and 10.
How are you guys testing? I have UBSAN and GCOV on, and don't see
crazy frames on either i386 or x86-64.
I see 72 bytes and 64 bytes respectively for chacha_permute() (plus
the register pushes, which is about the same size)
Linus
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: lib/crypto/chacha.c:65:1: warning: the frame size of 1604 bytes is larger than 1024 bytes
2020-08-27 17:34 ` Linus Torvalds
@ 2020-08-27 17:55 ` Linus Torvalds
2020-08-27 18:42 ` Kees Cook
2020-08-27 19:11 ` Arnd Bergmann
0 siblings, 2 replies; 23+ messages in thread
From: Linus Torvalds @ 2020-08-27 17:55 UTC (permalink / raw)
To: Herbert Xu
Cc: Ard Biesheuvel, Arnd Bergmann, kernel test robot,
Peter Oberparleiter, Kees Cook, Andrey Ryabinin, kbuild-all,
Linux Kernel Mailing List, Linux Crypto Mailing List
On Thu, Aug 27, 2020 at 10:34 AM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> How are you guys testing? I have UBSAN and GCOV on, and don't see
> crazy frames on either i386 or x86-64.
Oh, never mind. I also have COMPILE_TEST on, so it ends up disabling
GCOV_PROFILE_ALL and UBSAN_SANITIZE_ALL.
And yeah, this seems to be a gcc bug. It generates a ton of stack
slots for temporaries. It's -fsanitize=object-size that seems to do
it.
And "-fstack-reuse=all" doesn't seem to make any difference.
So I think
(a) our stack size check is good to catch this
(b) gcc and -fsanitize=object-size is basically an unusable combination
and it's not a bug in the kernel.
Linus
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: lib/crypto/chacha.c:65:1: warning: the frame size of 1604 bytes is larger than 1024 bytes
2020-08-27 17:55 ` Linus Torvalds
@ 2020-08-27 18:42 ` Kees Cook
2020-08-27 19:02 ` Linus Torvalds
2020-08-27 19:11 ` Arnd Bergmann
1 sibling, 1 reply; 23+ messages in thread
From: Kees Cook @ 2020-08-27 18:42 UTC (permalink / raw)
To: Linus Torvalds
Cc: Herbert Xu, Ard Biesheuvel, Arnd Bergmann, kernel test robot,
Peter Oberparleiter, Andrey Ryabinin, kbuild-all,
Linux Kernel Mailing List, Linux Crypto Mailing List
On Thu, Aug 27, 2020 at 10:55:32AM -0700, Linus Torvalds wrote:
> On Thu, Aug 27, 2020 at 10:34 AM Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> > How are you guys testing? I have UBSAN and GCOV on, and don't see
> > crazy frames on either i386 or x86-64.
>
> Oh, never mind. I also have COMPILE_TEST on, so it ends up disabling
> GCOV_PROFILE_ALL and UBSAN_SANITIZE_ALL.
>
> And yeah, this seems to be a gcc bug. It generates a ton of stack
> slots for temporaries. It's -fsanitize=object-size that seems to do
> it.
>
> And "-fstack-reuse=all" doesn't seem to make any difference.
>
> So I think
>
> (a) our stack size check is good to catch this
>
> (b) gcc and -fsanitize=object-size is basically an unusable combination
>
> and it's not a bug in the kernel.
Do you mean you checked both gcc and clang and it was only a problem with gcc?
(If so, I can tweak the "depends" below...)
This should let us avoid it, I'm currently testing:
diff --git a/lib/Kconfig.ubsan b/lib/Kconfig.ubsan
index 774315de555a..24091315c251 100644
--- a/lib/Kconfig.ubsan
+++ b/lib/Kconfig.ubsan
@@ -47,6 +47,19 @@ config UBSAN_BOUNDS
to the {str,mem}*cpy() family of functions (that is addressed
by CONFIG_FORTIFY_SOURCE).
+config UBSAN_OBJECT_SIZE
+ bool "Check for accesses beyond known object sizes"
+ default UBSAN
+ depends on !COMPILE_TEST
+ help
+ This option enables detection of cases where accesses may
+ happen beyond the end of an object's size, which happens in
+ places like invalid downcasts, or calling function pointers
+ through invalid pointers.
+
+ This uses much more stack space, and isn't recommended for
+ cases were stack utilization depth is a concern.
+
config UBSAN_MISC
bool "Enable all other Undefined Behavior sanity checks"
default UBSAN
diff --git a/scripts/Makefile.ubsan b/scripts/Makefile.ubsan
index 27348029b2b8..3ff67e9b17fd 100644
--- a/scripts/Makefile.ubsan
+++ b/scripts/Makefile.ubsan
@@ -7,12 +7,15 @@ ifdef CONFIG_UBSAN_BOUNDS
CFLAGS_UBSAN += $(call cc-option, -fsanitize=bounds)
endif
+ifdef CONFIG_UBSAN_OBJECT_SIZE
+ CFLAGS_UBSAN += $(call cc-option, -fsanitize=object-size)
+endif
+
ifdef CONFIG_UBSAN_MISC
CFLAGS_UBSAN += $(call cc-option, -fsanitize=shift)
CFLAGS_UBSAN += $(call cc-option, -fsanitize=integer-divide-by-zero)
CFLAGS_UBSAN += $(call cc-option, -fsanitize=unreachable)
CFLAGS_UBSAN += $(call cc-option, -fsanitize=signed-integer-overflow)
- CFLAGS_UBSAN += $(call cc-option, -fsanitize=object-size)
CFLAGS_UBSAN += $(call cc-option, -fsanitize=bool)
CFLAGS_UBSAN += $(call cc-option, -fsanitize=enum)
endif
--
Kees Cook
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: lib/crypto/chacha.c:65:1: warning: the frame size of 1604 bytes is larger than 1024 bytes
2020-08-27 18:42 ` Kees Cook
@ 2020-08-27 19:02 ` Linus Torvalds
2020-08-27 19:32 ` Kees Cook
0 siblings, 1 reply; 23+ messages in thread
From: Linus Torvalds @ 2020-08-27 19:02 UTC (permalink / raw)
To: Kees Cook
Cc: Herbert Xu, Ard Biesheuvel, Arnd Bergmann, kernel test robot,
Peter Oberparleiter, Andrey Ryabinin, kbuild-all,
Linux Kernel Mailing List, Linux Crypto Mailing List
On Thu, Aug 27, 2020 at 11:42 AM Kees Cook <keescook@chromium.org> wrote:
>
> Do you mean you checked both gcc and clang and it was only a problem with gcc?
I didn't check with clang, but Arnd claimed it was fine.
> (If so, I can tweak the "depends" below...)
Ugh.
Instead of making the Makefile even uglier, why don't you just make
this all be done in the Kconfig.
Also, I'm not seeing the point of your patch. You didn't actually
change anything, you just made a new config variable with the same
semantics as the old one.
Add a
depends on CLANG
or something, with a comment saying that it doesn't work on gcc due to
excessive stack use.
> +ifdef CONFIG_UBSAN_OBJECT_SIZE
> + CFLAGS_UBSAN += $(call cc-option, -fsanitize=object-size)
> +endif
All of this should be thrown out, and this code should use the proper
patterns for configuration entries in the Makefile, ie just
ubsan-cflags-$(CONFIG_UBSAN_OBJECT_SIZE) += -fsanitize=object-size
and the Kconfig file is the thing that should check if that CC option
exists with
config UBSAN_OBJECT_SIZE
bool "Check for accesses beyond known object sizes"
default UBSAN
depends on CLANG # gcc makes a mess of it
depends on $(cc-option,-fsanitize-coverage=trace-pc)
and the same goes for all the other cases too:
> ifdef CONFIG_UBSAN_MISC
> CFLAGS_UBSAN += $(call cc-option, -fsanitize=shift)
> CFLAGS_UBSAN += $(call cc-option, -fsanitize=integer-divide-by-zero)
> CFLAGS_UBSAN += $(call cc-option, -fsanitize=unreachable)
> CFLAGS_UBSAN += $(call cc-option, -fsanitize=signed-integer-overflow)
> - CFLAGS_UBSAN += $(call cc-option, -fsanitize=object-size)
> CFLAGS_UBSAN += $(call cc-option, -fsanitize=bool)
> CFLAGS_UBSAN += $(call cc-option, -fsanitize=enum)
> endif
and if you don't want to ask for them (which is a good idea), you keep that
config UBSAN_MISC
bool "Misc UBSAN.."
thing, and just make all of the above have the pattern of
config UBSAN_OBJECT_SIZE
def_bool UBSAN_MISC
depends on CLANG # gcc makes a mess of it
depends on $(cc-option,-fsanitize-coverage=trace-pc)
which makes the Makefile much cleaner, and makes all our choices very
visible in the config file when they then get passed around.
We should basically strive for our Makefiles to have as little "ifdef"
etc magic as possible. We did the config work already, the Makefiles
should primarily just have those
XYZ-$(CONFIG_OPTION) += abc
kind of lines (and then you often end up having
CFLAGS_UBSAN := $(ubsan-cflags-y)
at the end).
Doesn't that all look much cleaner?
Linus
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: lib/crypto/chacha.c:65:1: warning: the frame size of 1604 bytes is larger than 1024 bytes
2020-08-27 19:02 ` Linus Torvalds
@ 2020-08-27 19:32 ` Kees Cook
0 siblings, 0 replies; 23+ messages in thread
From: Kees Cook @ 2020-08-27 19:32 UTC (permalink / raw)
To: Linus Torvalds
Cc: Herbert Xu, Ard Biesheuvel, Arnd Bergmann, kernel test robot,
Peter Oberparleiter, Andrey Ryabinin, kbuild-all,
Linux Kernel Mailing List, Linux Crypto Mailing List
On Thu, Aug 27, 2020 at 12:02:12PM -0700, Linus Torvalds wrote:
> On Thu, Aug 27, 2020 at 11:42 AM Kees Cook <keescook@chromium.org> wrote:
> >
> > Do you mean you checked both gcc and clang and it was only a problem with gcc?
>
> I didn't check with clang, but Arnd claimed it was fine.
>
> > (If so, I can tweak the "depends" below...)
>
> Ugh.
>
> Instead of making the Makefile even uglier, why don't you just make
> this all be done in the Kconfig.
>
> Also, I'm not seeing the point of your patch. You didn't actually
> change anything, you just made a new config variable with the same
> semantics as the old one.
Hmm? Yeah it did: it disallowed CONFIG_COMPILE_TEST, which you said was
the missing piece, I thought? (It's hardly the first time COMPILE_TEST
has collided unhappily with *SAN-ish things.)
> All of this should be thrown out, and this code should use the proper
> patterns for configuration entries in the Makefile, ie just
>
> ubsan-cflags-$(CONFIG_UBSAN_OBJECT_SIZE) += -fsanitize=object-size
Yeah, that would be a better pattern for sure.
> and the Kconfig file is the thing that should check if that CC option
> exists with
>
> config UBSAN_OBJECT_SIZE
> bool "Check for accesses beyond known object sizes"
> default UBSAN
> depends on CLANG # gcc makes a mess of it
> depends on $(cc-option,-fsanitize-coverage=trace-pc)
Yup, for sure. I've only recently started poking at the ubsan stuff. I
can clean it up better.
> Doesn't that all look much cleaner?
Yup!
--
Kees Cook
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: lib/crypto/chacha.c:65:1: warning: the frame size of 1604 bytes is larger than 1024 bytes
2020-08-27 17:55 ` Linus Torvalds
2020-08-27 18:42 ` Kees Cook
@ 2020-08-27 19:11 ` Arnd Bergmann
2020-08-27 19:34 ` Kees Cook
2020-08-27 20:08 ` Linus Torvalds
1 sibling, 2 replies; 23+ messages in thread
From: Arnd Bergmann @ 2020-08-27 19:11 UTC (permalink / raw)
To: Linus Torvalds
Cc: Herbert Xu, Ard Biesheuvel, kernel test robot,
Peter Oberparleiter, Kees Cook, Andrey Ryabinin, kbuild-all,
Linux Kernel Mailing List, Linux Crypto Mailing List
On Thu, Aug 27, 2020 at 7:55 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Thu, Aug 27, 2020 at 10:34 AM Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> > How are you guys testing? I have UBSAN and GCOV on, and don't see
> > crazy frames on either i386 or x86-64.
>
> Oh, never mind. I also have COMPILE_TEST on, so it ends up disabling
> GCOV_PROFILE_ALL and UBSAN_SANITIZE_ALL.
Ah right, that explains why I never saw the warning in my randconfig
build tests, I run those with COMPILE_TEST force-enabled.
Arnd
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: lib/crypto/chacha.c:65:1: warning: the frame size of 1604 bytes is larger than 1024 bytes
2020-08-27 19:11 ` Arnd Bergmann
@ 2020-08-27 19:34 ` Kees Cook
2020-08-27 20:08 ` Linus Torvalds
1 sibling, 0 replies; 23+ messages in thread
From: Kees Cook @ 2020-08-27 19:34 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Linus Torvalds, Herbert Xu, Ard Biesheuvel, kernel test robot,
Peter Oberparleiter, Andrey Ryabinin, kbuild-all,
Linux Kernel Mailing List, Linux Crypto Mailing List
On Thu, Aug 27, 2020 at 09:11:52PM +0200, Arnd Bergmann wrote:
> On Thu, Aug 27, 2020 at 7:55 PM Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> > On Thu, Aug 27, 2020 at 10:34 AM Linus Torvalds
> > <torvalds@linux-foundation.org> wrote:
> > >
> > > How are you guys testing? I have UBSAN and GCOV on, and don't see
> > > crazy frames on either i386 or x86-64.
> >
> > Oh, never mind. I also have COMPILE_TEST on, so it ends up disabling
> > GCOV_PROFILE_ALL and UBSAN_SANITIZE_ALL.
>
> Ah right, that explains why I never saw the warning in my randconfig
> build tests, I run those with COMPILE_TEST force-enabled.
Ah, I got this backwards. It's not COMPILE_TEST breaking it, it's
actually FIXING it. :P Anyway, I'll go clean this up more.
--
Kees Cook
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: lib/crypto/chacha.c:65:1: warning: the frame size of 1604 bytes is larger than 1024 bytes
2020-08-27 19:11 ` Arnd Bergmann
2020-08-27 19:34 ` Kees Cook
@ 2020-08-27 20:08 ` Linus Torvalds
1 sibling, 0 replies; 23+ messages in thread
From: Linus Torvalds @ 2020-08-27 20:08 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Herbert Xu, Ard Biesheuvel, kernel test robot,
Peter Oberparleiter, Kees Cook, Andrey Ryabinin, kbuild-all,
Linux Kernel Mailing List, Linux Crypto Mailing List
On Thu, Aug 27, 2020 at 12:12 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> Ah right, that explains why I never saw the warning in my randconfig
> build tests, I run those with COMPILE_TEST force-enabled.
.. but your clang test did enable this?
.. never mind, I have clang locally anyway, and while I usually don't
do the allmodconfig test there, I did it now with COMPILE_TEST
disabled.
clang does seem fine. It generates 136 bytes of stack-frame (plus
register saves), which is certainly not optimal, but it's not horribly
excessive.
Of course, I don't know if clang actually does the same as gcc with
-fsanitize=object-size and -fprofile-arcs, but whatever they do, they
were on for that clang build.
So yes, this does seem to be a gcc-only problem.
Linus
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: lib/crypto/chacha.c:65:1: warning: the frame size of 1604 bytes is larger than 1024 bytes
2020-08-27 8:10 ` Ard Biesheuvel
2020-08-27 8:24 ` Herbert Xu
@ 2020-08-27 8:33 ` Arnd Bergmann
2020-08-27 8:42 ` Ard Biesheuvel
1 sibling, 1 reply; 23+ messages in thread
From: Arnd Bergmann @ 2020-08-27 8:33 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Herbert Xu, kernel test robot, Peter Oberparleiter, Kees Cook,
Andrey Ryabinin, Linus Torvalds, kbuild-all,
Linux Kernel Mailing List, Linux Crypto Mailing List
On Thu, Aug 27, 2020 at 10:10 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> On Thu, 27 Aug 2020 at 10:06, Herbert Xu <herbert@gondor.apana.org.au> wrote:
> >
> > On Thu, Aug 27, 2020 at 11:52:50AM +0800, kernel test robot wrote:
> > >
> > > First bad commit (maybe != root cause):
> > >
> > > tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> > > head: 15bc20c6af4ceee97a1f90b43c0e386643c071b4
> > > commit: 5fb8ef25803ef33e2eb60b626435828b937bed75 crypto: chacha - move existing library code into lib/crypto
> > > date: 9 months ago
> > > config: i386-randconfig-r015-20200827 (attached as .config)
> > > compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
> > > reproduce (this is a W=1 build):
> > > git checkout 5fb8ef25803ef33e2eb60b626435828b937bed75
> > > # save the attached .config to linux build tree
> > > make W=1 ARCH=i386
> > >
> > > If you fix the issue, kindly add following tag as appropriate
> > > Reported-by: kernel test robot <lkp@intel.com>
> > >
> > > All warnings (new ones prefixed by >>):
> > >
> > > lib/crypto/chacha.c: In function 'chacha_permute':
> > > >> lib/crypto/chacha.c:65:1: warning: the frame size of 1604 bytes is larger than 1024 bytes [-Wframe-larger-than=]
> > > 65 | }
> > > | ^
> >
> > This doesn't happen with a normal configuration. To recreate
> > this warning, you need to enable both GCOV_KERNEL and UBSAN.
> >
> > This is the minimal gcc command-line to recreate it:
> >
> > gcc -Wframe-larger-than=1024 -fprofile-arcs -fsanitize=object-size -c -O2 chacha.c
> >
> > If you take away either profile-arcs or sanitize=object-size then
> > the problem goes away.
> >
> > Any suggestions?
> >
>
> Is it really worth it to obsess about this? Special compiler
> instrumentation simply leads to a larger stack footprint in many
> cases, which is why we use a larger stack to begin with (at least we
> do so for Kasan, so if we don't for Ubsan, we should consider it)
>
> Past experience also shows that this is highly dependent on the exact
> compiler version, so issues like these are often moving targets.
Yes, I do think it is important to address these in some form,
for multiple reasons:
* With the limited amount of stack space in normal uninstrumented
kernels, I do think it is vital to have a fairly low default warning
limit in order to catch all cases that do something dangerously
stupid, either because of code bugs or compiler bugs.
* I also think we do want the warning enabled in other configurations,
in particular because the compiler tends to make increasingly stupid
decisions when combining less common instrumentations, which
again can lead to instant exploitable bugs, e.g. when a function's
stack frame grows beyond the total stack size. In many cases the
gcc and clang developers are both able to address these quickly
when we send a good bug report (which unfortunately can be a lot of
work).
* The crypto cipher code unfortunately is particularly prone to running
into these issues because each new compiler version tries to
find more clever tricks to optimize code that in turn implements
an algorithm that intentionally tries to not have any clever shortcuts.
In many cases the stack size warning that we see in some
configurations is an indicator for the compiler running into a false
optimization even on the non-instrumented code that leads to lower
performance from unnecessary register spilling that should be
avoided.
Arnd
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: lib/crypto/chacha.c:65:1: warning: the frame size of 1604 bytes is larger than 1024 bytes
2020-08-27 8:33 ` Arnd Bergmann
@ 2020-08-27 8:42 ` Ard Biesheuvel
2020-08-27 9:19 ` Arnd Bergmann
0 siblings, 1 reply; 23+ messages in thread
From: Ard Biesheuvel @ 2020-08-27 8:42 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Herbert Xu, kernel test robot, Peter Oberparleiter, Kees Cook,
Andrey Ryabinin, Linus Torvalds, kbuild-all,
Linux Kernel Mailing List, Linux Crypto Mailing List
On Thu, 27 Aug 2020 at 10:33, Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Thu, Aug 27, 2020 at 10:10 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> > On Thu, 27 Aug 2020 at 10:06, Herbert Xu <herbert@gondor.apana.org.au> wrote:
> > >
> > > On Thu, Aug 27, 2020 at 11:52:50AM +0800, kernel test robot wrote:
> > > >
> > > > First bad commit (maybe != root cause):
> > > >
> > > > tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> > > > head: 15bc20c6af4ceee97a1f90b43c0e386643c071b4
> > > > commit: 5fb8ef25803ef33e2eb60b626435828b937bed75 crypto: chacha - move existing library code into lib/crypto
> > > > date: 9 months ago
> > > > config: i386-randconfig-r015-20200827 (attached as .config)
> > > > compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
> > > > reproduce (this is a W=1 build):
> > > > git checkout 5fb8ef25803ef33e2eb60b626435828b937bed75
> > > > # save the attached .config to linux build tree
> > > > make W=1 ARCH=i386
> > > >
> > > > If you fix the issue, kindly add following tag as appropriate
> > > > Reported-by: kernel test robot <lkp@intel.com>
> > > >
> > > > All warnings (new ones prefixed by >>):
> > > >
> > > > lib/crypto/chacha.c: In function 'chacha_permute':
> > > > >> lib/crypto/chacha.c:65:1: warning: the frame size of 1604 bytes is larger than 1024 bytes [-Wframe-larger-than=]
> > > > 65 | }
> > > > | ^
> > >
> > > This doesn't happen with a normal configuration. To recreate
> > > this warning, you need to enable both GCOV_KERNEL and UBSAN.
> > >
> > > This is the minimal gcc command-line to recreate it:
> > >
> > > gcc -Wframe-larger-than=1024 -fprofile-arcs -fsanitize=object-size -c -O2 chacha.c
> > >
> > > If you take away either profile-arcs or sanitize=object-size then
> > > the problem goes away.
> > >
> > > Any suggestions?
> > >
> >
> > Is it really worth it to obsess about this? Special compiler
> > instrumentation simply leads to a larger stack footprint in many
> > cases, which is why we use a larger stack to begin with (at least we
> > do so for Kasan, so if we don't for Ubsan, we should consider it)
> >
> > Past experience also shows that this is highly dependent on the exact
> > compiler version, so issues like these are often moving targets.
>
> Yes, I do think it is important to address these in some form,
> for multiple reasons:
>
> * With the limited amount of stack space in normal uninstrumented
> kernels, I do think it is vital to have a fairly low default warning
> limit in order to catch all cases that do something dangerously
> stupid, either because of code bugs or compiler bugs.
>
> * I also think we do want the warning enabled in other configurations,
> in particular because the compiler tends to make increasingly stupid
> decisions when combining less common instrumentations, which
> again can lead to instant exploitable bugs, e.g. when a function's
> stack frame grows beyond the total stack size. In many cases the
> gcc and clang developers are both able to address these quickly
> when we send a good bug report (which unfortunately can be a lot of
> work).
>
> * The crypto cipher code unfortunately is particularly prone to running
> into these issues because each new compiler version tries to
> find more clever tricks to optimize code that in turn implements
> an algorithm that intentionally tries to not have any clever shortcuts.
> In many cases the stack size warning that we see in some
> configurations is an indicator for the compiler running into a false
> optimization even on the non-instrumented code that leads to lower
> performance from unnecessary register spilling that should be
> avoided.
>
In that case, I suppose we should simply disable instrumentation for
chacha_permute()? It is a straight-forward arithmetic transformation
on a u32[16] array, where ubsan has limited value afaict.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: lib/crypto/chacha.c:65:1: warning: the frame size of 1604 bytes is larger than 1024 bytes
2020-08-27 8:42 ` Ard Biesheuvel
@ 2020-08-27 9:19 ` Arnd Bergmann
2020-08-27 10:41 ` Ard Biesheuvel
0 siblings, 1 reply; 23+ messages in thread
From: Arnd Bergmann @ 2020-08-27 9:19 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Herbert Xu, kernel test robot, Peter Oberparleiter, Kees Cook,
Andrey Ryabinin, Linus Torvalds, kbuild-all,
Linux Kernel Mailing List, Linux Crypto Mailing List
On Thu, Aug 27, 2020 at 10:42 AM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> In that case, I suppose we should simply disable instrumentation for
> chacha_permute()? It is a straight-forward arithmetic transformation
> on a u32[16] array, where ubsan has limited value afaict.
I guess that always works as a last resort, but shouldn't we first try
to figure out why ubsan even makes a difference and whether the
object code without ubsan looks like a reasonable representation
of the source form?
Since it really is a fairly simple transformation, I would have
expected the compiler to not emit any ubsan checks. If gcc
only gets confused about the fixed offsets possibly overflowing
the fixed-length array, maybe it helps to give it a little extra
information like (untested):
--- a/lib/crypto/chacha.c
+++ b/lib/crypto/chacha.c
@@ -13,7 +13,7 @@
#include <asm/unaligned.h>
#include <crypto/chacha.h>
-static void chacha_permute(u32 *x, int nrounds)
+static void chacha_permute(u32 x[16], int nrounds)
{
int i;
Arnd
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: lib/crypto/chacha.c:65:1: warning: the frame size of 1604 bytes is larger than 1024 bytes
2020-08-27 9:19 ` Arnd Bergmann
@ 2020-08-27 10:41 ` Ard Biesheuvel
2020-08-27 11:51 ` Herbert Xu
0 siblings, 1 reply; 23+ messages in thread
From: Ard Biesheuvel @ 2020-08-27 10:41 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Herbert Xu, kernel test robot, Peter Oberparleiter, Kees Cook,
Andrey Ryabinin, Linus Torvalds, kbuild-all,
Linux Kernel Mailing List, Linux Crypto Mailing List
On Thu, 27 Aug 2020 at 11:20, Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Thu, Aug 27, 2020 at 10:42 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > In that case, I suppose we should simply disable instrumentation for
> > chacha_permute()? It is a straight-forward arithmetic transformation
> > on a u32[16] array, where ubsan has limited value afaict.
>
> I guess that always works as a last resort, but shouldn't we first try
> to figure out why ubsan even makes a difference and whether the
> object code without ubsan looks like a reasonable representation
> of the source form?
>
> Since it really is a fairly simple transformation, I would have
> expected the compiler to not emit any ubsan checks. If gcc
> only gets confused about the fixed offsets possibly overflowing
> the fixed-length array, maybe it helps to give it a little extra
> information like (untested):
>
> --- a/lib/crypto/chacha.c
> +++ b/lib/crypto/chacha.c
> @@ -13,7 +13,7 @@
> #include <asm/unaligned.h>
> #include <crypto/chacha.h>
>
> -static void chacha_permute(u32 *x, int nrounds)
> +static void chacha_permute(u32 x[16], int nrounds)
> {
> int i;
>
That does not help, unfortunately.
What does seem to work is
struct chacha_state { u32 x[16]; };
struct chacha_state chacha_permute(struct chacha_state st, int nrounds)
{
struct chacha_state ret = st;
u32 *x = ret.x;
...
return st;
}
(and updating the caller accordingly, obviously)
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: lib/crypto/chacha.c:65:1: warning: the frame size of 1604 bytes is larger than 1024 bytes
2020-08-27 10:41 ` Ard Biesheuvel
@ 2020-08-27 11:51 ` Herbert Xu
2020-08-27 16:25 ` Arnd Bergmann
0 siblings, 1 reply; 23+ messages in thread
From: Herbert Xu @ 2020-08-27 11:51 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Arnd Bergmann, kernel test robot, Peter Oberparleiter, Kees Cook,
Andrey Ryabinin, Linus Torvalds, kbuild-all,
Linux Kernel Mailing List, Linux Crypto Mailing List
On Thu, Aug 27, 2020 at 12:41:53PM +0200, Ard Biesheuvel wrote:
>
> That does not help, unfortunately.
>
> What does seem to work is
>
> struct chacha_state { u32 x[16]; };
>
> struct chacha_state chacha_permute(struct chacha_state st, int nrounds)
Passing 64 bytes by value is not good.
Passing struct chacha_state as a pointer doesn't work either.
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
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: lib/crypto/chacha.c:65:1: warning: the frame size of 1604 bytes is larger than 1024 bytes
2020-08-27 11:51 ` Herbert Xu
@ 2020-08-27 16:25 ` Arnd Bergmann
0 siblings, 0 replies; 23+ messages in thread
From: Arnd Bergmann @ 2020-08-27 16:25 UTC (permalink / raw)
To: Herbert Xu
Cc: Ard Biesheuvel, kernel test robot, Peter Oberparleiter,
Kees Cook, Andrey Ryabinin, Linus Torvalds, kbuild-all,
Linux Kernel Mailing List, Linux Crypto Mailing List
On Thu, Aug 27, 2020 at 1:51 PM Herbert Xu <herbert@gondor.apana.org.au> wrote:
>
> On Thu, Aug 27, 2020 at 12:41:53PM +0200, Ard Biesheuvel wrote:
> >
> > That does not help, unfortunately.
> >
> > What does seem to work is
> >
> > struct chacha_state { u32 x[16]; };
> >
> > struct chacha_state chacha_permute(struct chacha_state st, int nrounds)
>
> Passing 64 bytes by value is not good.
>
> Passing struct chacha_state as a pointer doesn't work either.
Marking the function as __always_inline avoids the problem, as it
lets the compiler see the issue, but seems to produce somewhat
worse object code.
I also tested with clang-11, which supports both -fsanitize-bounds
and -fprofile-arcs but only needs 8 bytes of stack for this function.
One more data point, I looked at the actual object code and found
that neither -fprofile-arcs nor -fsanitize-bounds has a noticeable
impact on the object code output by themselves (aside of not
leading to the warning as you already mentioned). I would
conclude that there is an actual problem with gcc here.
Arnd
^ permalink raw reply [flat|nested] 23+ messages in thread
* lib/crypto/chacha.c:65:1: warning: the frame size of 1604 bytes is larger than 1024 bytes
@ 2020-09-19 17:27 kernel test robot
0 siblings, 0 replies; 23+ messages in thread
From: kernel test robot @ 2020-09-19 17:27 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: kbuild-all, linux-kernel, Herbert Xu
[-- Attachment #1: Type: text/plain, Size: 6630 bytes --]
Hi Ard,
First bad commit (maybe != root cause):
tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
head: eb5f95f1593f7c22dac681b19e815828e2af3efd
commit: 5fb8ef25803ef33e2eb60b626435828b937bed75 crypto: chacha - move existing library code into lib/crypto
date: 10 months ago
config: i386-randconfig-r012-20200920 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce (this is a W=1 build):
git checkout 5fb8ef25803ef33e2eb60b626435828b937bed75
# save the attached .config to linux build tree
make W=1 ARCH=i386
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
lib/crypto/chacha.c: In function 'chacha_permute':
>> lib/crypto/chacha.c:65:1: warning: the frame size of 1604 bytes is larger than 1024 bytes [-Wframe-larger-than=]
65 | }
| ^
# https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5fb8ef25803ef33e2eb60b626435828b937bed75
git remote add linus https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
git fetch --no-tags linus master
git checkout 5fb8ef25803ef33e2eb60b626435828b937bed75
vim +65 lib/crypto/chacha.c
e192be9d9a3055 lib/chacha20.c Theodore Ts'o 2016-06-12 16
1ca1b917940c24 lib/chacha.c Eric Biggers 2018-11-16 17 static void chacha_permute(u32 *x, int nrounds)
e192be9d9a3055 lib/chacha20.c Theodore Ts'o 2016-06-12 18 {
e192be9d9a3055 lib/chacha20.c Theodore Ts'o 2016-06-12 19 int i;
e192be9d9a3055 lib/chacha20.c Theodore Ts'o 2016-06-12 20
1ca1b917940c24 lib/chacha.c Eric Biggers 2018-11-16 21 /* whitelist the allowed round counts */
aa7624093cb7fb lib/chacha.c Eric Biggers 2018-11-16 22 WARN_ON_ONCE(nrounds != 20 && nrounds != 12);
1ca1b917940c24 lib/chacha.c Eric Biggers 2018-11-16 23
1ca1b917940c24 lib/chacha.c Eric Biggers 2018-11-16 24 for (i = 0; i < nrounds; i += 2) {
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 25 x[0] += x[4]; x[12] = rol32(x[12] ^ x[0], 16);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 26 x[1] += x[5]; x[13] = rol32(x[13] ^ x[1], 16);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 27 x[2] += x[6]; x[14] = rol32(x[14] ^ x[2], 16);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 28 x[3] += x[7]; x[15] = rol32(x[15] ^ x[3], 16);
e192be9d9a3055 lib/chacha20.c Theodore Ts'o 2016-06-12 29
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 30 x[8] += x[12]; x[4] = rol32(x[4] ^ x[8], 12);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 31 x[9] += x[13]; x[5] = rol32(x[5] ^ x[9], 12);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 32 x[10] += x[14]; x[6] = rol32(x[6] ^ x[10], 12);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 33 x[11] += x[15]; x[7] = rol32(x[7] ^ x[11], 12);
e192be9d9a3055 lib/chacha20.c Theodore Ts'o 2016-06-12 34
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 35 x[0] += x[4]; x[12] = rol32(x[12] ^ x[0], 8);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 36 x[1] += x[5]; x[13] = rol32(x[13] ^ x[1], 8);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 37 x[2] += x[6]; x[14] = rol32(x[14] ^ x[2], 8);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 38 x[3] += x[7]; x[15] = rol32(x[15] ^ x[3], 8);
e192be9d9a3055 lib/chacha20.c Theodore Ts'o 2016-06-12 39
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 40 x[8] += x[12]; x[4] = rol32(x[4] ^ x[8], 7);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 41 x[9] += x[13]; x[5] = rol32(x[5] ^ x[9], 7);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 42 x[10] += x[14]; x[6] = rol32(x[6] ^ x[10], 7);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 43 x[11] += x[15]; x[7] = rol32(x[7] ^ x[11], 7);
e192be9d9a3055 lib/chacha20.c Theodore Ts'o 2016-06-12 44
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 45 x[0] += x[5]; x[15] = rol32(x[15] ^ x[0], 16);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 46 x[1] += x[6]; x[12] = rol32(x[12] ^ x[1], 16);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 47 x[2] += x[7]; x[13] = rol32(x[13] ^ x[2], 16);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 48 x[3] += x[4]; x[14] = rol32(x[14] ^ x[3], 16);
e192be9d9a3055 lib/chacha20.c Theodore Ts'o 2016-06-12 49
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 50 x[10] += x[15]; x[5] = rol32(x[5] ^ x[10], 12);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 51 x[11] += x[12]; x[6] = rol32(x[6] ^ x[11], 12);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 52 x[8] += x[13]; x[7] = rol32(x[7] ^ x[8], 12);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 53 x[9] += x[14]; x[4] = rol32(x[4] ^ x[9], 12);
e192be9d9a3055 lib/chacha20.c Theodore Ts'o 2016-06-12 54
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 55 x[0] += x[5]; x[15] = rol32(x[15] ^ x[0], 8);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 56 x[1] += x[6]; x[12] = rol32(x[12] ^ x[1], 8);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 57 x[2] += x[7]; x[13] = rol32(x[13] ^ x[2], 8);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 58 x[3] += x[4]; x[14] = rol32(x[14] ^ x[3], 8);
e192be9d9a3055 lib/chacha20.c Theodore Ts'o 2016-06-12 59
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 60 x[10] += x[15]; x[5] = rol32(x[5] ^ x[10], 7);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 61 x[11] += x[12]; x[6] = rol32(x[6] ^ x[11], 7);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 62 x[8] += x[13]; x[7] = rol32(x[7] ^ x[8], 7);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 63 x[9] += x[14]; x[4] = rol32(x[4] ^ x[9], 7);
e192be9d9a3055 lib/chacha20.c Theodore Ts'o 2016-06-12 64 }
dd333449d0fb66 lib/chacha20.c Eric Biggers 2018-11-16 @65 }
dd333449d0fb66 lib/chacha20.c Eric Biggers 2018-11-16 66
:::::: The code at line 65 was first introduced by commit
:::::: dd333449d0fb667c5250c42488a7e90470e16c77 crypto: chacha20-generic - add HChaCha20 library function
:::::: TO: Eric Biggers <ebiggers@google.com>
:::::: CC: Herbert Xu <herbert@gondor.apana.org.au>
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 30681 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread* lib/crypto/chacha.c:65:1: warning: the frame size of 1604 bytes is larger than 1024 bytes
@ 2020-10-18 19:13 kernel test robot
2020-10-19 15:47 ` Joe Perches
0 siblings, 1 reply; 23+ messages in thread
From: kernel test robot @ 2020-10-18 19:13 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: kbuild-all, linux-kernel, Herbert Xu
[-- Attachment #1: Type: text/plain, Size: 6600 bytes --]
Hi Ard,
First bad commit (maybe != root cause):
tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
head: 9d9af1007bc08971953ae915d88dc9bb21344b53
commit: 5fb8ef25803ef33e2eb60b626435828b937bed75 crypto: chacha - move existing library code into lib/crypto
date: 11 months ago
config: i386-randconfig-r023-20201019 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce (this is a W=1 build):
# https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5fb8ef25803ef33e2eb60b626435828b937bed75
git remote add linus https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
git fetch --no-tags linus master
git checkout 5fb8ef25803ef33e2eb60b626435828b937bed75
# save the attached .config to linux build tree
make W=1 ARCH=i386
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
lib/crypto/chacha.c: In function 'chacha_permute':
>> lib/crypto/chacha.c:65:1: warning: the frame size of 1604 bytes is larger than 1024 bytes [-Wframe-larger-than=]
65 | }
| ^
vim +65 lib/crypto/chacha.c
e192be9d9a3055 lib/chacha20.c Theodore Ts'o 2016-06-12 16
1ca1b917940c24 lib/chacha.c Eric Biggers 2018-11-16 17 static void chacha_permute(u32 *x, int nrounds)
e192be9d9a3055 lib/chacha20.c Theodore Ts'o 2016-06-12 18 {
e192be9d9a3055 lib/chacha20.c Theodore Ts'o 2016-06-12 19 int i;
e192be9d9a3055 lib/chacha20.c Theodore Ts'o 2016-06-12 20
1ca1b917940c24 lib/chacha.c Eric Biggers 2018-11-16 21 /* whitelist the allowed round counts */
aa7624093cb7fb lib/chacha.c Eric Biggers 2018-11-16 22 WARN_ON_ONCE(nrounds != 20 && nrounds != 12);
1ca1b917940c24 lib/chacha.c Eric Biggers 2018-11-16 23
1ca1b917940c24 lib/chacha.c Eric Biggers 2018-11-16 24 for (i = 0; i < nrounds; i += 2) {
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 25 x[0] += x[4]; x[12] = rol32(x[12] ^ x[0], 16);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 26 x[1] += x[5]; x[13] = rol32(x[13] ^ x[1], 16);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 27 x[2] += x[6]; x[14] = rol32(x[14] ^ x[2], 16);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 28 x[3] += x[7]; x[15] = rol32(x[15] ^ x[3], 16);
e192be9d9a3055 lib/chacha20.c Theodore Ts'o 2016-06-12 29
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 30 x[8] += x[12]; x[4] = rol32(x[4] ^ x[8], 12);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 31 x[9] += x[13]; x[5] = rol32(x[5] ^ x[9], 12);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 32 x[10] += x[14]; x[6] = rol32(x[6] ^ x[10], 12);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 33 x[11] += x[15]; x[7] = rol32(x[7] ^ x[11], 12);
e192be9d9a3055 lib/chacha20.c Theodore Ts'o 2016-06-12 34
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 35 x[0] += x[4]; x[12] = rol32(x[12] ^ x[0], 8);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 36 x[1] += x[5]; x[13] = rol32(x[13] ^ x[1], 8);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 37 x[2] += x[6]; x[14] = rol32(x[14] ^ x[2], 8);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 38 x[3] += x[7]; x[15] = rol32(x[15] ^ x[3], 8);
e192be9d9a3055 lib/chacha20.c Theodore Ts'o 2016-06-12 39
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 40 x[8] += x[12]; x[4] = rol32(x[4] ^ x[8], 7);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 41 x[9] += x[13]; x[5] = rol32(x[5] ^ x[9], 7);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 42 x[10] += x[14]; x[6] = rol32(x[6] ^ x[10], 7);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 43 x[11] += x[15]; x[7] = rol32(x[7] ^ x[11], 7);
e192be9d9a3055 lib/chacha20.c Theodore Ts'o 2016-06-12 44
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 45 x[0] += x[5]; x[15] = rol32(x[15] ^ x[0], 16);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 46 x[1] += x[6]; x[12] = rol32(x[12] ^ x[1], 16);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 47 x[2] += x[7]; x[13] = rol32(x[13] ^ x[2], 16);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 48 x[3] += x[4]; x[14] = rol32(x[14] ^ x[3], 16);
e192be9d9a3055 lib/chacha20.c Theodore Ts'o 2016-06-12 49
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 50 x[10] += x[15]; x[5] = rol32(x[5] ^ x[10], 12);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 51 x[11] += x[12]; x[6] = rol32(x[6] ^ x[11], 12);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 52 x[8] += x[13]; x[7] = rol32(x[7] ^ x[8], 12);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 53 x[9] += x[14]; x[4] = rol32(x[4] ^ x[9], 12);
e192be9d9a3055 lib/chacha20.c Theodore Ts'o 2016-06-12 54
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 55 x[0] += x[5]; x[15] = rol32(x[15] ^ x[0], 8);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 56 x[1] += x[6]; x[12] = rol32(x[12] ^ x[1], 8);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 57 x[2] += x[7]; x[13] = rol32(x[13] ^ x[2], 8);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 58 x[3] += x[4]; x[14] = rol32(x[14] ^ x[3], 8);
e192be9d9a3055 lib/chacha20.c Theodore Ts'o 2016-06-12 59
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 60 x[10] += x[15]; x[5] = rol32(x[5] ^ x[10], 7);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 61 x[11] += x[12]; x[6] = rol32(x[6] ^ x[11], 7);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 62 x[8] += x[13]; x[7] = rol32(x[7] ^ x[8], 7);
7660b1fb367eb3 lib/chacha20.c Eric Biggers 2017-12-31 63 x[9] += x[14]; x[4] = rol32(x[4] ^ x[9], 7);
e192be9d9a3055 lib/chacha20.c Theodore Ts'o 2016-06-12 64 }
dd333449d0fb66 lib/chacha20.c Eric Biggers 2018-11-16 @65 }
dd333449d0fb66 lib/chacha20.c Eric Biggers 2018-11-16 66
:::::: The code at line 65 was first introduced by commit
:::::: dd333449d0fb667c5250c42488a7e90470e16c77 crypto: chacha20-generic - add HChaCha20 library function
:::::: TO: Eric Biggers <ebiggers@google.com>
:::::: CC: Herbert Xu <herbert@gondor.apana.org.au>
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 35574 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: lib/crypto/chacha.c:65:1: warning: the frame size of 1604 bytes is larger than 1024 bytes
2020-10-18 19:13 kernel test robot
@ 2020-10-19 15:47 ` Joe Perches
2020-10-20 8:00 ` David Laight
0 siblings, 1 reply; 23+ messages in thread
From: Joe Perches @ 2020-10-19 15:47 UTC (permalink / raw)
To: kernel test robot, Ard Biesheuvel, Steven Rostedt
Cc: kbuild-all, linux-kernel, Herbert Xu
On Mon, 2020-10-19 at 03:13 +0800, kernel test robot wrote:
> Hi Ard,
>
> First bad commit (maybe != root cause):
>
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> head: 9d9af1007bc08971953ae915d88dc9bb21344b53
> commit: 5fb8ef25803ef33e2eb60b626435828b937bed75 crypto: chacha - move existing library code into lib/crypto
> date: 11 months ago
> config: i386-randconfig-r023-20201019 (attached as .config)
> compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
> reproduce (this is a W=1 build):
> # https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5fb8ef25803ef33e2eb60b626435828b937bed75
> git remote add linus https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> git fetch --no-tags linus master
> git checkout 5fb8ef25803ef33e2eb60b626435828b937bed75
> # save the attached .config to linux build tree
> make W=1 ARCH=i386
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
>
> All warnings (new ones prefixed by >>):
>
> lib/crypto/chacha.c: In function 'chacha_permute':
> > > lib/crypto/chacha.c:65:1: warning: the frame size of 1604 bytes is larger than 1024 bytes [-Wframe-larger-than=]
> 65 | }
> | ^
>
> vim +65 lib/crypto/chacha.c
This seems to come from function tracing overhead.
It it really useful to add function tracing to what
should be a single instruction?
AFAIK:
There isn't a good way to turn function tracing off
on a per-function basis.
Adding notrace doesn't work for that. Should it?
^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: lib/crypto/chacha.c:65:1: warning: the frame size of 1604 bytes is larger than 1024 bytes
2020-10-19 15:47 ` Joe Perches
@ 2020-10-20 8:00 ` David Laight
2020-10-20 10:13 ` Joe Perches
0 siblings, 1 reply; 23+ messages in thread
From: David Laight @ 2020-10-20 8:00 UTC (permalink / raw)
To: 'Joe Perches', kernel test robot, Ard Biesheuvel, Steven Rostedt
Cc: kbuild-all, linux-kernel, Herbert Xu
From: Joe Perches
> Sent: 19 October 2020 16:47
> On Mon, 2020-10-19 at 03:13 +0800, kernel test robot wrote:
> > Hi Ard,
> >
> > First bad commit (maybe != root cause):
> >
> > tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> > head: 9d9af1007bc08971953ae915d88dc9bb21344b53
> > commit: 5fb8ef25803ef33e2eb60b626435828b937bed75 crypto: chacha - move existing library code into
> lib/crypto
> > date: 11 months ago
> > config: i386-randconfig-r023-20201019 (attached as .config)
> > compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
> > reproduce (this is a W=1 build):
> > #
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5fb8ef25803ef33e2eb60b62
> 6435828b937bed75
> > git remote add linus https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> > git fetch --no-tags linus master
> > git checkout 5fb8ef25803ef33e2eb60b626435828b937bed75
> > # save the attached .config to linux build tree
> > make W=1 ARCH=i386
> >
> > If you fix the issue, kindly add following tag as appropriate
> > Reported-by: kernel test robot <lkp@intel.com>
> >
> > All warnings (new ones prefixed by >>):
> >
> > lib/crypto/chacha.c: In function 'chacha_permute':
> > > > lib/crypto/chacha.c:65:1: warning: the frame size of 1604 bytes is larger than 1024 bytes [-
> Wframe-larger-than=]
> > 65 | }
> > | ^
> >
> > vim +65 lib/crypto/chacha.c
>
> This seems to come from function tracing overhead.
Are you sure?
I've not got the x86 object, but the x86-64 version caches the 16 x[]
values (from the parameter) in registers.
The 32 bit cpu doesn't have enough registers, but gcc tends to
compile assuming an infinite number.
So it may have spilled lots of virtual registers to different
stack locations - instead of writing the values to their 'target'
address.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: lib/crypto/chacha.c:65:1: warning: the frame size of 1604 bytes is larger than 1024 bytes
2020-10-20 8:00 ` David Laight
@ 2020-10-20 10:13 ` Joe Perches
0 siblings, 0 replies; 23+ messages in thread
From: Joe Perches @ 2020-10-20 10:13 UTC (permalink / raw)
To: David Laight, kernel test robot, Ard Biesheuvel, Steven Rostedt
Cc: kbuild-all, linux-kernel, Herbert Xu
On Tue, 2020-10-20 at 08:00 +0000, David Laight wrote:
> From: Joe Perches
> > Sent: 19 October 2020 16:47
> > On Mon, 2020-10-19 at 03:13 +0800, kernel test robot wrote:
> > > Hi Ard,
> > >
> > > First bad commit (maybe != root cause):
> > >
> > > tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> > > head: 9d9af1007bc08971953ae915d88dc9bb21344b53
> > > commit: 5fb8ef25803ef33e2eb60b626435828b937bed75 crypto: chacha - move existing library code into
> > lib/crypto
> > > date: 11 months ago
> > > config: i386-randconfig-r023-20201019 (attached as .config)
> > > compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
> > > reproduce (this is a W=1 build):
> > > #
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5fb8ef25803ef33e2eb60b62
> > 6435828b937bed75
> > > git remote add linus https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> > > git fetch --no-tags linus master
> > > git checkout 5fb8ef25803ef33e2eb60b626435828b937bed75
> > > # save the attached .config to linux build tree
> > > make W=1 ARCH=i386
> > >
> > > If you fix the issue, kindly add following tag as appropriate
> > > Reported-by: kernel test robot <lkp@intel.com>
> > >
> > > All warnings (new ones prefixed by >>):
> > >
> > > lib/crypto/chacha.c: In function 'chacha_permute':
> > > > > lib/crypto/chacha.c:65:1: warning: the frame size of 1604 bytes is larger than 1024 bytes [-
> > Wframe-larger-than=]
> > > 65 | }
> > > | ^
> > >
> > > vim +65 lib/crypto/chacha.c
> >
> > This seems to come from function tracing overhead.
>
> Are you sure?
No. I'm trying to isolate it now.
> I've not got the x86 object, but the x86-64 version caches the 16 x[]
> values (from the parameter) in registers.
> The 32 bit cpu doesn't have enough registers, but gcc tends to
> compile assuming an infinite number.
> So it may have spilled lots of virtual registers to different
> stack locations - instead of writing the values to their 'target'
> address.
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2020-10-20 10:14 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-27 3:52 lib/crypto/chacha.c:65:1: warning: the frame size of 1604 bytes is larger than 1024 bytes kernel test robot
2020-08-27 8:05 ` Herbert Xu
2020-08-27 8:10 ` Ard Biesheuvel
2020-08-27 8:24 ` Herbert Xu
2020-08-27 17:34 ` Linus Torvalds
2020-08-27 17:55 ` Linus Torvalds
2020-08-27 18:42 ` Kees Cook
2020-08-27 19:02 ` Linus Torvalds
2020-08-27 19:32 ` Kees Cook
2020-08-27 19:11 ` Arnd Bergmann
2020-08-27 19:34 ` Kees Cook
2020-08-27 20:08 ` Linus Torvalds
2020-08-27 8:33 ` Arnd Bergmann
2020-08-27 8:42 ` Ard Biesheuvel
2020-08-27 9:19 ` Arnd Bergmann
2020-08-27 10:41 ` Ard Biesheuvel
2020-08-27 11:51 ` Herbert Xu
2020-08-27 16:25 ` Arnd Bergmann
2020-09-19 17:27 kernel test robot
2020-10-18 19:13 kernel test robot
2020-10-19 15:47 ` Joe Perches
2020-10-20 8:00 ` David Laight
2020-10-20 10:13 ` Joe Perches
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).