Re: [PATCH v2 3/4] crypto: inside-secure - only update the result buffer when provided

[Antoine Tenart <antoine.tenart@free-electrons.com>]

Hi Kamil,

On Thu, Nov 30, 2017 at 10:19:26AM +0100, Kamil Konieczny wrote:
> On 28.11.2017 16:42, Antoine Tenart wrote:
> > The patch fixes the ahash support by only updating the result buffer
> > when provided. Otherwise the driver could crash with NULL pointer
> > exceptions, because the ahash caller isn't required to supply a result
> > buffer on all calls.
> 
> Can you point to bug crush report ?

Do you want the crash dump? (It'll only be a "normal" NULL pointer
dereference).

> > Fixes: 1b44c5a60c13 ("crypto: inside-secure - add SafeXcel EIP197 crypto engine driver")
> > Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
> > ---
> >  drivers/crypto/inside-secure/safexcel_hash.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/crypto/inside-secure/safexcel_hash.c b/drivers/crypto/inside-secure/safexcel_hash.c
> > index 6135c9f5742c..424f4c5d4d25 100644
> > --- a/drivers/crypto/inside-secure/safexcel_hash.c
> > +++ b/drivers/crypto/inside-secure/safexcel_hash.c
> > @@ -150,7 +150,12 @@ static int safexcel_handle_req_result(struct safexcel_crypto_priv *priv, int rin
> >  
> >  	if (sreq->finish)
> >  		result_sz = crypto_ahash_digestsize(ahash);
> > -	memcpy(sreq->state, areq->result, result_sz);
> > +
> > +	/* The called isn't required to supply a result buffer. Updated it only
> > +	 * when provided.
> > +	 */
> > +	if (areq->result)
> > +		memcpy(sreq->state, areq->result, result_sz);
> >  
> >  	dma_unmap_sg(priv->dev, areq->src,
> >  		     sg_nents_for_len(areq->src, areq->nbytes), DMA_TO_DEVICE);
> > 
> 
> can the driver get request for final/finup/digest with null req->result ?

I don't think that can happen. But having an update called without
req->result provided is a valid call (though it's not well documented).

Thanks,
Antoine

-- 
Antoine Ténart, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com