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 <[email protected]>
Signed-off-by: Herbert Xu <[email protected]>
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 <asm/unaligned.h>
#include <linux/scatterlist.h>
/* 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, &v, 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, &v, 4);
+ v = readl(ss->base + SS_MD0 + i * 4);
+ put_unaligned_le32(v, areq->result + i * 4);
}
}
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
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=[<reimport,nosimd>85.99%@+3999, 5.85%@+30, <reimport>0.96%@+25, <reimport,nosimd>5.9%@+2263, <flush,nosimd>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=[<reimport>99.1%@+2668, <reimport>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 <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
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=[<reimport,nosimd>85.99%@+3999, 5.85%@+30, <reimport>0.96%@+25, <reimport,nosimd>5.9%@+2263, <flush,nosimd>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=[<reimport>99.1%@+2668, <reimport>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 <[email protected]>
Acked-by: Corentin Labbe <[email protected]>
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=[<reimport,nosimd>85.99%@+3999, 5.85%@+30, <reimport>0.96%@+25, <reimport,nosimd>5.9%@+2263, <flush,nosimd>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=[<reimport>99.1%@+2668, <reimport>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, <reimport>0.61%@+4046, 0.87%@+24, <reimport,nosimd>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 ?
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, <reimport>0.61%@+4046, 0.87%@+24, <reimport,nosimd>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 <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
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, <flush>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.
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, <flush>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