crypto: marvell/cesa - Fix type mismatch warning

Message ID 20230523083313.899332-1-arnd@kernel.org
State New
Headers
Series crypto: marvell/cesa - Fix type mismatch warning |

Commit Message

Arnd Bergmann May 23, 2023, 8:33 a.m. UTC
  From: Arnd Bergmann <arnd@arndb.de>

Commit df8fc4e934c1 ("kbuild: Enable -fstrict-flex-arrays=3") uncovered
a type mismatch in cesa 3des support that leads to a memcpy beyond the
end of a structure:

In function 'fortify_memcpy_chk',
    inlined from 'mv_cesa_des3_ede_setkey' at 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]
  583 |                         __write_overflow_field(p_size_field, size);
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

This is probably harmless as the actual data that is copied has the correct
type, but clearly worth fixing nonetheless.

Fixes: 4ada48397823 ("crypto: marvell/cesa - add Triple-DES support")
Cc: Kees Cook <keescook@chromium.org>
Cc: Gustavo A. R. Silva" <gustavoars@kernel.org>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/crypto/marvell/cesa/cipher.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Comments

Kees Cook May 23, 2023, 5:04 p.m. UTC | #1
On Tue, May 23, 2023 at 10:33:04AM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> Commit df8fc4e934c1 ("kbuild: Enable -fstrict-flex-arrays=3") uncovered
> a type mismatch in cesa 3des support that leads to a memcpy beyond the
> end of a structure:
> 
> In function 'fortify_memcpy_chk',
>     inlined from 'mv_cesa_des3_ede_setkey' at 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]
>   583 |                         __write_overflow_field(p_size_field, size);
>       |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> This is probably harmless as the actual data that is copied has the correct
> type, but clearly worth fixing nonetheless.
> 
> Fixes: 4ada48397823 ("crypto: marvell/cesa - add Triple-DES support")
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Gustavo A. R. Silva" <gustavoars@kernel.org>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Reviewed-by: Kees Cook <keescook@chromium.org>
  
Kees Cook May 30, 2023, 10:48 p.m. UTC | #2
On Tue, 23 May 2023 10:33:04 +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> Commit df8fc4e934c1 ("kbuild: Enable -fstrict-flex-arrays=3") uncovered
> a type mismatch in cesa 3des support that leads to a memcpy beyond the
> end of a structure:
> 
> In function 'fortify_memcpy_chk',
>     inlined from 'mv_cesa_des3_ede_setkey' at 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]
>   583 |                         __write_overflow_field(p_size_field, size);
>       |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> [...]

Applied to for-next/hardening, thanks!

[1/1] crypto: marvell/cesa - Fix type mismatch warning
      https://git.kernel.org/kees/c/37f3abddda8d
  
Herbert Xu May 31, 2023, 10:58 a.m. UTC | #3
On Tue, May 30, 2023 at 03:48:49PM -0700, Kees Cook wrote:
> On Tue, 23 May 2023 10:33:04 +0200, Arnd Bergmann wrote:
> > From: Arnd Bergmann <arnd@arndb.de>
> > 
> > Commit df8fc4e934c1 ("kbuild: Enable -fstrict-flex-arrays=3") uncovered
> > a type mismatch in cesa 3des support that leads to a memcpy beyond the
> > end of a structure:
> > 
> > In function 'fortify_memcpy_chk',
> >     inlined from 'mv_cesa_des3_ede_setkey' at 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]
> >   583 |                         __write_overflow_field(p_size_field, size);
> >       |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > 
> > [...]
> 
> Applied to for-next/hardening, thanks!
> 
> [1/1] crypto: marvell/cesa - Fix type mismatch warning
>       https://git.kernel.org/kees/c/37f3abddda8d

Why did you apply it to your tree? This patch makes sense on its
own regardless of the fortify changes.

Thanks,
  
Kees Cook May 31, 2023, 4:31 p.m. UTC | #4
On Wed, May 31, 2023 at 06:58:10PM +0800, Herbert Xu wrote:
> On Tue, May 30, 2023 at 03:48:49PM -0700, Kees Cook wrote:
> > On Tue, 23 May 2023 10:33:04 +0200, Arnd Bergmann wrote:
> > > From: Arnd Bergmann <arnd@arndb.de>
> > > 
> > > Commit df8fc4e934c1 ("kbuild: Enable -fstrict-flex-arrays=3") uncovered
> > > a type mismatch in cesa 3des support that leads to a memcpy beyond the
> > > end of a structure:
> > > 
> > > In function 'fortify_memcpy_chk',
> > >     inlined from 'mv_cesa_des3_ede_setkey' at 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]
> > >   583 |                         __write_overflow_field(p_size_field, size);
> > >       |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > 
> > > [...]
> > 
> > Applied to for-next/hardening, thanks!
> > 
> > [1/1] crypto: marvell/cesa - Fix type mismatch warning
> >       https://git.kernel.org/kees/c/37f3abddda8d
> 
> Why did you apply it to your tree? This patch makes sense on its
> own regardless of the fortify changes.

I snagged it since a week had gone by with no additional discussion and
it fixed an issue exposed by work in the hardening tree. Let me know if
you'd prefer I drop it for you to carry instead.

-Kees
  
Herbert Xu June 1, 2023, 10:18 a.m. UTC | #5
On Wed, May 31, 2023 at 09:31:18AM -0700, Kees Cook wrote:
.
> I snagged it since a week had gone by with no additional discussion and
> it fixed an issue exposed by work in the hardening tree. Let me know if
> you'd prefer I drop it for you to carry instead.

Yes because these sort of changes cause unnecessary conflicts.
It's not as if the patch depends on something in the hardening
tree.

Thanks,
  
Kees Cook June 1, 2023, 2:09 p.m. UTC | #6
On Thu, Jun 01, 2023 at 06:18:37PM +0800, Herbert Xu wrote:
> On Wed, May 31, 2023 at 09:31:18AM -0700, Kees Cook wrote:
> .
> > I snagged it since a week had gone by with no additional discussion and
> > it fixed an issue exposed by work in the hardening tree. Let me know if
> > you'd prefer I drop it for you to carry instead.
> 
> Yes because these sort of changes cause unnecessary conflicts.
> It's not as if the patch depends on something in the hardening
> tree.

Done! :)
  
Herbert Xu June 2, 2023, 10:23 a.m. UTC | #7
On Tue, May 23, 2023 at 10:33:04AM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> Commit df8fc4e934c1 ("kbuild: Enable -fstrict-flex-arrays=3") uncovered
> a type mismatch in cesa 3des support that leads to a memcpy beyond the
> end of a structure:
> 
> In function 'fortify_memcpy_chk',
>     inlined from 'mv_cesa_des3_ede_setkey' at 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]
>   583 |                         __write_overflow_field(p_size_field, size);
>       |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> This is probably harmless as the actual data that is copied has the correct
> type, but clearly worth fixing nonetheless.
> 
> Fixes: 4ada48397823 ("crypto: marvell/cesa - add Triple-DES support")
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Gustavo A. R. Silva" <gustavoars@kernel.org>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/crypto/marvell/cesa/cipher.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Patch applied.  Thanks.
  

Patch

diff --git a/drivers/crypto/marvell/cesa/cipher.c b/drivers/crypto/marvell/cesa/cipher.c
index c6f2fa753b7c..0f37dfd42d85 100644
--- a/drivers/crypto/marvell/cesa/cipher.c
+++ b/drivers/crypto/marvell/cesa/cipher.c
@@ -297,7 +297,7 @@  static int mv_cesa_des_setkey(struct crypto_skcipher *cipher, const u8 *key,
 static int mv_cesa_des3_ede_setkey(struct crypto_skcipher *cipher,
 				   const u8 *key, unsigned int len)
 {
-	struct mv_cesa_des_ctx *ctx = crypto_skcipher_ctx(cipher);
+	struct mv_cesa_des3_ctx *ctx = crypto_skcipher_ctx(cipher);
 	int err;
 
 	err = verify_skcipher_des3_key(cipher, key);