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

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

Hi Herbert,

On Fri, Dec 01, 2017 at 09:35:52PM +1100, Herbert Xu wrote:
> On Fri, Dec 01, 2017 at 09:11:57AM +0100, Antoine Tenart wrote:
> >
> > I agree this should not be the case.
> > 
> > But:
> > - Other drivers are doing this check (grep "if (!req->result)" or
> >   "if (req->result)" to see some of them).
> > - I see at least one commit fixing the exact same issue I'm facing here,
> >   393897c5156a415533ff85aa381458840417b032:
> > 
> >     crypto: ccp - Check for caller result area before using it
> > 
> >     For a hash operation, the caller doesn't have to supply a result
> >     area on every call so don't use it / update it if it hasn't
> >     been supplied.
> > 
> > I'm not entirely sure what was the code path that leads to this, I'll
> > reproduce the issue and try to understand what is going on (I clearly
> > recall having this crash though).
> 
> That's different.  In that case an unconditional copy is made
> regardless of whether the operation is final or update.  That's
> why a check is required.
> 
> If the operation is finup/final/digest then req->result must be
> set and you don't need to check it.

Ah, I didn't understand your point then. Of course ->result should be
allocated for finup/final/digest. The function where I fix this is
called regardless of the operation that was performed, so it can be an
update() as well.

Thanks,
Antoine

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