From: Ofer Heifetz <[email protected]>
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 <[email protected]>
---
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
Hi Ofer,
Quoting [email protected] (2022-07-02 09:14:26)
> From: Ofer Heifetz <[email protected]>
>
> 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 <[email protected]>
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 <[email protected]>
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
>
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 <[email protected]>
Sent: Monday, July 4, 2022 10:26 AM
To: [email protected]; [email protected]; [email protected]; Ofer Heifetz <[email protected]>
Subject: [EXT] Re: [PATCH] crypto: inside-secure: fix packed bit-field result descriptor
External Email
----------------------------------------------------------------------
Hi Ofer,
Quoting [email protected] (2022-07-02 09:14:26)
> From: Ofer Heifetz <[email protected]>
>
> 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 <[email protected]>
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 <[email protected]>
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
>
On Sat, Jul 02, 2022 at 10:14:26AM +0300, [email protected] wrote:
> From: Ofer Heifetz <[email protected]>
>
> 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 <[email protected]>
> ---
> drivers/crypto/inside-secure/safexcel.h | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
Patch applied. Thanks.
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt