Re: [v2 PATCH] crypto: sun4i-ss - Fix sparse endianness markers
On Thu, Oct 08, 2020 at 08:36:23AM +0200, Corentin Labbe wrote: > On Thu, Oct 08, 2020 at 04:52:38PM +1100, Herbert Xu wrote: > > On Thu, Sep 24, 2020 at 03:27:38PM +0200, Corentin Labbe wrote: > > > > > > This is an example on next-20200923+BigEndian > > > alg: ahash: sha1 test failed (wrong result) on test vector \"random: > > > psize=194 ksize=0\", cfg=\"random: inplace may_sleep use_finup > > > src_divs=[98.25%@+1124, 1.75%@+5] iv_offset=18\" This failure is in one of the randomly generated test cases. If it doesn't reproduce reliably, you can set cryptomgr.fuzz_iterations=1000 on the kernel command line (increased from the default 100). It is confusing that it says just "sha1". This seems to be a quirk specific to how tcrypt calls alg_test(). It's probably really testing "sha1-sun4i-ss". I guess that testmgr.c should be using the actual cra_driver_name in the log messages, not the 'driver' string that was passed into alg_test(). - Eric
Re: [v2 PATCH] crypto: sun4i-ss - Fix sparse endianness markers
On Thu, Oct 08, 2020 at 04:52:38PM +1100, Herbert Xu wrote: > On Thu, Sep 24, 2020 at 03:27:38PM +0200, Corentin Labbe wrote: > > > > This is an example on next-20200923+BigEndian > > alg: ahash: sha1 test failed (wrong result) on test vector \"random: > > psize=194 ksize=0\", cfg=\"random: inplace may_sleep use_finup > > src_divs=[98.25%@+1124, 1.75%@+5] iv_offset=18\" > > Hmm, the only way I can see this happening is if it was triggered > by tcrypt. Were you using tcrypt by any chance? > > Ccing Eric in case he has any insight. > > > === DUMP /proc/crypto === > > name : sha1 > > driver : sha1-sun4i-ss > > module : kernel > > priority : 300 > > refcnt : 1 > > selftest : passed > > internal : no > > type : ahash > > async: no > > blocksize: 64 > > digestsize : 20 > > ... > > > name : sha1 > > driver : sha1-generic > > module : kernel > > priority : 100 > > refcnt : 1 > > selftest : passed > > internal : no > > type : shash > > blocksize: 64 > > digestsize : 20 > > Thanks, Yes I use tcrypt to force all algos to be tested.
Re: [v2 PATCH] crypto: sun4i-ss - Fix sparse endianness markers
On Thu, Sep 24, 2020 at 03:27:38PM +0200, Corentin Labbe wrote: > > This is an example on next-20200923+BigEndian > alg: ahash: sha1 test failed (wrong result) on test vector \"random: > psize=194 ksize=0\", cfg=\"random: inplace may_sleep use_finup > src_divs=[98.25%@+1124, 1.75%@+5] iv_offset=18\" Hmm, the only way I can see this happening is if it was triggered by tcrypt. Were you using tcrypt by any chance? Ccing Eric in case he has any insight. > === DUMP /proc/crypto === > name : sha1 > driver : sha1-sun4i-ss > module : kernel > priority : 300 > refcnt : 1 > selftest : passed > internal : no > type : ahash > async: no > blocksize: 64 > digestsize : 20 ... > name : sha1 > driver : sha1-generic > module : kernel > priority : 100 > refcnt : 1 > selftest : passed > internal : no > type : shash > blocksize: 64 > digestsize : 20 Thanks, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [v2 PATCH] crypto: sun4i-ss - Fix sparse endianness markers
On Thu, Sep 24, 2020 at 01:08:59PM +1000, Herbert Xu wrote: > On Mon, Sep 14, 2020 at 12:40:58PM +0200, Corentin Labbe wrote: > > > > I got this on next-20200910/multi_v7_defconfig BigEndian > > [ 12.137856] alg: hash: skipping comparison tests for md5-sun4i-ss > > because md5-generic is unavailable > > md5-sun4i-ss md5 reqs=763 > > [ 98.286632] alg: ahash: md5 test failed (wrong result) on test vector > > \"random: psize=65 ksize=0\", cfg=\"random: use_finup > > src_divs=[95.28%@+1052, 0.61%@+4046, 0.87%@+24, > > 3.24%@+542] key_offset=54\" > > > > So sun4i-ss is not involved. > > Strangely /proc/crypto show: > > name : md5 > > > > driver : md5-generic > > > > module : md5 > > > > priority : 0 > > > > refcnt : 1 > > > > selftest : passed > > > > internal : no > > > > type : shash > > > > blocksize: 64 > > > > digestsize : 16 > > > > and I didnt see anything failed/unknow in /proc/crypto > > > > Why the failed algorithm is not visible ? > > Please include the complete /proc/crypto after you get the error. > Hello This is an example on next-20200923+BigEndian alg: ahash: sha1 test failed (wrong result) on test vector \"random: psize=194 ksize=0\", cfg=\"random: inplace may_sleep use_finup src_divs=[98.25%@+1124, 1.75%@+5] iv_offset=18\" === DUMP /proc/crypto === name : ctr(sm4) driver : ctr(sm4-generic) module : ctr priority : 100 refcnt : 1 selftest : passed internal : no type : skcipher async: no blocksize: 1 min keysize : 16 max keysize : 16 ivsize : 16 chunksize: 16 walksize : 16 name : cbc(sm4) driver : cbc(sm4-generic) module : kernel priority : 100 refcnt : 1 selftest : passed internal : no type : skcipher async: no blocksize: 16 min keysize : 16 max keysize : 16 ivsize : 16 chunksize: 16 walksize : 16 name : ecb(sm4) driver : ecb(sm4-generic) module : kernel priority : 100 refcnt : 1 selftest : passed internal : no type : skcipher async: no blocksize: 16 min keysize : 16 max keysize : 16 ivsize : 0 chunksize: 16 walksize : 16 name : sm4 driver : sm4-generic module : sm4_generic priority : 100 refcnt : 1 selftest : passed internal : no type : cipher blocksize: 16 min keysize : 16 max keysize : 16 name : authenc(hmac(sha512),cbc(des3_ede)) driver : authenc(hmac(sha512-generic),cbc(des3_ede-generic)) module : authenc priority : 1100 refcnt : 1 selftest : passed internal : no type : aead async: no blocksize: 8 ivsize : 8 maxauthsize : 64 geniv: name : authenc(hmac(sha512),cbc(des3_ede)) driver : authenc(hmac(sha512-generic),cbc-des3-sun4i-ss) module : authenc priority : 3100 refcnt : 1 selftest : passed internal : no type : aead async: no blocksize: 8 ivsize : 8 maxauthsize : 64 geniv: name : authenc(hmac(sha512),cbc(des)) driver : authenc(hmac(sha512-generic),cbc(des-generic)) module : authenc priority : 1100 refcnt : 1 selftest : passed internal : no type : aead async: no blocksize: 8 ivsize : 8 maxauthsize : 64 geniv: name : authenc(hmac(sha512),cbc(des)) driver : authenc(hmac(sha512-generic),cbc-des-sun4i-ss) module : authenc priority : 3100 refcnt : 1 selftest : passed internal : no type : aead async: no blocksize: 8 ivsize : 8 maxauthsize : 64 geniv: name : authenc(hmac(sha384),cbc(des3_ede)) driver : authenc(hmac(sha384-generic),cbc(des3_ede-generic)) module : authenc priority : 1100 refcnt : 1 selftest : passed internal : no type : aead async: no blocksize: 8 ivsize : 8 maxauthsize : 48 geniv: name : authenc(hmac(sha384),cbc(des3_ede)) driver : authenc(hmac(sha384-generic),cbc-des3-sun4i-ss) module : authenc priority : 3100 refcnt : 1
Re: [v2 PATCH] crypto: sun4i-ss - Fix sparse endianness markers
On Mon, Sep 14, 2020 at 12:40:58PM +0200, Corentin Labbe wrote: > > I got this on next-20200910/multi_v7_defconfig BigEndian > [ 12.137856] alg: hash: skipping comparison tests for md5-sun4i-ss because > md5-generic is unavailable > md5-sun4i-ss md5 reqs=763 > [ 98.286632] alg: ahash: md5 test failed (wrong result) on test vector > \"random: psize=65 ksize=0\", cfg=\"random: use_finup src_divs=[95.28%@+1052, > 0.61%@+4046, 0.87%@+24, 3.24%@+542] key_offset=54\" > > So sun4i-ss is not involved. > Strangely /proc/crypto show: > name : md5 > > driver : md5-generic > > module : md5 > > priority : 0 > > refcnt : 1 > > selftest : passed > > internal : no > > type : shash > > blocksize: 64 > > digestsize : 16 > > and I didnt see anything failed/unknow in /proc/crypto > > Why the failed algorithm is not visible ? Please include the complete /proc/crypto after you get the error. Thanks, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [v2 PATCH] crypto: sun4i-ss - Fix sparse endianness markers
On Fri, Sep 11, 2020 at 02:13:55PM +1000, Herbert Xu wrote: > On Thu, Sep 10, 2020 at 02:22:48PM +0200, Corentin Labbe wrote: > > > > I get some md5 error on both A20+BE: > > alg: ahash: md5 test failed (wrong result) on test vector \"random: > > psize=129 ksize=0\", cfg=\"random: inplace use_finup nosimd > > src_divs=[85.99%@+3999, 5.85%@+30, 0.96%@+25, > > 5.9%@+2263, 2.11%@+1950] iv_offset=2 > > key_offset=43\" > > and A33+BE: > > [ 84.469045] alg: ahash: md5 test failed (wrong result) on test vector > > \"random: psize=322 ksize=0\", cfg=\"random: inplace may_sleep use_finup > > src_divs=[99.1%@+2668, 0.88%@alignmask+3630, > > 0.11%@+3403] iv_offset=33\" > > +[ 84.469074] need:35966fc8 b31ea266 2bf064e9 f20f40ad > > +[ 84.469084] have:e29e4491 f3b6effc fa366691 00e04bd9 > > > > Thoses errors are random. (1 boot out of 2) > > Do these really go away without this patch applied? AFAICS the > generated code should be identical. > I got this on next-20200910/multi_v7_defconfig BigEndian [ 12.137856] alg: hash: skipping comparison tests for md5-sun4i-ss because md5-generic is unavailable md5-sun4i-ss md5 reqs=763 [ 98.286632] alg: ahash: md5 test failed (wrong result) on test vector \"random: psize=65 ksize=0\", cfg=\"random: use_finup src_divs=[95.28%@+1052, 0.61%@+4046, 0.87%@+24, 3.24%@+542] key_offset=54\" So sun4i-ss is not involved. Strangely /proc/crypto show: name : md5 driver : md5-generic module : md5 priority : 0 refcnt : 1 selftest : passed internal : no type : shash blocksize: 64 digestsize : 16 and I didnt see anything failed/unknow in /proc/crypto Why the failed algorithm is not visible ?
Re: [v2 PATCH] crypto: sun4i-ss - Fix sparse endianness markers
On Fri, Sep 11, 2020 at 02:13:55PM +1000, Herbert Xu wrote: > On Thu, Sep 10, 2020 at 02:22:48PM +0200, Corentin Labbe wrote: > > > > I get some md5 error on both A20+BE: > > alg: ahash: md5 test failed (wrong result) on test vector \"random: > > psize=129 ksize=0\", cfg=\"random: inplace use_finup nosimd > > src_divs=[85.99%@+3999, 5.85%@+30, 0.96%@+25, > > 5.9%@+2263, 2.11%@+1950] iv_offset=2 > > key_offset=43\" > > and A33+BE: > > [ 84.469045] alg: ahash: md5 test failed (wrong result) on test vector > > \"random: psize=322 ksize=0\", cfg=\"random: inplace may_sleep use_finup > > src_divs=[99.1%@+2668, 0.88%@alignmask+3630, > > 0.11%@+3403] iv_offset=33\" > > +[ 84.469074] need:35966fc8 b31ea266 2bf064e9 f20f40ad > > +[ 84.469084] have:e29e4491 f3b6effc fa366691 00e04bd9 > > > > Thoses errors are random. (1 boot out of 2) > > Do these really go away without this patch applied? AFAICS the > generated code should be identical. > It happens without your patch, so your patch is unrelated to this issue. You can add: Tested-by: Corentin Labbe Acked-by: Corentin Labbe
Re: [v2 PATCH] crypto: sun4i-ss - Fix sparse endianness markers
On Thu, Sep 10, 2020 at 02:22:48PM +0200, Corentin Labbe wrote: > > I get some md5 error on both A20+BE: > alg: ahash: md5 test failed (wrong result) on test vector \"random: psize=129 > ksize=0\", cfg=\"random: inplace use_finup nosimd > src_divs=[85.99%@+3999, 5.85%@+30, 0.96%@+25, > 5.9%@+2263, 2.11%@+1950] iv_offset=2 > key_offset=43\" > and A33+BE: > [ 84.469045] alg: ahash: md5 test failed (wrong result) on test vector > \"random: psize=322 ksize=0\", cfg=\"random: inplace may_sleep use_finup > src_divs=[99.1%@+2668, 0.88%@alignmask+3630, 0.11%@+3403] > iv_offset=33\" > +[ 84.469074] need:35966fc8 b31ea266 2bf064e9 f20f40ad > +[ 84.469084] have:e29e4491 f3b6effc fa366691 00e04bd9 > > Thoses errors are random. (1 boot out of 2) Do these really go away without this patch applied? AFAICS the generated code should be identical. Thanks, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [v2 PATCH] crypto: sun4i-ss - Fix sparse endianness markers
On Tue, Sep 08, 2020 at 03:00:36PM +1000, Herbert Xu wrote:
> On Mon, Sep 07, 2020 at 06:00:29PM +0200, Corentin Labbe wrote:
> >
> > The put_unaligned should be _le32.
> >
> > This fix the modprobe tcrypt fail.
>
> Thanks. Yes the original code was correct.
>
> ---8<---
> This patch also fixes the incorrect endianness markings in the
> sun4i-ss driver. It should have no effect in the genereated code.
>
> Instead of using cpu_to_Xe32 followed by a memcpy, this patch
> converts the final hash write to use put_unaligned_X instead.
>
> Reported-by: kernel test robot
> Signed-off-by: Herbert Xu
>
> diff --git a/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-hash.c
> b/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-hash.c
> index dc35edd90034..1dff48558f53 100644
> --- a/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-hash.c
> +++ b/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-hash.c
> @@ -9,6 +9,7 @@
> * You could find the datasheet in Documentation/arm/sunxi.rst
> */
> #include "sun4i-ss.h"
> +#include
> #include
>
> /* This is a totally arbitrary value */
> @@ -196,7 +197,7 @@ static int sun4i_hash(struct ahash_request *areq)
> struct sg_mapping_iter mi;
> int in_r, err = 0;
> size_t copied = 0;
> - __le32 wb = 0;
> + u32 wb = 0;
>
> dev_dbg(ss->dev, "%s %s bc=%llu len=%u mode=%x wl=%u h0=%0x",
> __func__, crypto_tfm_alg_name(areq->base.tfm),
> @@ -408,7 +409,7 @@ static int sun4i_hash(struct ahash_request *areq)
>
> nbw = op->len - 4 * nwait;
> if (nbw) {
> - wb = cpu_to_le32(*(u32 *)(op->buf + nwait * 4));
> + wb = le32_to_cpup((__le32 *)(op->buf + nwait * 4));
> wb &= GENMASK((nbw * 8) - 1, 0);
>
> op->byte_count += nbw;
> @@ -417,7 +418,7 @@ static int sun4i_hash(struct ahash_request *areq)
>
> /* write the remaining bytes of the nbw buffer */
> wb |= ((1 << 7) << (nbw * 8));
> - bf[j++] = le32_to_cpu(wb);
> + ((__le32 *)bf)[j++] = cpu_to_le32(wb);
>
> /*
>* number of space to pad to obtain 64o minus 8(size) minus 4 (final 1)
> @@ -479,16 +480,16 @@ static int sun4i_hash(struct ahash_request *areq)
> /* Get the hash from the device */
> if (op->mode == SS_OP_SHA1) {
> for (i = 0; i < 5; i++) {
> + v = readl(ss->base + SS_MD0 + i * 4);
> if (ss->variant->sha1_in_be)
> - v = cpu_to_le32(readl(ss->base + SS_MD0 + i *
> 4));
> + put_unaligned_le32(v, areq->result + i * 4);
> else
> - v = cpu_to_be32(readl(ss->base + SS_MD0 + i *
> 4));
> - memcpy(areq->result + i * 4, , 4);
> + put_unaligned_be32(v, areq->result + i * 4);
> }
> } else {
> for (i = 0; i < 4; i++) {
> - v = cpu_to_le32(readl(ss->base + SS_MD0 + i * 4));
> - memcpy(areq->result + i * 4, , 4);
> + v = readl(ss->base + SS_MD0 + i * 4);
> + put_unaligned_le32(v, areq->result + i * 4);
> }
> }
>
I get some md5 error on both A20+BE:
alg: ahash: md5 test failed (wrong result) on test vector \"random: psize=129
ksize=0\", cfg=\"random: inplace use_finup nosimd
src_divs=[85.99%@+3999, 5.85%@+30, 0.96%@+25,
5.9%@+2263, 2.11%@+1950] iv_offset=2
key_offset=43\"
and A33+BE:
[ 84.469045] alg: ahash: md5 test failed (wrong result) on test vector
\"random: psize=322 ksize=0\", cfg=\"random: inplace may_sleep use_finup
src_divs=[99.1%@+2668, 0.88%@alignmask+3630, 0.11%@+3403]
iv_offset=33\"
+[ 84.469074] need:35966fc8 b31ea266 2bf064e9 f20f40ad
+[ 84.469084] have:e29e4491 f3b6effc fa366691 00e04bd9
Thoses errors are random. (1 boot out of 2)
The ahash-md5-sun4i-ss is set as "selftest: passed" and I didnt see any
failling/absent test in /proc/crypto
So what is this md5 which fail ?
I am still investigating and will try on more platform.
[v2 PATCH] crypto: sun4i-ss - Fix sparse endianness markers
On Mon, Sep 07, 2020 at 06:00:29PM +0200, Corentin Labbe wrote:
>
> The put_unaligned should be _le32.
>
> This fix the modprobe tcrypt fail.
Thanks. Yes the original code was correct.
---8<---
This patch also fixes the incorrect endianness markings in the
sun4i-ss driver. It should have no effect in the genereated code.
Instead of using cpu_to_Xe32 followed by a memcpy, this patch
converts the final hash write to use put_unaligned_X instead.
Reported-by: kernel test robot
Signed-off-by: Herbert Xu
diff --git a/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-hash.c
b/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-hash.c
index dc35edd90034..1dff48558f53 100644
--- a/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-hash.c
+++ b/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-hash.c
@@ -9,6 +9,7 @@
* You could find the datasheet in Documentation/arm/sunxi.rst
*/
#include "sun4i-ss.h"
+#include
#include
/* This is a totally arbitrary value */
@@ -196,7 +197,7 @@ static int sun4i_hash(struct ahash_request *areq)
struct sg_mapping_iter mi;
int in_r, err = 0;
size_t copied = 0;
- __le32 wb = 0;
+ u32 wb = 0;
dev_dbg(ss->dev, "%s %s bc=%llu len=%u mode=%x wl=%u h0=%0x",
__func__, crypto_tfm_alg_name(areq->base.tfm),
@@ -408,7 +409,7 @@ static int sun4i_hash(struct ahash_request *areq)
nbw = op->len - 4 * nwait;
if (nbw) {
- wb = cpu_to_le32(*(u32 *)(op->buf + nwait * 4));
+ wb = le32_to_cpup((__le32 *)(op->buf + nwait * 4));
wb &= GENMASK((nbw * 8) - 1, 0);
op->byte_count += nbw;
@@ -417,7 +418,7 @@ static int sun4i_hash(struct ahash_request *areq)
/* write the remaining bytes of the nbw buffer */
wb |= ((1 << 7) << (nbw * 8));
- bf[j++] = le32_to_cpu(wb);
+ ((__le32 *)bf)[j++] = cpu_to_le32(wb);
/*
* number of space to pad to obtain 64o minus 8(size) minus 4 (final 1)
@@ -479,16 +480,16 @@ static int sun4i_hash(struct ahash_request *areq)
/* Get the hash from the device */
if (op->mode == SS_OP_SHA1) {
for (i = 0; i < 5; i++) {
+ v = readl(ss->base + SS_MD0 + i * 4);
if (ss->variant->sha1_in_be)
- v = cpu_to_le32(readl(ss->base + SS_MD0 + i *
4));
+ put_unaligned_le32(v, areq->result + i * 4);
else
- v = cpu_to_be32(readl(ss->base + SS_MD0 + i *
4));
- memcpy(areq->result + i * 4, , 4);
+ put_unaligned_be32(v, areq->result + i * 4);
}
} else {
for (i = 0; i < 4; i++) {
- v = cpu_to_le32(readl(ss->base + SS_MD0 + i * 4));
- memcpy(areq->result + i * 4, , 4);
+ v = readl(ss->base + SS_MD0 + i * 4);
+ put_unaligned_le32(v, areq->result + i * 4);
}
}
--
Email: Herbert Xu
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
