* [PATCH] crypto: salsa20 - fix blkcipher_walk API usage
@ 2017-11-29 4:56 Eric Biggers
2017-11-29 5:26 ` Herbert Xu
0 siblings, 1 reply; 2+ messages in thread
From: Eric Biggers @ 2017-11-29 4:56 UTC (permalink / raw)
To: linux-crypto, Herbert Xu
Cc: David S . Miller, linux-kernel, Eric Biggers, stable
From: Eric Biggers <ebiggers@google.com>
When asked to encrypt or decrypt 0 bytes, both the generic and x86
implementations of Salsa20 crash in blkcipher_walk_done(), either when
doing 'kfree(walk->buffer)' or 'free_page((unsigned long)walk->page)',
because walk->buffer and walk->page have not been initialized.
The bug is that Salsa20 is calling blkcipher_walk_done() even when
nothing is in 'walk.nbytes'. But blkcipher_walk_done() is only meant to
be called when a nonzero number of bytes have been provided.
The broken code is part of an optimization that tries to make only one
call to salsa20_encrypt_bytes() to process inputs that are not evenly
divisible by 64 bytes. To fix the bug, just remove this "optimization"
and use the blkcipher_walk API the same way all the other users do.
Reproducer:
#include <linux/if_alg.h>
#include <sys/socket.h>
#include <unistd.h>
int main()
{
int algfd, reqfd;
struct sockaddr_alg addr = {
.salg_type = "skcipher",
.salg_name = "salsa20",
};
char key[16] = { 0 };
algfd = socket(AF_ALG, SOCK_SEQPACKET, 0);
bind(algfd, (void *)&addr, sizeof(addr));
reqfd = accept(algfd, 0, 0);
setsockopt(algfd, SOL_ALG, ALG_SET_KEY, key, sizeof(key));
read(reqfd, key, sizeof(key));
}
Reported-by: syzbot <syzkaller@googlegroups.com>
Fixes: eb6f13eb9f81 ("[CRYPTO] salsa20_generic: Fix multi-page processing")
Cc: <stable@vger.kernel.org> # v2.6.25+
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
arch/x86/crypto/salsa20_glue.c | 7 -------
crypto/salsa20_generic.c | 7 -------
2 files changed, 14 deletions(-)
diff --git a/arch/x86/crypto/salsa20_glue.c b/arch/x86/crypto/salsa20_glue.c
index 399a29d067d6..cb91a64a99e7 100644
--- a/arch/x86/crypto/salsa20_glue.c
+++ b/arch/x86/crypto/salsa20_glue.c
@@ -59,13 +59,6 @@ static int encrypt(struct blkcipher_desc *desc,
salsa20_ivsetup(ctx, walk.iv);
- if (likely(walk.nbytes == nbytes))
- {
- salsa20_encrypt_bytes(ctx, walk.src.virt.addr,
- walk.dst.virt.addr, nbytes);
- return blkcipher_walk_done(desc, &walk, 0);
- }
-
while (walk.nbytes >= 64) {
salsa20_encrypt_bytes(ctx, walk.src.virt.addr,
walk.dst.virt.addr,
diff --git a/crypto/salsa20_generic.c b/crypto/salsa20_generic.c
index f550b5d94630..d7da0eea5622 100644
--- a/crypto/salsa20_generic.c
+++ b/crypto/salsa20_generic.c
@@ -188,13 +188,6 @@ static int encrypt(struct blkcipher_desc *desc,
salsa20_ivsetup(ctx, walk.iv);
- if (likely(walk.nbytes == nbytes))
- {
- salsa20_encrypt_bytes(ctx, walk.dst.virt.addr,
- walk.src.virt.addr, nbytes);
- return blkcipher_walk_done(desc, &walk, 0);
- }
-
while (walk.nbytes >= 64) {
salsa20_encrypt_bytes(ctx, walk.dst.virt.addr,
walk.src.virt.addr,
--
2.15.0
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] crypto: salsa20 - fix blkcipher_walk API usage
2017-11-29 4:56 [PATCH] crypto: salsa20 - fix blkcipher_walk API usage Eric Biggers
@ 2017-11-29 5:26 ` Herbert Xu
0 siblings, 0 replies; 2+ messages in thread
From: Herbert Xu @ 2017-11-29 5:26 UTC (permalink / raw)
To: Eric Biggers
Cc: linux-crypto, David S . Miller, linux-kernel, Eric Biggers, stable
On Tue, Nov 28, 2017 at 08:56:59PM -0800, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
>
> When asked to encrypt or decrypt 0 bytes, both the generic and x86
> implementations of Salsa20 crash in blkcipher_walk_done(), either when
> doing 'kfree(walk->buffer)' or 'free_page((unsigned long)walk->page)',
> because walk->buffer and walk->page have not been initialized.
>
> The bug is that Salsa20 is calling blkcipher_walk_done() even when
> nothing is in 'walk.nbytes'. But blkcipher_walk_done() is only meant to
> be called when a nonzero number of bytes have been provided.
>
> The broken code is part of an optimization that tries to make only one
> call to salsa20_encrypt_bytes() to process inputs that are not evenly
> divisible by 64 bytes. To fix the bug, just remove this "optimization"
> and use the blkcipher_walk API the same way all the other users do.
>
> Reproducer:
>
> #include <linux/if_alg.h>
> #include <sys/socket.h>
> #include <unistd.h>
>
> int main()
> {
> int algfd, reqfd;
> struct sockaddr_alg addr = {
> .salg_type = "skcipher",
> .salg_name = "salsa20",
> };
> char key[16] = { 0 };
>
> algfd = socket(AF_ALG, SOCK_SEQPACKET, 0);
> bind(algfd, (void *)&addr, sizeof(addr));
> reqfd = accept(algfd, 0, 0);
> setsockopt(algfd, SOL_ALG, ALG_SET_KEY, key, sizeof(key));
> read(reqfd, key, sizeof(key));
> }
>
> Reported-by: syzbot <syzkaller@googlegroups.com>
> Fixes: eb6f13eb9f81 ("[CRYPTO] salsa20_generic: Fix multi-page processing")
> Cc: <stable@vger.kernel.org> # v2.6.25+
> Signed-off-by: Eric Biggers <ebiggers@google.com>
Patch applied. Thanks.
--
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
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2017-11-29 5:27 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-29 4:56 [PATCH] crypto: salsa20 - fix blkcipher_walk API usage Eric Biggers
2017-11-29 5:26 ` Herbert Xu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).