[PATCH] crypto: sun4i-ss: add missing statesize

[PATCH] crypto: sun4i-ss: add missing statesize

[LABBE Corentin]

sun4i-ss implementaton of md5/sha1 is via ahash algorithms.
Commit 8996eafdcbad ("crypto: ahash - ensure statesize is non-zero")
made impossible to load them without giving statesize. This patch
specifiy statesize for sha1 and md5.

Fixes: 6298e948215f ("crypto: sunxi-ss - Add Allwinner Security System crypto accelerator")
Cc: <stable@vger.kernel.org> # v4.3+
Tested-by: Chen-Yu Tsai <wens@csie.org>
Signed-off-by: LABBE Corentin <clabbe.montjoie@gmail.com>
---
 drivers/crypto/sunxi-ss/sun4i-ss-core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/crypto/sunxi-ss/sun4i-ss-core.c b/drivers/crypto/sunxi-ss/sun4i-ss-core.c
index eab6fe2..107cd2a 100644
--- a/drivers/crypto/sunxi-ss/sun4i-ss-core.c
+++ b/drivers/crypto/sunxi-ss/sun4i-ss-core.c
@@ -39,6 +39,7 @@ static struct sun4i_ss_alg_template ss_algs[] = {
 		.import = sun4i_hash_import_md5,
 		.halg = {
 			.digestsize = MD5_DIGEST_SIZE,
+			.statesize = sizeof(struct md5_state),
 			.base = {
 				.cra_name = "md5",
 				.cra_driver_name = "md5-sun4i-ss",
@@ -66,6 +67,7 @@ static struct sun4i_ss_alg_template ss_algs[] = {
 		.import = sun4i_hash_import_sha1,
 		.halg = {
 			.digestsize = SHA1_DIGEST_SIZE,
+			.statesize = sizeof(struct sha1_state),
 			.base = {
 				.cra_name = "sha1",
 				.cra_driver_name = "sha1-sun4i-ss",
-- 
2.4.10

[PATCH] crypto: sun4i-ss: add missing statesize

[Herbert Xu]

On Fri, Nov 13, 2015 at 02:10:28PM +0100, LABBE Corentin wrote:
> sun4i-ss implementaton of md5/sha1 is via ahash algorithms.
> Commit 8996eafdcbad ("crypto: ahash - ensure statesize is non-zero")
> made impossible to load them without giving statesize. This patch
> specifiy statesize for sha1 and md5.
> 
> Fixes: 6298e948215f ("crypto: sunxi-ss - Add Allwinner Security System crypto accelerator")
> Cc: <stable@vger.kernel.org> # v4.3+
> Tested-by: Chen-Yu Tsai <wens@csie.org>
> Signed-off-by: LABBE Corentin <clabbe.montjoie@gmail.com>

Patch applied to cryptodev as I don't think it needs to go into
stable straight away.  Thanks.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[PATCH] crypto: sun4i-ss: add missing statesize

[LABBE Corentin]

On Tue, Nov 17, 2015 at 10:09:38PM +0800, Herbert Xu wrote:
> On Fri, Nov 13, 2015 at 02:10:28PM +0100, LABBE Corentin wrote:
> > sun4i-ss implementaton of md5/sha1 is via ahash algorithms.
> > Commit 8996eafdcbad ("crypto: ahash - ensure statesize is non-zero")
> > made impossible to load them without giving statesize. This patch
> > specifiy statesize for sha1 and md5.
> > 
> > Fixes: 6298e948215f ("crypto: sunxi-ss - Add Allwinner Security System crypto accelerator")
> > Cc: <stable@vger.kernel.org> # v4.3+
> > Tested-by: Chen-Yu Tsai <wens@csie.org>
> > Signed-off-by: LABBE Corentin <clabbe.montjoie@gmail.com>
> 
> Patch applied to cryptodev as I don't think it needs to go into
> stable straight away.  Thanks.

Hello

I have lots of user reporting that they cannot use the device since it wont load.
For me it need to go stable since it respect all "stable rules", simple, tested, minimal, and fix a real bug that bother people.

Thanks in advance.
Regards

[PATCH] crypto: sun4i-ss: add missing statesize

[Herbert Xu]

On Tue, Nov 17, 2015 at 03:26:03PM +0100, LABBE Corentin wrote:
>
> I have lots of user reporting that they cannot use the device since it wont load.
> For me it need to go stable since it respect all "stable rules", simple, tested, minimal, and fix a real bug that bother people.

Well it will hit stable eventually.  I just don't feel comfortable
with sending it in straight away.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[PATCH] crypto: sun4i-ss: add missing statesize

[Greg KH]

On Tue, Nov 17, 2015 at 10:27:42PM +0800, Herbert Xu wrote:
> On Tue, Nov 17, 2015 at 03:26:03PM +0100, LABBE Corentin wrote:
> >
> > I have lots of user reporting that they cannot use the device since it wont load.
> > For me it need to go stable since it respect all "stable rules", simple, tested, minimal, and fix a real bug that bother people.
> 
> Well it will hit stable eventually.  I just don't feel comfortable
> with sending it in straight away.

You can't send it in "straight away", the rule is that it has to be in
Linus's tree first, don't worry, you are doing it correctly.

thanks,

greg k-h

[PATCH] crypto: sun4i-ss: add missing statesize

[Herbert Xu]

On Mon, Nov 09, 2015 at 08:27:06AM +0100, LABBE Corentin wrote:
> sun4i-ss implementaton of md5/sha1 is via ahash algorithms.
> A recent change make impossible to load them without giving statesize.
> This patch specifiy statesize for sha1 and md5.
> 
> Fixes: 8996eafdcbad ("crypto: ahash - ensure statesize is non-zero")

Nack.  As I said this commit is absolutely correct and if you really
have to slap on a fixes header then make it the original changeset
that added the driver.  It's the driver that's broken, not the API.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[PATCH] crypto: sun4i-ss: add missing statesize

[LABBE Corentin]

sun4i-ss implementaton of md5/sha1 is via ahash algorithms.
A recent change make impossible to load them without giving statesize.
This patch specifiy statesize for sha1 and md5.

Signed-off-by: LABBE Corentin <clabbe.montjoie@gmail.com>
Cc: stable at vger.kernel.org
---
 drivers/crypto/sunxi-ss/sun4i-ss-core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/crypto/sunxi-ss/sun4i-ss-core.c b/drivers/crypto/sunxi-ss/sun4i-ss-core.c
index eab6fe2..107cd2a 100644
--- a/drivers/crypto/sunxi-ss/sun4i-ss-core.c
+++ b/drivers/crypto/sunxi-ss/sun4i-ss-core.c
@@ -39,6 +39,7 @@ static struct sun4i_ss_alg_template ss_algs[] = {
 		.import = sun4i_hash_import_md5,
 		.halg = {
 			.digestsize = MD5_DIGEST_SIZE,
+			.statesize = sizeof(struct md5_state),
 			.base = {
 				.cra_name = "md5",
 				.cra_driver_name = "md5-sun4i-ss",
@@ -66,6 +67,7 @@ static struct sun4i_ss_alg_template ss_algs[] = {
 		.import = sun4i_hash_import_sha1,
 		.halg = {
 			.digestsize = SHA1_DIGEST_SIZE,
+			.statesize = sizeof(struct sha1_state),
 			.base = {
 				.cra_name = "sha1",
 				.cra_driver_name = "sha1-sun4i-ss",
-- 
2.4.10

[PATCH] crypto: sun4i-ss: add missing statesize

[Chen-Yu Tsai]

On Thu, Nov 5, 2015 at 3:48 PM, LABBE Corentin
<clabbe.montjoie@gmail.com> wrote:
> sun4i-ss implementaton of md5/sha1 is via ahash algorithms.
> A recent change make impossible to load them without giving statesize.
> This patch specifiy statesize for sha1 and md5.
>
> Signed-off-by: LABBE Corentin <clabbe.montjoie@gmail.com>
> Cc: stable at vger.kernel.org

Confirmed the driver properly loads again.

Tested-by: Chen-Yu Tsai <wens@csie.org>

> ---
>  drivers/crypto/sunxi-ss/sun4i-ss-core.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/crypto/sunxi-ss/sun4i-ss-core.c b/drivers/crypto/sunxi-ss/sun4i-ss-core.c
> index eab6fe2..107cd2a 100644
> --- a/drivers/crypto/sunxi-ss/sun4i-ss-core.c
> +++ b/drivers/crypto/sunxi-ss/sun4i-ss-core.c
> @@ -39,6 +39,7 @@ static struct sun4i_ss_alg_template ss_algs[] = {
>                 .import = sun4i_hash_import_md5,
>                 .halg = {
>                         .digestsize = MD5_DIGEST_SIZE,
> +                       .statesize = sizeof(struct md5_state),
>                         .base = {
>                                 .cra_name = "md5",
>                                 .cra_driver_name = "md5-sun4i-ss",
> @@ -66,6 +67,7 @@ static struct sun4i_ss_alg_template ss_algs[] = {
>                 .import = sun4i_hash_import_sha1,
>                 .halg = {
>                         .digestsize = SHA1_DIGEST_SIZE,
> +                       .statesize = sizeof(struct sha1_state),
>                         .base = {
>                                 .cra_name = "sha1",
>                                 .cra_driver_name = "sha1-sun4i-ss",
> --
> 2.4.10
>

[PATCH] crypto: sun4i-ss: add missing statesize

[Maxime Ripard]

Hi,

On Thu, Nov 05, 2015 at 08:48:57AM +0100, LABBE Corentin wrote:
> sun4i-ss implementaton of md5/sha1 is via ahash algorithms.
> A recent change make impossible to load them without giving statesize.

Which one?

> This patch specifiy statesize for sha1 and md5.
> 
> Signed-off-by: LABBE Corentin <clabbe.montjoie@gmail.com>
> Cc: stable at vger.kernel.org

Please also add a Fixes tag (and the stable version it applies to).

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20151105/ae805bb8/attachment.sig>

[PATCH] crypto: sun4i-ss: add missing statesize

[Herbert Xu]

On Thu, Nov 05, 2015 at 08:07:19AM -0800, Maxime Ripard wrote:
> 
> On Thu, Nov 05, 2015 at 08:48:57AM +0100, LABBE Corentin wrote:
> > sun4i-ss implementaton of md5/sha1 is via ahash algorithms.
> > A recent change make impossible to load them without giving statesize.
> 
> Which one?

We recently disabled ahash drivers that do not declare statesize
because it can lead to a crash when the driver is used through
algif.

Not declaring statesize is a bug anyway but the fact that it
is exported through algif makes it much worse.

> > This patch specifiy statesize for sha1 and md5.
> > 
> > Signed-off-by: LABBE Corentin <clabbe.montjoie@gmail.com>
> > Cc: stable at vger.kernel.org
> 
> Please also add a Fixes tag (and the stable version it applies to).

I don't see the point for a fixes tag as it would simply refer
to the original patch-set that added the driver.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[PATCH] crypto: sun4i-ss: add missing statesize

[Maxime Ripard]

On Fri, Nov 06, 2015 at 12:56:39PM +0800, Herbert Xu wrote:
> On Thu, Nov 05, 2015 at 08:07:19AM -0800, Maxime Ripard wrote:
> > 
> > On Thu, Nov 05, 2015 at 08:48:57AM +0100, LABBE Corentin wrote:
> > > sun4i-ss implementaton of md5/sha1 is via ahash algorithms.
> > > A recent change make impossible to load them without giving statesize.
> > 
> > Which one?
> 
> We recently disabled ahash drivers that do not declare statesize
> because it can lead to a crash when the driver is used through
> algif.

"Recently" is relative and really doesn't help.

Having the commit ID that made this change is an absolute reference,
and really helps to identify when that behaviour changed.

> Not declaring statesize is a bug anyway but the fact that it
> is exported through algif makes it much worse.
> 
> > > This patch specifiy statesize for sha1 and md5.
> > > 
> > > Signed-off-by: LABBE Corentin <clabbe.montjoie@gmail.com>
> > > Cc: stable at vger.kernel.org
> > 
> > Please also add a Fixes tag (and the stable version it applies to).
> 
> I don't see the point for a fixes tag as it would simply refer
> to the original patch-set that added the driver.

What's the problem with that?

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20151108/4a203d5f/attachment.sig>

[PATCH] crypto: sun4i-ss: add missing statesize

[Christoph Biedl]

Maxime Ripard wrote...

> > > > This patch specifiy statesize for sha1 and md5.
> > > > 
> > > > Signed-off-by: LABBE Corentin <clabbe.montjoie@gmail.com>
> > > > Cc: stable at vger.kernel.org
> > > 
> > > Please also add a Fixes tag (and the stable version it applies to).
> > 
> > I don't see the point for a fixes tag as it would simply refer
> > to the original patch-set that added the driver.
> 
> What's the problem with that?

Fixes: should rather point to the commit that caused the breakage in my
opinion. Which did this by intention:

| commit 8996eafdcbad149ac0f772fb1649fbb75c482a6a
| Author: Russell King <rmk+kernel@arm.linux.org.uk>
| Date:   Fri Oct 9 20:43:33 2015 +0100
| 
|     crypto: ahash - ensure statesize is non-zero
(...)
+     This patch adds a check to prevent these drivers from registering
+     ahash algorithms until they are fixed.

Another crypto subsystem (mv_cesa) suffers from the same problem. I
have a patch ready but would prefer a consensus on these formalities
before submitting.

Aside from this, if you really need another Tested-by:, add me. And
also Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=107281

    Christoph
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20151108/2e54c0cc/attachment.sig>

[PATCH] crypto: sun4i-ss: add missing statesize

[Herbert Xu]

Christoph Biedl <linux-kernel.bfrz@manchmal.in-ulm.de> wrote:
>
> Fixes: should rather point to the commit that caused the breakage in my
> opinion. Which did this by intention:

Absolutely not.  That patch is correct and if you revert that
you will simply end up registering a broken driver into the system
that may then lead to crashes that can be triggered from user-space.

> | commit 8996eafdcbad149ac0f772fb1649fbb75c482a6a
> | Author: Russell King <rmk+kernel@arm.linux.org.uk>
> | Date:   Fri Oct 9 20:43:33 2015 +0100
> | 
> |     crypto: ahash - ensure statesize is non-zero
> (...)
> +     This patch adds a check to prevent these drivers from registering
> +     ahash algorithms until they are fixed.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[PATCH] crypto: sun4i-ss: add missing statesize

[Maxime Ripard]

On Mon, Nov 09, 2015 at 11:58:48AM +0800, Herbert Xu wrote:
> Christoph Biedl <linux-kernel.bfrz@manchmal.in-ulm.de> wrote:
> >
> > Fixes: should rather point to the commit that caused the breakage in my
> > opinion. Which did this by intention:
> 
> Absolutely not.  That patch is correct and if you revert that
> you will simply end up registering a broken driver into the system
> that may then lead to crashes that can be triggered from user-space.

Yeah my point was that the commit log should be something like

"""
sun4i-ss implementaton of md5/sha1 is via ahash algorithms.

Commit 8996eafdcbad ('crypto: ahash - ensure statesize is non-zero')
made impossible to load them without giving statesize. This patch
specifiy statesize for sha1 and md5.

Fixes: 6298e948215f ("crypto: sunxi-ss - Add Allwinner Security System crypto accelerator")
"""

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20151111/a7e0287d/attachment-0001.sig>

[PATCH] crypto: sun4i-ss: add missing statesize

[LABBE Corentin]

sun4i-ss implementaton of md5/sha1 is via ahash algorithms.
A recent change make impossible to load them without giving statesize.
This patch specifiy statesize for sha1 and md5.

Signed-off-by: LABBE Corentin <clabbe.montjoie@gmail.com>
---
 drivers/crypto/sunxi-ss/sun4i-ss-core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/crypto/sunxi-ss/sun4i-ss-core.c b/drivers/crypto/sunxi-ss/sun4i-ss-core.c
index eab6fe2..107cd2a 100644
--- a/drivers/crypto/sunxi-ss/sun4i-ss-core.c
+++ b/drivers/crypto/sunxi-ss/sun4i-ss-core.c
@@ -39,6 +39,7 @@ static struct sun4i_ss_alg_template ss_algs[] = {
 		.import = sun4i_hash_import_md5,
 		.halg = {
 			.digestsize = MD5_DIGEST_SIZE,
+			.statesize = sizeof(struct md5_state),
 			.base = {
 				.cra_name = "md5",
 				.cra_driver_name = "md5-sun4i-ss",
@@ -66,6 +67,7 @@ static struct sun4i_ss_alg_template ss_algs[] = {
 		.import = sun4i_hash_import_sha1,
 		.halg = {
 			.digestsize = SHA1_DIGEST_SIZE,
+			.statesize = sizeof(struct sha1_state),
 			.base = {
 				.cra_name = "sha1",
 				.cra_driver_name = "sha1-sun4i-ss",
-- 
2.4.10