[PATCH] crypto: qat - set to zero DH parameters before free

[PATCH] crypto: qat - set to zero DH parameters before free

[Giovanni Cabiddu]

Set to zero the context buffers containing the DH key before they are
freed.
This is a defense in depth measure that avoids keys to be recovered from
memory in case the system is compromised between the free of the buffer
and when that area of memory (containing keys) gets overwritten.

Cc: stable@vger.kernel.org
Fixes: c9839143ebbf ("crypto: qat - Add DH support")
Signed-off-by: Giovanni Cabiddu <giovanni.cabiddu@intel.com>
Reviewed-by: Adam Guerin <adam.guerin@intel.com>
Reviewed-by: Wojciech Ziemba <wojciech.ziemba@intel.com>
---
 drivers/crypto/qat/qat_common/qat_asym_algs.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/crypto/qat/qat_common/qat_asym_algs.c b/drivers/crypto/qat/qat_common/qat_asym_algs.c
index b0b78445418b..5633f9df3b6f 100644
--- a/drivers/crypto/qat/qat_common/qat_asym_algs.c
+++ b/drivers/crypto/qat/qat_common/qat_asym_algs.c
@@ -420,14 +420,17 @@ static int qat_dh_set_params(struct qat_dh_ctx *ctx, struct dh *params)
 static void qat_dh_clear_ctx(struct device *dev, struct qat_dh_ctx *ctx)
 {
 	if (ctx->g) {
+		memset(ctx->g, 0, ctx->p_size);
 		dma_free_coherent(dev, ctx->p_size, ctx->g, ctx->dma_g);
 		ctx->g = NULL;
 	}
 	if (ctx->xa) {
+		memset(ctx->xa, 0, ctx->p_size);
 		dma_free_coherent(dev, ctx->p_size, ctx->xa, ctx->dma_xa);
 		ctx->xa = NULL;
 	}
 	if (ctx->p) {
+		memset(ctx->p, 0, ctx->p_size);
 		dma_free_coherent(dev, ctx->p_size, ctx->p, ctx->dma_p);
 		ctx->p = NULL;
 	}
-- 
2.35.1

Re: [PATCH] crypto: qat - set to zero DH parameters before free

[Greg KH]

On Mon, May 09, 2022 at 02:19:27PM +0100, Giovanni Cabiddu wrote:
> Set to zero the context buffers containing the DH key before they are
> freed.
> This is a defense in depth measure that avoids keys to be recovered from
> memory in case the system is compromised between the free of the buffer
> and when that area of memory (containing keys) gets overwritten.
> 
> Cc: stable@vger.kernel.org
> Fixes: c9839143ebbf ("crypto: qat - Add DH support")
> Signed-off-by: Giovanni Cabiddu <giovanni.cabiddu@intel.com>
> Reviewed-by: Adam Guerin <adam.guerin@intel.com>
> Reviewed-by: Wojciech Ziemba <wojciech.ziemba@intel.com>
> ---
>  drivers/crypto/qat/qat_common/qat_asym_algs.c | 3 +++
>  1 file changed, 3 insertions(+)

Why isn't this part of the other series for this "driver"?

thanks,

greg k-h

Re: [PATCH] crypto: qat - set to zero DH parameters before free

[Giovanni Cabiddu]

On Mon, May 09, 2022 at 04:09:55PM +0200, Greg KH wrote:
> On Mon, May 09, 2022 at 02:19:27PM +0100, Giovanni Cabiddu wrote:
> > Set to zero the context buffers containing the DH key before they are
> > freed.
> > This is a defense in depth measure that avoids keys to be recovered from
> > memory in case the system is compromised between the free of the buffer
> > and when that area of memory (containing keys) gets overwritten.
> > 
> > Cc: stable@vger.kernel.org
> > Fixes: c9839143ebbf ("crypto: qat - Add DH support")
> > Signed-off-by: Giovanni Cabiddu <giovanni.cabiddu@intel.com>
> > Reviewed-by: Adam Guerin <adam.guerin@intel.com>
> > Reviewed-by: Wojciech Ziemba <wojciech.ziemba@intel.com>
> > ---
> >  drivers/crypto/qat/qat_common/qat_asym_algs.c | 3 +++
> >  1 file changed, 3 insertions(+)
> 
> Why isn't this part of the other series for this "driver"?
Just for consistency.
I preferred to decouple this from the set `crypto: qat - re-enable algorithms`
since differently from the other patches in that set, this is not fixing
a functional issue in the driver but it is adding a protection measure.

Regards,

-- 
Giovanni

Re: [PATCH] crypto: qat - set to zero DH parameters before free

[Herbert Xu]

On Mon, May 09, 2022 at 02:19:27PM +0100, Giovanni Cabiddu wrote:
> Set to zero the context buffers containing the DH key before they are
> freed.
> This is a defense in depth measure that avoids keys to be recovered from
> memory in case the system is compromised between the free of the buffer
> and when that area of memory (containing keys) gets overwritten.
> 
> Cc: stable@vger.kernel.org
> Fixes: c9839143ebbf ("crypto: qat - Add DH support")
> Signed-off-by: Giovanni Cabiddu <giovanni.cabiddu@intel.com>
> Reviewed-by: Adam Guerin <adam.guerin@intel.com>
> Reviewed-by: Wojciech Ziemba <wojciech.ziemba@intel.com>
> ---
>  drivers/crypto/qat/qat_common/qat_asym_algs.c | 3 +++
>  1 file changed, 3 insertions(+)

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