Message-ID: <1460481936.2705.1.camel@decadent.org.uk>
Subject: Re: [PATCH 4.5 079/238] crypto: ccp - Dont assume export/import
 areas are aligned
From: Ben Hutchings <ben@decadent.org.uk>
To: Tom Lendacky <thomas.lendacky@amd.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-kernel@vger.kernel.org, stable@vger.kernel.org,
        Herbert Xu <herbert@gondor.apana.org.au>
Date: Tue, 12 Apr 2016 18:25:36 +0100
In-Reply-To: <570D29DD.5000001@amd.com>
References: <20160410183456.398741366@linuxfoundation.org>
	 <20160410183500.908546902@linuxfoundation.org>
	 <1460426212.25201.98.camel@decadent.org.uk>
	 <20160412142841.GH7996@kroah.com> <570D29DD.5000001@amd.com>
Content-Type: multipart/signed; micalg="pgp-sha512";
	protocol="application/pgp-signature"; boundary="=-g1EgAU6qBikIR5IQaHGa"
Mime-Version: 1.0
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2896
Lines: 73


--=-g1EgAU6qBikIR5IQaHGa
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Tue, 2016-04-12 at 12:01 -0500, Tom Lendacky wrote:
> On 04/12/2016 09:28 AM, Greg Kroah-Hartman wrote:
> >=20
> > On Tue, Apr 12, 2016 at 02:56:52AM +0100, Ben Hutchings wrote:
> > >=20
> > > On Sun, 2016-04-10 at 11:34 -0700, Greg Kroah-Hartman wrote:
[...]
> > > > -	state->type =3D rctx->type;
> > > > -	state->msg_bits =3D rctx->msg_bits;
> > > > -	state->first =3D rctx->first;
> > > > -	memcpy(state->ctx, rctx->ctx, sizeof(state->ctx));
> > > > -	state->buf_count =3D rctx->buf_count;
> > > > -	memcpy(state->buf, rctx->buf, sizeof(state->buf));
> > > > +	state.type =3D rctx->type;
> > > > +	state.msg_bits =3D rctx->msg_bits;
> > > > +	state.first =3D rctx->first;
> > > > +	memcpy(state.ctx, rctx->ctx, sizeof(state.ctx));
> > > > +	state.buf_count =3D rctx->buf_count;
> > > > +	memcpy(state.buf, rctx->buf, sizeof(state.buf));
> > > > +
> > > > +	/* 'out' may not be aligned so memcpy from local variable */
> > > > +	memcpy(out, &state, sizeof(state));
> > > [...]
> > >=20
> > > The padding was not initialised, but here we copy it to userland.
> > Nice catch.=C2=A0=C2=A0Given that the user/kernel structure here doesn'=
t seem very
> > sane (implicit padding, etc.), shouldn't that be where this is fixed up
> > to be a properly packed structure?=C2=A0=C2=A0Or have padding where nee=
ded, along
> > with a memset() call?
> The structure is not meant for use outside the kernel - it's an opaque
> blob that will be processed by the driver import function. So would it
> be enough to just memset the struct ccp_sha_exp_ctx state variable to 0
> before setting and copying it?=C2=A0=C2=A0That should take care of any pa=
dding not
> being initialized.

I think that would be enough.

Ben.

--=20
Ben Hutchings
This sentence contradicts itself - no actually it doesn't.
--=-g1EgAU6qBikIR5IQaHGa
Content-Type: application/pgp-signature; name="signature.asc"
Content-Description: This is a digitally signed message part

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAABCgAGBQJXDS+QAAoJEOe/yOyVhhEJ9lMQAK7G7Q8UeEsuZTAwql7uORTF
T3DYrnL1Y2hRaZTmyDSd02EN1MiXxgVsDImqZdRDyQq1J+Dh+U//FSI75gnHiaE5
TSehgEhOjrtTVkIHCOYcTsdazmG5RnRXtzfNu+cGQHEwGH3lkemSHHCjB7fhkB4W
/IdF3YX92w7MHgeRcE0Glh9eVR3bIZkAA/iEybqfmnIGpOqKUzNoTIEIc2FuG/JL
CTsMU3P5J0vG14OQFkXODrH3UpzHM+wp6Y+O4JQw8kG3gjnV0l+sInxBxf+BHES8
mbvqn2LyoutwHgo5l1OrHOVwERlb71ZYkPP5N6K1NBfbCGxrEJe8pb5JBkK0e84R
dlZ08N2B45aPfypdxGxdN1/FAEeFKSVy+aNV/1lkbjjwuebMtPMxPbC82DOGlpEp
xpi2ZYWD7JFytEKYwH3Oeg5X9mrgCjZ9TUG7jl6N8h8TgXbEbkZuMkPGE1TDP+KE
ac/Rrw66ISN6Z3HIAXI0rpMdNxwqBeY1fPSZvHrfXQZURG4BACxmqqEFmxuuCQqw
u5dghpXUdWa+xfP6aUuz4UvGXFqJ9b3DC0CQye0K8I9D6Bj1L1iMbSQw/rJQgEmU
HKHXEPVcIXVE80+ZQCZ7abz2pNNaKIUs1ncSnRdYoMaTWW0C17UWST0Jj8IRjqQN
5+tFNJzsAKv1VMBLgNyL
=3csB
-----END PGP SIGNATURE-----

--=-g1EgAU6qBikIR5IQaHGa--