[SRU,Trusty,Artful,1/1] crypto: salsa20 - fix blkcipher_walk API usage

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>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

CVE-2017-17805
(cherry picked from commit ecaaab5649781c5a0effdaf298a925063020500e)
Signed-off-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com>
---
 arch/x86/crypto/salsa20_glue.c | 7 -------
 crypto/salsa20_generic.c       | 7 -------
 2 files changed, 14 deletions(-)

On 01/02/18 12:14, Kleber Sacilotto de Souza 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>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
> 
> CVE-2017-17805
> (cherry picked from commit ecaaab5649781c5a0effdaf298a925063020500e)
> Signed-off-by: Kleber Sacilotto de Souza <kleber.souza@canonical.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,
> 

Acked-by: Colin Ian King <colin.king@canonical.com>