Re: [PATCH] crypto: x86/sha256-mb - fix digest copy in sha256_mb_mgr_get_comp_job_avx2()

[Herbert Xu]

On Fri, Jun 29, 2018 at 02:14:35PM -0700, Eric Biggers wrote:
> From: Eric Biggers 
> 
> There is a copy-paste error where sha256_mb_mgr_get_comp_job_avx2()
> copies the SHA-256 digest state from sha256_mb_mgr::args::digest to
> job_sha256::result_digest.  Consequently, the sha256_mb algorithm
> sometimes calculates the wrong digest.  Fix it.
> 
> Reproducer using AF_ALG:
> 
> #include 
> #include 
> #include 
> #include 
> #include 
> #include 
> 
> static const __u8 expected[32] =
> "\xad\x7f\xac\xb2\x58\x6f\xc6\xe9\x66\xc0\x04\xd7\xd1\xd1\x6b\x02"
> "\x4f\x58\x05\xff\x7c\xb4\x7c\x7a\x85\xda\xbd\x8b\x48\x89\x2c\xa7";
> 
> int main()
> {
> int fd;
> struct sockaddr_alg addr = {
> .salg_type = "hash",
> .salg_name = "sha256_mb",
> };
> __u8 data[4096] = { 0 };
> __u8 digest[32];
> int ret;
> int i;
> 
> fd = socket(AF_ALG, SOCK_SEQPACKET, 0);
> bind(fd, (void *), sizeof(addr));
> fork();
> fd = accept(fd, 0, 0);
> do {
> ret = write(fd, data, 4096);
> assert(ret == 4096);
> ret = read(fd, digest, 32);
> assert(ret == 32);
> } while (memcmp(digest, expected, 32) == 0);
> 
> printf("wrong digest: ");
> for (i = 0; i < 32; i++)
> printf("%02x", digest[i]);
> printf("\n");
> }
> 
> Output was:
> 
> wrong digest: 
> ad7facb2ffef7cb47c7a85dabd8b48892ca7
> 
> Fixes: 172b1d6b5a93 ("crypto: sha256-mb - fix ctx pointer and digest copy")
> Cc:  # v4.8+
> Signed-off-by: Eric Biggers 

Patch applied.  Thanks.
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[PATCH] crypto: x86/sha256-mb - fix digest copy in sha256_mb_mgr_get_comp_job_avx2()

[Eric Biggers]

From: Eric Biggers 

There is a copy-paste error where sha256_mb_mgr_get_comp_job_avx2()
copies the SHA-256 digest state from sha256_mb_mgr::args::digest to
job_sha256::result_digest.  Consequently, the sha256_mb algorithm
sometimes calculates the wrong digest.  Fix it.

Reproducer using AF_ALG:

#include 
#include 
#include 
#include 
#include 
#include 

static const __u8 expected[32] =
"\xad\x7f\xac\xb2\x58\x6f\xc6\xe9\x66\xc0\x04\xd7\xd1\xd1\x6b\x02"
"\x4f\x58\x05\xff\x7c\xb4\x7c\x7a\x85\xda\xbd\x8b\x48\x89\x2c\xa7";

int main()
{
int fd;
struct sockaddr_alg addr = {
.salg_type = "hash",
.salg_name = "sha256_mb",
};
__u8 data[4096] = { 0 };
__u8 digest[32];
int ret;
int i;

fd = socket(AF_ALG, SOCK_SEQPACKET, 0);
bind(fd, (void *), sizeof(addr));
fork();
fd = accept(fd, 0, 0);
do {
ret = write(fd, data, 4096);
assert(ret == 4096);
ret = read(fd, digest, 32);
assert(ret == 32);
} while (memcmp(digest, expected, 32) == 0);

printf("wrong digest: ");
for (i = 0; i < 32; i++)
printf("%02x", digest[i]);
printf("\n");
}

Output was:

wrong digest: 
ad7facb2ffef7cb47c7a85dabd8b48892ca7

Fixes: 172b1d6b5a93 ("crypto: sha256-mb - fix ctx pointer and digest copy")
Cc:  # v4.8+
Signed-off-by: Eric Biggers 
---
 arch/x86/crypto/sha256-mb/sha256_mb_mgr_flush_avx2.S | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/crypto/sha256-mb/sha256_mb_mgr_flush_avx2.S 
b/arch/x86/crypto/sha256-mb/sha256_mb_mgr_flush_avx2.S
index 16c4ccb1f154..d2364c55bbde 100644
--- a/arch/x86/crypto/sha256-mb/sha256_mb_mgr_flush_avx2.S
+++ b/arch/x86/crypto/sha256-mb/sha256_mb_mgr_flush_avx2.S
@@ -265,7 +265,7 @@ ENTRY(sha256_mb_mgr_get_comp_job_avx2)
vpinsrd $1, _args_digest+1*32(state, idx, 4), %xmm0, %xmm0
vpinsrd $2, _args_digest+2*32(state, idx, 4), %xmm0, %xmm0
vpinsrd $3, _args_digest+3*32(state, idx, 4), %xmm0, %xmm0
-   vmovd   _args_digest(state , idx, 4) , %xmm0
+   vmovd   _args_digest+4*32(state, idx, 4), %xmm1
vpinsrd $1, _args_digest+5*32(state, idx, 4), %xmm1, %xmm1
vpinsrd $2, _args_digest+6*32(state, idx, 4), %xmm1, %xmm1
vpinsrd $3, _args_digest+7*32(state, idx, 4), %xmm1, %xmm1
-- 
2.18.0.399.gad0ab374a1-goog