| Message ID | 1446709737-11316-1-git-send-email-clabbe.montjoie@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 >
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
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,
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 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
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,
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
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",
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(+)