2020-09-08 07:26:14

by Van Leeuwen, Pascal

Subject: [PATCH] crypto: inside-secure - Prevent missing of processing errors

On systems with coherence issues, packet processed could succeed while
it should have failed, e.g. because of an authentication fail.
This is because the driver would read stale status information that had
all error bits initialised to zero = no error.
Since this is potential a security risk, we want to prevent it from being
a possibility at all. So initialize all error bits to error state, so
that reading stale status information will always result in errors.

Signed-off-by: Pascal van Leeuwen <[email protected]>
---
drivers/crypto/inside-secure/safexcel_ring.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/crypto/inside-secure/safexcel_ring.c b/drivers/crypto/inside-secure/safexcel_ring.c
index e454c3d..90f1503 100644
--- a/drivers/crypto/inside-secure/safexcel_ring.c
+++ b/drivers/crypto/inside-secure/safexcel_ring.c
@@ -236,8 +236,8 @@ struct safexcel_result_desc *safexcel_add_rdesc(struct safexcel_crypto_priv *pri

rdesc->particle_size = len;
rdesc->rsvd0 = 0;
- rdesc->descriptor_overflow = 0;
- rdesc->buffer_overflow = 0;
+ rdesc->descriptor_overflow = 1; /* assume error */
+ rdesc->buffer_overflow = 1; /* assume error */
rdesc->last_seg = last;
rdesc->first_seg = first;
rdesc->result_size = EIP197_RD64_RESULT_SIZE;
@@ -245,9 +245,10 @@ struct safexcel_result_desc *safexcel_add_rdesc(struct safexcel_crypto_priv *pri
rdesc->data_lo = lower_32_bits(data);
rdesc->data_hi = upper_32_bits(data);

- /* Clear length & error code in result token */
+ /* Clear length in result token */
rtoken->packet_length = 0;
- rtoken->error_code = 0;
+ /* Assume errors - HW will clear if not the case */
+ rtoken->error_code = 0x7fff;

return rdesc;
}
--
1.8.3.1

2020-09-16 09:29:06

by Antoine Tenart

Subject: Re: [PATCH] crypto: inside-secure - Prevent missing of processing errors

Hi Pascal,

Quoting Pascal van Leeuwen (2020-09-08 08:10:45)
> On systems with coherence issues, packet processed could succeed while
> it should have failed, e.g. because of an authentication fail.
> This is because the driver would read stale status information that had
> all error bits initialised to zero = no error.
> Since this is potential a security risk, we want to prevent it from being
> a possibility at all. So initialize all error bits to error state, so
> that reading stale status information will always result in errors.
>
> Signed-off-by: Pascal van Leeuwen <[email protected]>

Acked-by: Antoine Tenart <[email protected]>

Thanks!
Antoine

> ---
> drivers/crypto/inside-secure/safexcel_ring.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/crypto/inside-secure/safexcel_ring.c b/drivers/crypto/inside-secure/safexcel_ring.c
> index e454c3d..90f1503 100644
> --- a/drivers/crypto/inside-secure/safexcel_ring.c
> +++ b/drivers/crypto/inside-secure/safexcel_ring.c
> @@ -236,8 +236,8 @@ struct safexcel_result_desc *safexcel_add_rdesc(struct safexcel_crypto_priv *pri
>
> rdesc->particle_size = len;
> rdesc->rsvd0 = 0;
> - rdesc->descriptor_overflow = 0;
> - rdesc->buffer_overflow = 0;
> + rdesc->descriptor_overflow = 1; /* assume error */
> + rdesc->buffer_overflow = 1; /* assume error */
> rdesc->last_seg = last;
> rdesc->first_seg = first;
> rdesc->result_size = EIP197_RD64_RESULT_SIZE;
> @@ -245,9 +245,10 @@ struct safexcel_result_desc *safexcel_add_rdesc(struct safexcel_crypto_priv *pri
> rdesc->data_lo = lower_32_bits(data);
> rdesc->data_hi = upper_32_bits(data);
>
> - /* Clear length & error code in result token */
> + /* Clear length in result token */
> rtoken->packet_length = 0;
> - rtoken->error_code = 0;
> + /* Assume errors - HW will clear if not the case */
> + rtoken->error_code = 0x7fff;
>
> return rdesc;
> }
> --
> 1.8.3.1
>

--
Antoine Ténart, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

2020-09-18 07:31:34

by Herbert Xu

Subject: Re: [PATCH] crypto: inside-secure - Prevent missing of processing errors

On Tue, Sep 08, 2020 at 08:10:45AM +0200, Pascal van Leeuwen wrote:
> On systems with coherence issues, packet processed could succeed while
> it should have failed, e.g. because of an authentication fail.
> This is because the driver would read stale status information that had
> all error bits initialised to zero = no error.
> Since this is potential a security risk, we want to prevent it from being
> a possibility at all. So initialize all error bits to error state, so
> that reading stale status information will always result in errors.
>
> Signed-off-by: Pascal van Leeuwen <[email protected]>
> ---
> drivers/crypto/inside-secure/safexcel_ring.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 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