So I don't see anything that has changed, and I suspect the only
change is that my compiler version changed, but my arm64 build fails
right now with FORTIFY_STRING enabled.
On arm64 I now get this warning:
In function 'fortify_memcpy_chk',
inlined from 'mv_cesa_des3_ede_setkey' and
drivers/crypto/marvell/cesa/cipher.c:307:2:
./include/linux/fortify-string.h:583:25: error: call to
'__write_overflow_field' declared with attribute warning: detected
write beyond size of field (1st parameter); maybe use struct_group()?
[-Werror=attribute-warning[
but I haven't been compiling regularly enough to know when this
warning suddenly started showing up.
I enabled the cesa driver on x86-64 (by also letting it build with
COMPILE_TEST), and I do *not* see this warning on x86-64, which makes
me think it's the compiler version that matters here.
On my arm64 setup, I have gcc-13.1.1, while my x86-64 build is still 12.3.1.
But I think the warning is correct. The problem is that the 'ctx'
pointer is wrongly typed, and it's using "struct mv_cesa_des_ctx"
(which has a "key[]" size of DES_KEY_SIZE).
And it *should* use "struct mv_cesa_des3_ctx" which has otherwise the
same layout, but the right key size (DES3_EDE_KEY_SIZE).
Fixing that type fixes the warning.
I'm actually surprised that gcc-12 doesn't seem to warn about this.
Kees? This looks like a rather obvious overflow, which makes me think
I'm missing something.
I get a similar error in 'irdma_clr_wqes()' at
drivers/infiniband/hw/irdma/uk.c:103 (and same thing on line 105). I
don't see what the right solution there is, but it looks like we have
IRDMA_CQP_WQE_SIZE = 8
__le64 elem[IRDMA_CQP_WQE_SIZE];
and it's doing a 4kB memset to that element. The mistake is not as
obvious as in the cesa driver.
Kees, any idea why I'm seeing it now? Is it the new
-fstrict-flex-arrays=3? And if so, why? None of this is about flex
arrays...
Anyway, please forward me fixes so that I can have a working arm64
test build again....
Linus
On Wed, Jun 28, 2023 at 08:13:25PM -0700, Linus Torvalds wrote:
> So I don't see anything that has changed, and I suspect the only
> change is that my compiler version changed, but my arm64 build fails
> right now with FORTIFY_STRING enabled.
>
> On arm64 I now get this warning:
>
> In function 'fortify_memcpy_chk',
> inlined from 'mv_cesa_des3_ede_setkey' and
> drivers/crypto/marvell/cesa/cipher.c:307:2:
> ./include/linux/fortify-string.h:583:25: error: call to
> '__write_overflow_field' declared with attribute warning: detected
> write beyond size of field (1st parameter); maybe use struct_group()?
> [-Werror=attribute-warning[
This was fixed very recently here:
https://lore.kernel.org/all/[email protected]/
and Herbert took it.
I assume the crypto tree hasn't been merged yet?
> Kees, any idea why I'm seeing it now? Is it the new
> -fstrict-flex-arrays=3? And if so, why? None of this is about flex
> arrays...
The unexpected bit is that without -fstrict-flex-arrays=3 (i.e. the
default since the dawn of time), the compiler treats any array that
happens to be the last struct member as a flexible array. So with it
enabled, FORTIFY_SOURCE gains coverage over things it should have been
examining before.
--
Kees Cook
On Wed, Jun 28, 2023 at 08:13:25PM -0700, Linus Torvalds wrote:
> I get a similar error in 'irdma_clr_wqes()' at
> drivers/infiniband/hw/irdma/uk.c:103 (and same thing on line 105). I
> don't see what the right solution there is, but it looks like we have
>
> IRDMA_CQP_WQE_SIZE = 8
> __le64 elem[IRDMA_CQP_WQE_SIZE];
>
> and it's doing a 4kB memset to that element. The mistake is not as
> obvious as in the cesa driver.
I pressed "send" too fast. :)
This should also already be fixed:
https://lore.kernel.org/all/[email protected]/
--
Kees Cook
On Wed, Jun 28, 2023 at 08:48:19PM -0700, Kees Cook wrote:
>
> This was fixed very recently here:
> https://lore.kernel.org/all/[email protected]/
> and Herbert took it.
>
> I assume the crypto tree hasn't been merged yet?
Yes it's in cryptodev:
https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git/commit/?id=efbc7764c4446566edb76ca05e903b5905673d2e
Now that this seems to be causing build failures, I'll add it to the
crypto tree.
Thanks,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
On Wed, 28 Jun 2023 at 20:48, Kees Cook <[email protected]> wrote:
>
> The unexpected bit is that without -fstrict-flex-arrays=3 (i.e. the
> default since the dawn of time), the compiler treats any array that
> happens to be the last struct member as a flexible array.
Oh. Ok, that explains why it's showing up for me now, at least. It's
an odd rule, but I can see why people would have done that.
I've only seen the zero- and one-sized arrays commonly used for the
traditional "fake flex array", but I guess other sizes can easily
happen.
Linus
On Wed, Jun 28, 2023 at 08:53:26PM -0700, Kees Cook wrote:
> On Wed, Jun 28, 2023 at 08:13:25PM -0700, Linus Torvalds wrote:
> > I get a similar error in 'irdma_clr_wqes()' at
> > drivers/infiniband/hw/irdma/uk.c:103 (and same thing on line 105). I
> > don't see what the right solution there is, but it looks like we have
> >
> > IRDMA_CQP_WQE_SIZE = 8
> > __le64 elem[IRDMA_CQP_WQE_SIZE];
> >
> > and it's doing a 4kB memset to that element. The mistake is not as
> > obvious as in the cesa driver.
>
> I pressed "send" too fast. :)
>
> This should also already be fixed:
> https://lore.kernel.org/all/[email protected]/
The fix is in latest RDMA PR:
https://lore.kernel.org/linux-rdma/[email protected]/T/#u
Arnd Bergmann (1):
RDMA/irdma: avoid fortify-string warning in irdma_clr_wqes
Thanks
>
> --
> Kees Cook
On Wed, Jun 28, 2023 at 08:13:25PM -0700, Linus Torvalds wrote:
> I get a similar error in 'irdma_clr_wqes()' at
> drivers/infiniband/hw/irdma/uk.c:103 (and same thing on line 105). I
> don't see what the right solution there is, but it looks like we have
>
> IRDMA_CQP_WQE_SIZE = 8
> __le64 elem[IRDMA_CQP_WQE_SIZE];
>
> and it's doing a 4kB memset to that element. The mistake is not as
> obvious as in the cesa driver.
I think this fix is in the RDMA PR i just sent you:
commit b002760f877c0d91ecd3c78565b52f4bbac379dd
Author: Arnd Bergmann <[email protected]>
Date: Tue May 23 13:18:45 2023 +0200
RDMA/irdma: avoid fortify-string warning in irdma_clr_wqes
Commit df8fc4e934c1 ("kbuild: Enable -fstrict-flex-arrays=3") triggers a
warning for fortified memset():
In function 'fortify_memset_chk',
inlined from 'irdma_clr_wqes' at drivers/infiniband/hw/irdma/uk.c:103:4:
include/linux/fortify-string.h:493:25: error: call to '__write_overflow_field' declared with attribute warning: detected write b>
493 | __write_overflow_field(p_size_field, size);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
The problem here isthat the inner array only has four 8-byte elements, so
clearing 4096 bytes overflows that. As this structure is part of an outer
array, change the code to pass a pointer to the irdma_qp_quanta instead,
and change the size argument for readability, matching the comment above
it.
Fixes: 551c46edc769 ("RDMA/irdma: Add user/kernel shared libraries")
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Arnd Bergmann <[email protected]>
Acked-by: Shiraz Saleem <[email protected]>
Signed-off-by: Jason Gunthorpe <[email protected]>
Jason