crypto: salsa20 - fix blkcipher_walk API usage
Subsystems (3)
PATCH 1/1 crypto: salsa20 - fix blkcipher_walk API usage
From: Eric Biggers <[email protected]>
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));
}--- arch/x86/crypto/salsa20_glue.c | 7 ------- crypto/salsa20_generic.c | 7 ------- 2 files changed, 14 deletions(-)
arch/x86/crypto/salsa20_glue.c +0 -7
@@ -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,
crypto/salsa20_generic.c +0 -7
@@ -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,
Signature · 1 line
2.15.0
Herbert Xu
maintainerQuoted 38 lines · Eric Biggers
From: Eric Biggers <[email protected]>
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)', becausewalk->bufferandwalk->pagehave not been initialized.The bug is that Salsa20 is calling
blkcipher_walk_done()even when nothing is in 'walk.nbytes'. Butblkcipher_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 theblkcipher_walkAPI 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)); }
Quoted 4 lines
Reported-by syzbot <[email protected]>Fixes eb6f13eb9f81 ("[CRYPTO] salsa20_generic: Fix multi-page processing")Cc <[email protected]> # v2.6.25+Signed-off-by Eric Biggers <[email protected]>
Patch applied. Thanks.
Signature · 3 lines
Email: Herbert Xu <[email protected]> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt