Date:   Tue, 2 Apr 2019 09:57:49 -0700
From:   Eric Biggers <ebiggers@kernel.org>
To:     Vitaly Chikunov <vt@altlinux.org>
Cc:     linux-crypto@vger.kernel.org
Subject: Re: [RFC/RFT PATCH 09/18] crypto: streebog - fix unaligned memory
 accesses
Message-ID: <20190402165747.GA13413@gmail.com>
References: <20190331200428.26597-1-ebiggers@kernel.org>
 <20190331200428.26597-10-ebiggers@kernel.org>
 <20190331214717.4erxk2racxphfbha@altlinux.org>
 <20190402161557.lg37muib4qy4az22@altlinux.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20190402161557.lg37muib4qy4az22@altlinux.org>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-crypto-owner@vger.kernel.org
Precedence: bulk

On Tue, Apr 02, 2019 at 07:15:57PM +0300, Vitaly Chikunov wrote:
> > >  
> > >  static void streebog_stage2(struct streebog_state *ctx, const u8 *data)
> > >  {
> > > -	streebog_g(&ctx->h, &ctx->N, data);
> > > +	struct streebog_uint512 m;
> > > +
> > > +	memcpy(&m, data, sizeof(m));
> > > +
> > > +	streebog_g(&ctx->h, &ctx->N, &m);
> > >  
> > >  	streebog_add512(&ctx->N, &buffer512, &ctx->N);
> > > -	streebog_add512(&ctx->Sigma, (const struct streebog_uint512 *)data,
> > > -			&ctx->Sigma);
> > > +	streebog_add512(&ctx->Sigma, &m, &ctx->Sigma);
> > >  }
> > 
> > As I understand, this is the actual fix.
> 
> Probably, even better would be to use CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
> to optimize out memcpy() for such architectures.
> 

Having multiple code paths is more error-prone, and contrary to popular belief
you can't break alignment rules without informing the compiler, even when
CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS.  See
https://patchwork.kernel.org/cover/10631429/.

If you want to code up something yourself using get_unaligned_le64() or
__attribute__((packed)), that probably would be the way to go.  But for now I
just want to fix it to not cause a test failure.  I don't have any particular
interest in optimizing Streebog myself, especially the C implementation (if you
really cared about performance you'd add an assembly implementation).

- Eric