All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thorsten Blum <thorsten.blum@linux.dev>
To: Eric Biggers <ebiggers@kernel.org>
Cc: Herbert Xu <herbert@gondor.apana.org.au>,
	"David S. Miller" <davem@davemloft.net>,
	linux-crypto@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/2] crypto: blake2s - use memcpy_and_pad in __blake2s_init
Date: Wed, 15 Apr 2026 14:56:11 +0200	[thread overview]
Message-ID: <ad-K6-ohL0AZoOQk@linux.dev> (raw)
In-Reply-To: <20260414173915.GB24456@quark>

On Tue, Apr 14, 2026 at 10:39:15AM -0700, Eric Biggers wrote:
> On Tue, Apr 14, 2026 at 05:49:04PM +0200, Thorsten Blum wrote:
> > Use memcpy_and_pad() instead of memcpy() followed by memset() to
> > simplify __blake2s_init(). Use sizeof(ctx->buf) instead of the macro
> > BLAKE2S_BLOCK_SIZE.
> > 
> > Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> > ---
> >  include/crypto/blake2s.h | 5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)
> > 
> > diff --git a/include/crypto/blake2s.h b/include/crypto/blake2s.h
> > index 648cb7824358..f0e0ce0b30a5 100644
> > --- a/include/crypto/blake2s.h
> > +++ b/include/crypto/blake2s.h
> > @@ -70,9 +70,8 @@ static inline void __blake2s_init(struct blake2s_ctx *ctx, size_t outlen,
> >  	ctx->buflen = 0;
> >  	ctx->outlen = outlen;
> >  	if (keylen) {
> > -		memcpy(ctx->buf, key, keylen);
> > -		memset(&ctx->buf[keylen], 0, BLAKE2S_BLOCK_SIZE - keylen);
> > -		ctx->buflen = BLAKE2S_BLOCK_SIZE;
> > +		memcpy_and_pad(ctx->buf, sizeof(ctx->buf), key, keylen, 0);
> > +		ctx->buflen = sizeof(ctx->buf);
> 
> I'm wondering if this is actually better.  It's another helper function
> to remember.  Also 'keylen' can be a compile-time constant here, and
> compilers know what memcpy() and memset() do, so they will optimize the
> code accordingly.  The helper function takes away the compiler's ability
> to perform this optimization.  If this was already an out-of-line
> function, it would be a bit more convincing.

My motivation was readability/maintainability and avoiding the manual
tail-size arithmetic.  memcpy_and_pad() is just a thin wrapper around
memcpy()/memset(), with an additional safety check to prevent integer
wraparound.

Currently, nothing stops __blake2s_init() or blake2s_init_key() from
being called with keylen > BLAKE2S_BLOCK_SIZE. The WARN_ON() in
blake2s_init_key() is only available in DEBUG builds.

That said, happy to drop the patch if the explicit version is preferred
for performance reasons.

Thanks,
Thorsten

  reply	other threads:[~2026-04-15 12:56 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-14 15:49 [PATCH 1/2] crypto: blake2b - use memcpy_and_pad in __blake2b_init Thorsten Blum
2026-04-14 15:49 ` [PATCH 2/2] crypto: blake2s - use memcpy_and_pad in __blake2s_init Thorsten Blum
2026-04-14 17:39   ` Eric Biggers
2026-04-15 12:56     ` Thorsten Blum [this message]
2026-04-15 17:37       ` Eric Biggers

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ad-K6-ohL0AZoOQk@linux.dev \
    --to=thorsten.blum@linux.dev \
    --cc=davem@davemloft.net \
    --cc=ebiggers@kernel.org \
    --cc=herbert@gondor.apana.org.au \
    --cc=linux-crypto@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.