[PATCH] crypto: inside-secure: fix packed bit-field result descriptor

[PATCH] crypto: inside-secure: fix packed bit-field result descriptor

[oferh]

From: Ofer Heifetz <oferh@marvell.com>

When mixing bit-field and none bit-filed in packed struct the
none bit-field starts at a distinct memory location, thus adding
an additional byte to the overall structure which is used in
memory zero-ing and other configuration calculations.

Fix this by removing the none bit-field that has a following
bit-field.

Signed-off-by: Ofer Heifetz <oferh@marvell.com>
---
 drivers/crypto/inside-secure/safexcel.h | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/crypto/inside-secure/safexcel.h b/drivers/crypto/inside-secure/safexcel.h
index ce1e611a163e..797ff91512e0 100644
--- a/drivers/crypto/inside-secure/safexcel.h
+++ b/drivers/crypto/inside-secure/safexcel.h
@@ -497,15 +497,15 @@ struct result_data_desc {
 	u32 packet_length:17;
 	u32 error_code:15;
 
-	u8 bypass_length:4;
-	u8 e15:1;
-	u16 rsvd0;
-	u8 hash_bytes:1;
-	u8 hash_length:6;
-	u8 generic_bytes:1;
-	u8 checksum:1;
-	u8 next_header:1;
-	u8 length:1;
+	u32 bypass_length:4;
+	u32 e15:1;
+	u32 rsvd0:16;
+	u32 hash_bytes:1;
+	u32 hash_length:6;
+	u32 generic_bytes:1;
+	u32 checksum:1;
+	u32 next_header:1;
+	u32 length:1;
 
 	u16 application_id;
 	u16 rsvd1;
-- 
2.25.1

Re: [PATCH] crypto: inside-secure: fix packed bit-field result descriptor

[Antoine Tenart]

Hi Ofer,

Quoting oferh@marvell.com (2022-07-02 09:14:26)
> From: Ofer Heifetz <oferh@marvell.com>
> 
> When mixing bit-field and none bit-filed in packed struct the
> none bit-field starts at a distinct memory location, thus adding
> an additional byte to the overall structure which is used in
> memory zero-ing and other configuration calculations.
> 
> Fix this by removing the none bit-field that has a following
> bit-field.
> 
> Signed-off-by: Ofer Heifetz <oferh@marvell.com>

Nice catch!

Note: since those fields were not used before and IIRC the below result
struct size is set dynamically (the h/w doesn't expect a fixed size)
this doesn't need to be backported to stable trees. Can't test it on
real h/w though.

Acked-by: Antoine Tenart <atenart@kernel.org>

Thanks!

> ---
>  drivers/crypto/inside-secure/safexcel.h | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/crypto/inside-secure/safexcel.h b/drivers/crypto/inside-secure/safexcel.h
> index ce1e611a163e..797ff91512e0 100644
> --- a/drivers/crypto/inside-secure/safexcel.h
> +++ b/drivers/crypto/inside-secure/safexcel.h
> @@ -497,15 +497,15 @@ struct result_data_desc {
>         u32 packet_length:17;
>         u32 error_code:15;
>  
> -       u8 bypass_length:4;
> -       u8 e15:1;
> -       u16 rsvd0;
> -       u8 hash_bytes:1;
> -       u8 hash_length:6;
> -       u8 generic_bytes:1;
> -       u8 checksum:1;
> -       u8 next_header:1;
> -       u8 length:1;
> +       u32 bypass_length:4;
> +       u32 e15:1;
> +       u32 rsvd0:16;
> +       u32 hash_bytes:1;
> +       u32 hash_length:6;
> +       u32 generic_bytes:1;
> +       u32 checksum:1;
> +       u32 next_header:1;
> +       u32 length:1;
>  
>         u16 application_id;
>         u16 rsvd1;
> -- 
> 2.25.1
> 

RE: [EXT] Re: [PATCH] crypto: inside-secure: fix packed bit-field result descriptor

[Ofer Heifetz]

Hi Antoine,

The HW uses the size that is allocated to it, in our case we have an additional byte but since the descriptor size is in words (4 bytes), the division by 4 lets the HW use the correct word count (remainder byte is dropped), the only setting of the result_data_desc (LKv5.19) are fields before the none bit-field rsvd0, so it should be fine, but in older kernels like LKv5.4 we did memset(0) this structure.

I tested this change on Armada 7K.

-----Original Message-----
From: Antoine Tenart <atenart@kernel.org> 
Sent: Monday, July 4, 2022 10:26 AM
To: davem@davemloft.net; herbert@gondor.apana.org.au; linux-crypto@vger.kernel.org; Ofer Heifetz <oferh@marvell.com>
Subject: [EXT] Re: [PATCH] crypto: inside-secure: fix packed bit-field result descriptor

External Email

----------------------------------------------------------------------
Hi Ofer,

Quoting oferh@marvell.com (2022-07-02 09:14:26)
> From: Ofer Heifetz <oferh@marvell.com>
> 
> When mixing bit-field and none bit-filed in packed struct the none 
> bit-field starts at a distinct memory location, thus adding an 
> additional byte to the overall structure which is used in memory 
> zero-ing and other configuration calculations.
> 
> Fix this by removing the none bit-field that has a following 
> bit-field.
> 
> Signed-off-by: Ofer Heifetz <oferh@marvell.com>

Nice catch!

Note: since those fields were not used before and IIRC the below result struct size is set dynamically (the h/w doesn't expect a fixed size) this doesn't need to be backported to stable trees. Can't test it on real h/w though.

Acked-by: Antoine Tenart <atenart@kernel.org>

Thanks!

> ---
>  drivers/crypto/inside-secure/safexcel.h | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/crypto/inside-secure/safexcel.h 
> b/drivers/crypto/inside-secure/safexcel.h
> index ce1e611a163e..797ff91512e0 100644
> --- a/drivers/crypto/inside-secure/safexcel.h
> +++ b/drivers/crypto/inside-secure/safexcel.h
> @@ -497,15 +497,15 @@ struct result_data_desc {
>         u32 packet_length:17;
>         u32 error_code:15;
>  
> -       u8 bypass_length:4;
> -       u8 e15:1;
> -       u16 rsvd0;
> -       u8 hash_bytes:1;
> -       u8 hash_length:6;
> -       u8 generic_bytes:1;
> -       u8 checksum:1;
> -       u8 next_header:1;
> -       u8 length:1;
> +       u32 bypass_length:4;
> +       u32 e15:1;
> +       u32 rsvd0:16;
> +       u32 hash_bytes:1;
> +       u32 hash_length:6;
> +       u32 generic_bytes:1;
> +       u32 checksum:1;
> +       u32 next_header:1;
> +       u32 length:1;
>  
>         u16 application_id;
>         u16 rsvd1;
> --
> 2.25.1
> 

Re: [PATCH] crypto: inside-secure: fix packed bit-field result descriptor

[Herbert Xu]

On Sat, Jul 02, 2022 at 10:14:26AM +0300, oferh@marvell.com wrote:
> From: Ofer Heifetz <oferh@marvell.com>
> 
> When mixing bit-field and none bit-filed in packed struct the
> none bit-field starts at a distinct memory location, thus adding
> an additional byte to the overall structure which is used in
> memory zero-ing and other configuration calculations.
> 
> Fix this by removing the none bit-field that has a following
> bit-field.
> 
> Signed-off-by: Ofer Heifetz <oferh@marvell.com>
> ---
>  drivers/crypto/inside-secure/safexcel.h | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)

Patch applied.  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