Hi, Eric,
Thanks for your reply.
I have run your program on an original kernel and it reproduced the
crash. And I also run the program on a kernel with our patch, but there was no
crash.
I think the reason of the crash is the parameter buffer is aligned
with the page . So the address of the parameter buffer starts at the beginning
of the page, which making "walk->offset = 0" and generating the crash. I add
some logs in "scatterwalk_pagedone()" to print the value of walk->offset, and
the log before the crash shows that "walk->offset = 0".
And I do not understand why "walk->offset = 0" means no data to be
processed. In the structure " scatterlist", the member "offset" represents the
offset of the buffer in the page, and the member length represents the length
of the buffer. In function "af_alg_make_sg()", if a buffer occupies more than
one pages, the offset will also be set to 0 in the second and following pages.
And In function scatterwalk_done(), walk->offset = 0 will also allow to call
"scatterwalk_pagedone()". So I think that when "walk->offset = 0" the page
needs to be flushed as well.
BRs
GaoKui
> -邮件原件-
> 发件人: linux-crypto-ow...@vger.kernel.org [mailto:linux-crypto-
> ow...@vger.kernel.org] 代表 Eric Biggers
> 发送时间: 2018年7月24日 1:55
> 收件人: linux-crypto@vger.kernel.org; Herbert Xu
>
> 抄送: Liuchao (H) ; 罗新强
> ; gaokui (A) ; Eric
> Biggers
> 主题: [PATCH 1/3] crypto: skcipher - fix crash flushing dcache in error path
>
> From: Eric Biggers
>
> 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
> skcipher_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 skcipher_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
> #include
> #include
>
> int main()
> {
> struct sockaddr_alg addr = {
> .salg_type = "skcipher",
> .salg_name = "cbc(aes-generic)",
> };
> char buffer[4096] __attribute__((aligned(4096))) = { 0 };
> int fd;
>
> fd = socket(AF_ALG, SOCK_SEQPACKET, 0);
> bind(fd, (void *), 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
> Fixes: b286d8b1a690 ("crypto: skcipher - Add skcipher walk interface")
> Cc: # v4.10+
> Signed-off-by: Eric Biggers
> ---
> crypto/skcipher.c | 53 ---
> 1 file changed, 27 insertions(+), 26 deletions(-)
>
> diff --git a/crypto/skcipher.c b/crypto/skcipher.c index
> 7d6a49fe3047..5f7017b36d75 100644
> --- a/crypto/skcipher.c
> +++ b/crypto/skcipher.c
> @@ -95,7 +95,7 @@ static inline u8 *skcipher_get_spot(u8 *start,
> unsigned int len)
> return max(start, end_page);
> }
>
> -static int skcipher_done_slow(struct skcipher_walk *walk, unsigned int
> bsize)
> +static void skcipher_done_slow(struct skcipher_walk *walk, unsigned int
> +bsize)
> {
> u8 *addr;
>
> @@ -103,23 +103,24 @@ static int skcipher_done_slow(struct
> skcipher_walk *walk, unsigned int bsize)
> addr = skcipher_get_spot(addr, bsize);
> scatterwalk_copychunks(addr, >out, bsize,
> (walk->flags & SKCIPHER_WALK_PHYS) ? 2 : 1);
> - return 0;
> }
>
> int skcipher_walk_done(struct skcipher_walk *walk, int err) {
> - unsigned int n = walk->nbytes - err;
> - unsigned int nbytes;
> -
> - nbytes = walk->total - n;
> -
> - if (unlikely(err < 0)) {
> - nbytes = 0;
> - n = 0;
> - } else if (likely(!(walk->flags & (SKCIPHER_WALK_PHYS |
> -SKCIPHER_WALK_SLOW |
> -SKCIPHER_WALK_COPY |
> -SKCIPHER_WALK_DIFF {
> + unsigned int n; /* bytes processed */
> + bool more;
> +
> + if (unlikely(err < 0))
> + goto finish;
> +
> + n = walk->nbytes - err;
> + walk->total -= n;
> + more = (walk->total != 0);
> +
> + if (likely(!(walk->flags & (SKCIPHER_WALK_PHYS |
> + SKCIPHER_WALK_SLOW |
> +
