| Message ID | 20180713081231.ofllv36sg3bftbvs@gondor.apana.org.au (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Herbert Xu |
| Headers | show |
Herbert Xu wrote: > This was detected by the self-test thanks to Ard's chunking patch. > > I finally got around to testing this out on my ancient Via box. It > turns out that the workaround got the assembly wrong and we end up > doing count + initial cycles of the loop instead of just count. > > This obviously causes corruption, either by overwriting the source > that is yet to be processed, or writing over the end of the buffer. > > On CPUs that don't require the workaround only ECB is affected. > On Nano CPUs both ECB and CBC are affected. > > This patch fixes it by doing the subtraction prior to the assembly. > > Fixes: a76c1c23d0c3 ("crypto: padlock-aes - work around Nano CPU...") > Cc: <stable@vger.kernel.org> > Reported-by: Jamie Heilman <jamie@audible.transient.net> > Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au> Yay! Tested against 4.14.55 and the self-test passes now on my Esther system. > diff --git a/drivers/crypto/padlock-aes.c b/drivers/crypto/padlock-aes.c > index 1c6cbda..09d823d 100644 > --- a/drivers/crypto/padlock-aes.c > +++ b/drivers/crypto/padlock-aes.c > @@ -266,6 +266,8 @@ static inline void padlock_xcrypt_ecb(const u8 *input, u8 *output, void *key, > return; > } > > + count -= initial; > + > if (initial) > asm volatile (".byte 0xf3,0x0f,0xa7,0xc8" /* rep xcryptecb */ > : "+S"(input), "+D"(output) > @@ -273,7 +275,7 @@ static inline void padlock_xcrypt_ecb(const u8 *input, u8 *output, void *key, > > asm volatile (".byte 0xf3,0x0f,0xa7,0xc8" /* rep xcryptecb */ > : "+S"(input), "+D"(output) > - : "d"(control_word), "b"(key), "c"(count - initial)); > + : "d"(control_word), "b"(key), "c"(count)); > } > > static inline u8 *padlock_xcrypt_cbc(const u8 *input, u8 *output, void *key, > @@ -284,6 +286,8 @@ static inline u8 *padlock_xcrypt_cbc(const u8 *input, u8 *output, void *key, > if (count < cbc_fetch_blocks) > return cbc_crypt(input, output, key, iv, control_word, count); > > + count -= initial; > + > if (initial) > asm volatile (".byte 0xf3,0x0f,0xa7,0xd0" /* rep xcryptcbc */ > : "+S" (input), "+D" (output), "+a" (iv) > @@ -291,7 +295,7 @@ static inline u8 *padlock_xcrypt_cbc(const u8 *input, u8 *output, void *key, > > asm volatile (".byte 0xf3,0x0f,0xa7,0xd0" /* rep xcryptcbc */ > : "+S" (input), "+D" (output), "+a" (iv) > - : "d" (control_word), "b" (key), "c" (count-initial)); > + : "d" (control_word), "b" (key), "c" (count)); > return iv; > } > > -- > Email: Herbert Xu <herbert@gondor.apana.org.au> > Home Page: http://gondor.apana.org.au/~herbert/ > PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
diff --git a/drivers/crypto/padlock-aes.c b/drivers/crypto/padlock-aes.c index 1c6cbda..09d823d 100644 --- a/drivers/crypto/padlock-aes.c +++ b/drivers/crypto/padlock-aes.c @@ -266,6 +266,8 @@ static inline void padlock_xcrypt_ecb(const u8 *input, u8 *output, void *key, return; } + count -= initial; + if (initial) asm volatile (".byte 0xf3,0x0f,0xa7,0xc8" /* rep xcryptecb */ : "+S"(input), "+D"(output) @@ -273,7 +275,7 @@ static inline void padlock_xcrypt_ecb(const u8 *input, u8 *output, void *key, asm volatile (".byte 0xf3,0x0f,0xa7,0xc8" /* rep xcryptecb */ : "+S"(input), "+D"(output) - : "d"(control_word), "b"(key), "c"(count - initial)); + : "d"(control_word), "b"(key), "c"(count)); } static inline u8 *padlock_xcrypt_cbc(const u8 *input, u8 *output, void *key, @@ -284,6 +286,8 @@ static inline u8 *padlock_xcrypt_cbc(const u8 *input, u8 *output, void *key, if (count < cbc_fetch_blocks) return cbc_crypt(input, output, key, iv, control_word, count); + count -= initial; + if (initial) asm volatile (".byte 0xf3,0x0f,0xa7,0xd0" /* rep xcryptcbc */ : "+S" (input), "+D" (output), "+a" (iv) @@ -291,7 +295,7 @@ static inline u8 *padlock_xcrypt_cbc(const u8 *input, u8 *output, void *key, asm volatile (".byte 0xf3,0x0f,0xa7,0xd0" /* rep xcryptcbc */ : "+S" (input), "+D" (output), "+a" (iv) - : "d" (control_word), "b" (key), "c" (count-initial)); + : "d" (control_word), "b" (key), "c" (count)); return iv; }
This was detected by the self-test thanks to Ard's chunking patch. I finally got around to testing this out on my ancient Via box. It turns out that the workaround got the assembly wrong and we end up doing count + initial cycles of the loop instead of just count. This obviously causes corruption, either by overwriting the source that is yet to be processed, or writing over the end of the buffer. On CPUs that don't require the workaround only ECB is affected. On Nano CPUs both ECB and CBC are affected. This patch fixes it by doing the subtraction prior to the assembly. Fixes: a76c1c23d0c3 ("crypto: padlock-aes - work around Nano CPU...") Cc: <stable@vger.kernel.org> Reported-by: Jamie Heilman <jamie@audible.transient.net> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>