Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1262981 > unrolled thread

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

Back to article view | Back to linux.kernel

Contents

  [PATCH] crypto: sun4i-ss: add missing statesize LABBE Corentin <clabbe.montjoie@gmail.com> - 2015-11-05 08:50 +0100
    Re: [PATCH] crypto: sun4i-ss: add missing statesize Chen-Yu Tsai <wens@csie.org> - 2015-11-05 11:40 +0100
    Re: [PATCH] crypto: sun4i-ss: add missing statesize Maxime Ripard <maxime.ripard@free-electrons.com> - 2015-11-05 17:10 +0100
      Re: [PATCH] crypto: sun4i-ss: add missing statesize Herbert Xu <herbert@gondor.apana.org.au> - 2015-11-06 06:00 +0100
        Re: [PATCH] crypto: sun4i-ss: add missing statesize Maxime Ripard <maxime.ripard@free-electrons.com> - 2015-11-08 19:00 +0100
          Re: [PATCH] crypto: sun4i-ss: add missing statesize Christoph Biedl <linux-kernel.bfrz@manchmal.in-ulm.de> - 2015-11-08 19:30 +0100
            Re: [PATCH] crypto: sun4i-ss: add missing statesize Herbert Xu <herbert@gondor.apana.org.au> - 2015-11-09 05:00 +0100
              Re: [PATCH] crypto: sun4i-ss: add missing statesize Maxime Ripard <maxime.ripard@free-electrons.com> - 2015-11-11 23:30 +0100

#1262981 — [PATCH] crypto: sun4i-ss: add missing statesize

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@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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1263081

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@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
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1263366

[Multipart message — attachments visible in raw view] — view raw

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@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

[toc] | [prev] | [next] | [standalone]

#1263739

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@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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1265146

[Multipart message — attachments visible in raw view] — view raw

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@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

[toc] | [prev] | [next] | [standalone]

#1265163

[Multipart message — attachments visible in raw view] — view raw

Maxime Ripard wrote...

> > > > This patch specifiy statesize for sha1 and md5.
> > > > 
> > > > Signed-off-by: LABBE Corentin <clabbe.montjoie@gmail.com>
> > > > Cc: stable@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

[toc] | [prev] | [next] | [standalone]

#1265325

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1267488

[Multipart message — attachments visible in raw view] — view raw

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web