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 <[email protected]>
Cc: [email protected]
---
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
On Thu, Nov 5, 2015 at 3:48 PM, LABBE Corentin
<[email protected]> 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 <[email protected]>
> Cc: [email protected]
Confirmed the driver properly loads again.
Tested-by: Chen-Yu Tsai <[email protected]>
> ---
> 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
>
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 <[email protected]>
> Cc: [email protected]
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
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 <[email protected]>
> > Cc: [email protected]
>
> 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 <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
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 <[email protected]>
> > > Cc: [email protected]
> >
> > 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
Maxime Ripard wrote...
> > > > This patch specifiy statesize for sha1 and md5.
> > > >
> > > > Signed-off-by: LABBE Corentin <[email protected]>
> > > > Cc: [email protected]
> > >
> > > 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 <[email protected]>
| 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
Christoph Biedl <[email protected]> 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 <[email protected]>
> | 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 <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
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 <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
On Mon, Nov 09, 2015 at 11:58:48AM +0800, Herbert Xu wrote:
> Christoph Biedl <linux-kernel.bfrz-o6iJJGtClBpGEOBiBlW9YYQuADTiUCJX@public.gmane.org> 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