Re: [PATCH v7 1/2] crypto: add blake2b generic implementation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Oct 24, 2019 at 10:15:50PM -0700, Eric Biggers wrote:
> On Thu, Oct 24, 2019 at 06:28:31PM +0200, David Sterba wrote:
> > The patch brings support of several BLAKE2 variants (2b with various
> > digest lengths).  The keyed digest is supported, using tfm->setkey call.
> > The in-tree user will be btrfs (for checksumming), we're going to use
> > the BLAKE2b-256 variant.
> > 
> > The code is reference implementation taken from the official sources and
> > modified in terms of kernel coding style (whitespace, comments, uintXX_t
> > -> uXX types, removed unused prototypes and #ifdefs, removed testing
> > code, changed secure_zero_memory -> memzero_explicit, used own helpers
> > for unaligned reads/writes and rotations).
> > 
> > Further changes removed sanity checks of key length or output size,
> > these values are verified in the crypto API callbacks or hardcoded in
> > shash_alg and not exposed to users.
> > 
> > Signed-off-by: David Sterba <dsterba@xxxxxxxx>
> > ---
> >  crypto/Kconfig           |  17 ++
> >  crypto/Makefile          |   1 +
> >  crypto/blake2b_generic.c | 435 +++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 453 insertions(+)
> >  create mode 100644 crypto/blake2b_generic.c
> > 
> 
> This looks good enough now, though it would be nice to clean up some more of the
> cruft that isn't actually needed and add a few of the optimizations from the
> BLAKE2s code that is being proposed.  E.g. we could easily save 120 lines with
> the following:

While I see your point, my intention is to avoid changes beyond
formatting and interface adjustments so the kernel code would not
diverge from the reference one. IMHO having this as a separate commit is
a good thing.

Further changes would need another round of review(s) and testing and
then it should be called 'based on reference code and heavily modified',
like the blake2s patch says.

The proposed patch looks good, though I'd rather address that
independently and perhaps also split it to individual changes so it's
clear how the code is transformed.



[Index of Archives]     [Kernel]     [Gnu Classpath]     [Gnu Crypto]     [DM Crypt]     [Netfilter]     [Bugtraq]

  Powered by Linux