| From 0868def3e4100591e7a1fdbf3eed1439cc8f7ca3 Mon Sep 17 00:00:00 2001 |
| From: Eric Biggers <ebiggers@google.com> |
| Date: Mon, 23 Jul 2018 10:54:57 -0700 |
| Subject: crypto: blkcipher - fix crash flushing dcache in error path |
| |
| From: Eric Biggers <ebiggers@google.com> |
| |
| commit 0868def3e4100591e7a1fdbf3eed1439cc8f7ca3 upstream. |
| |
| Like the skcipher_walk case: |
| |
| scatterwalk_done() is only meant to be called after a nonzero number of |
| bytes have been processed, since scatterwalk_pagedone() will flush the |
| dcache of the *previous* page. But in the error case of |
| blkcipher_walk_done(), e.g. if the input wasn't an integer number of |
| blocks, scatterwalk_done() was actually called after advancing 0 bytes. |
| This caused a crash ("BUG: unable to handle kernel paging request") |
| during '!PageSlab(page)' on architectures like arm and arm64 that define |
| ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE, provided that the input was |
| page-aligned as in that case walk->offset == 0. |
| |
| Fix it by reorganizing blkcipher_walk_done() to skip the |
| scatterwalk_advance() and scatterwalk_done() if an error has occurred. |
| |
| This bug was found by syzkaller fuzzing. |
| |
| Reproducer, assuming ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE: |
| |
| #include <linux/if_alg.h> |
| #include <sys/socket.h> |
| #include <unistd.h> |
| |
| int main() |
| { |
| struct sockaddr_alg addr = { |
| .salg_type = "skcipher", |
| .salg_name = "ecb(aes-generic)", |
| }; |
| char buffer[4096] __attribute__((aligned(4096))) = { 0 }; |
| int fd; |
| |
| fd = socket(AF_ALG, SOCK_SEQPACKET, 0); |
| bind(fd, (void *)&addr, sizeof(addr)); |
| setsockopt(fd, SOL_ALG, ALG_SET_KEY, buffer, 16); |
| fd = accept(fd, NULL, NULL); |
| write(fd, buffer, 15); |
| read(fd, buffer, 15); |
| } |
| |
| Reported-by: Liu Chao <liuchao741@huawei.com> |
| Fixes: 5cde0af2a982 ("[CRYPTO] cipher: Added block cipher type") |
| Cc: <stable@vger.kernel.org> # v2.6.19+ |
| Signed-off-by: Eric Biggers <ebiggers@google.com> |
| Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| --- |
| crypto/blkcipher.c | 54 +++++++++++++++++++++++++---------------------------- |
| 1 file changed, 26 insertions(+), 28 deletions(-) |
| |
| --- a/crypto/blkcipher.c |
| +++ b/crypto/blkcipher.c |
| @@ -70,19 +70,18 @@ static inline u8 *blkcipher_get_spot(u8 |
| return max(start, end_page); |
| } |
| |
| -static inline unsigned int blkcipher_done_slow(struct blkcipher_walk *walk, |
| - unsigned int bsize) |
| +static inline void blkcipher_done_slow(struct blkcipher_walk *walk, |
| + unsigned int bsize) |
| { |
| u8 *addr; |
| |
| addr = (u8 *)ALIGN((unsigned long)walk->buffer, walk->alignmask + 1); |
| addr = blkcipher_get_spot(addr, bsize); |
| scatterwalk_copychunks(addr, &walk->out, bsize, 1); |
| - return bsize; |
| } |
| |
| -static inline unsigned int blkcipher_done_fast(struct blkcipher_walk *walk, |
| - unsigned int n) |
| +static inline void blkcipher_done_fast(struct blkcipher_walk *walk, |
| + unsigned int n) |
| { |
| if (walk->flags & BLKCIPHER_WALK_COPY) { |
| blkcipher_map_dst(walk); |
| @@ -96,49 +95,48 @@ static inline unsigned int blkcipher_don |
| |
| scatterwalk_advance(&walk->in, n); |
| scatterwalk_advance(&walk->out, n); |
| - |
| - return n; |
| } |
| |
| int blkcipher_walk_done(struct blkcipher_desc *desc, |
| struct blkcipher_walk *walk, int err) |
| { |
| - unsigned int nbytes = 0; |
| + unsigned int n; /* bytes processed */ |
| + bool more; |
| |
| - if (likely(err >= 0)) { |
| - unsigned int n = walk->nbytes - err; |
| + if (unlikely(err < 0)) |
| + goto finish; |
| |
| - if (likely(!(walk->flags & BLKCIPHER_WALK_SLOW))) |
| - n = blkcipher_done_fast(walk, n); |
| - else if (WARN_ON(err)) { |
| + n = walk->nbytes - err; |
| + walk->total -= n; |
| + more = (walk->total != 0); |
| + |
| + if (likely(!(walk->flags & BLKCIPHER_WALK_SLOW))) { |
| + blkcipher_done_fast(walk, n); |
| + } else { |
| + if (WARN_ON(err)) { |
| + /* unexpected case; didn't process all bytes */ |
| err = -EINVAL; |
| - goto err; |
| - } else |
| - n = blkcipher_done_slow(walk, n); |
| - |
| - nbytes = walk->total - n; |
| - err = 0; |
| + goto finish; |
| + } |
| + blkcipher_done_slow(walk, n); |
| } |
| |
| - scatterwalk_done(&walk->in, 0, nbytes); |
| - scatterwalk_done(&walk->out, 1, nbytes); |
| - |
| -err: |
| - walk->total = nbytes; |
| - walk->nbytes = nbytes; |
| + scatterwalk_done(&walk->in, 0, more); |
| + scatterwalk_done(&walk->out, 1, more); |
| |
| - if (nbytes) { |
| + if (more) { |
| crypto_yield(desc->flags); |
| return blkcipher_walk_next(desc, walk); |
| } |
| - |
| + err = 0; |
| +finish: |
| + walk->nbytes = 0; |
| if (walk->iv != desc->info) |
| memcpy(desc->info, walk->iv, walk->ivsize); |
| if (walk->buffer != walk->page) |
| kfree(walk->buffer); |
| if (walk->page) |
| free_page((unsigned long)walk->page); |
| - |
| return err; |
| } |
| EXPORT_SYMBOL_GPL(blkcipher_walk_done); |