| Message ID | 1531127419-47014-1-git-send-email-liuchao741@huawei.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Herbert Xu |
| Headers | show |
Hi Liu, On Mon, Jul 09, 2018 at 05:10:19PM +0800, Liu Chao wrote: > From: Luo Xinqiang <luoxinqiang2@huawei.com> > > In function scatterwalk_pagedone(), a kernel panic of invalid > page will occur if walk->offset equals 0. This patch fixes the > problem by setting the page addresswith sg_page(walk->sg) > directly if walk->offset equals 0. > > Panic call stack: > [<ffffff9632b4f418>] blkcipher_walk_done+0x430/0x8dc > [<ffffff9632b4ed50>] blkcipher_walk_next+0x750/0x9e8 > [<ffffff9632b4fe08>] blkcipher_walk_first+0x110/0x2c0 > [<ffffff9632b50084>] blkcipher_walk_virt+0xcc/0xe0 > [<ffffff96324b3680>] cbc_decrypt+0xdc/0x1a8 > [<ffffff9632ba3f90>] ablk_decrypt+0x138/0x224 > [<ffffff9632b50a90>] skcipher_decrypt_ablkcipher+0x130/0x150 > [<ffffff9632b9d6d4>] skcipher_recvmsg_sync.isra.17+0x270/0x404 > [<ffffff9632b9d900>] skcipher_recvmsg+0x98/0xb8 > [<ffffff9634044908>] SyS_recvfrom+0x2ac/0x2fc > [<ffffff96324839c0>] el0_svc_naked+0x34/0x38 > > Test: do syskaller fuzz test on 4.9 & 4.4 > > Signed-off-by: Gao Kui <gaokui1@huawei.com> > Signed-off-by: Luo Xinqiang <luoxinqiang2@huawei.com> > --- > crypto/scatterwalk.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/crypto/scatterwalk.c b/crypto/scatterwalk.c > index bc769c4..a265907 100644 > --- a/crypto/scatterwalk.c > +++ b/crypto/scatterwalk.c > @@ -53,7 +53,11 @@ static void scatterwalk_pagedone(struct scatter_walk *walk, int out, > if (out) { > struct page *page; > > - page = sg_page(walk->sg) + ((walk->offset - 1) >> PAGE_SHIFT); > + if (likely(walk->offset)) > + page = sg_page(walk->sg) + > + ((walk->offset - 1) >> PAGE_SHIFT); > + else > + page = sg_page(walk->sg); > /* Test ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE first as > * PageSlab cannot be optimised away per se due to > * use of volatile pointer. Interesting, I guess the reason this wasn't found by syzbot yet is that syzbot currently only runs on x86, where ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE isn't defined. Otherwise this crash reproduces on the latest kernel by running the following program: #include <linux/if_alg.h> #include <sys/socket.h> #include <unistd.h> int main() { struct sockaddr_alg addr = { .salg_type = "skcipher", .salg_name = "cbc(aes)", }; int algfd, reqfd; char buffer[4096] __attribute__((aligned(4096))) = { 0 }; algfd = socket(AF_ALG, SOCK_SEQPACKET, 0); bind(algfd, (void *)&addr, sizeof(addr)); setsockopt(algfd, SOL_ALG, ALG_SET_KEY, buffer, 32); reqfd = accept(algfd, NULL, NULL); write(reqfd, buffer, 15); read(reqfd, buffer, 15); } I don't think your fix makes sense though, because if walk->offset = 0 then no data was processed, so there would be no need to flush any page at all. I think the original intent was that scatterwalk_pagedone() only be called when a nonzero length was processed. So a better fix is probably to update blkcipher_walk_done() (and skcipher_walk_done() and ablkcipher_walk_done()) to avoid calling scatterwalk_pagedone() in the error case where no bytes were processed. I'm working on that fix but it's not ready quite yet. Thanks! - Eric
diff --git a/crypto/scatterwalk.c b/crypto/scatterwalk.c index bc769c4..a265907 100644 --- a/crypto/scatterwalk.c +++ b/crypto/scatterwalk.c @@ -53,7 +53,11 @@ static void scatterwalk_pagedone(struct scatter_walk *walk, int out, if (out) { struct page *page; - page = sg_page(walk->sg) + ((walk->offset - 1) >> PAGE_SHIFT); + if (likely(walk->offset)) + page = sg_page(walk->sg) + + ((walk->offset - 1) >> PAGE_SHIFT); + else + page = sg_page(walk->sg); /* Test ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE first as * PageSlab cannot be optimised away per se due to * use of volatile pointer.