general protection fault in af_alg_free_areq_sgls

[syzbot]

Hello,

syzkaller hit the following crash on
b0a84f19a5161418d4360cd57603e94ed489915e
git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/master
compiler: gcc (GCC) 7.1.1 20170620
.config is attached
Raw console output is attached.

Unfortunately, I don't have any reproducer for this bug yet.


R10: 0000000000000000 R11: 0000000000000212 R12: 00000000004b759b
R13: 00007f197dd47bc8 R14: 00000000004b759b R15: 0000000000000000
kasan: CONFIG_KASAN_INLINE enabled
kasan: GPF could be caused by NULL-ptr deref or user memory access
general protection fault: 0000 [#1] SMP KASAN
Dumping ftrace buffer:
(ftrace buffer empty)
Modules linked in:
CPU: 0 PID: 17307 Comm: syz-executor3 Not tainted 4.14.0-next-20171127+ #53
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
task: ffff880182f0e4c0 task.stack: ffff8801cf0b0000
RIP: 0010:compound_head include/linux/page-flags.h:147 [inline]
RIP: 0010:put_page include/linux/mm.h:850 [inline]
RIP: 0010:af_alg_free_areq_sgls+0x5d1/0xab0 crypto/af_alg.c:678
RSP: 0018:ffff8801cf0b7690 EFLAGS: 00010246
RAX: 0000000000000000 RBX: dffffc0000000000 RCX: ffffffff823a1c9a
RDX: 000000000000c34d RSI: ffffc900027de000 RDI: 0000000000000000
RBP: ffff8801cf0b7888 R08: 1ffff10039e16e56 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
R13: 0000000000000000 R14: ffff8801d87a7940 R15: ffffed0039e16f04
FS: 00007f197dd48700(0000) GS:ffff8801db400000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f197dd27000 CR3: 00000001d987d000 CR4: 00000000001406f0
DR0: 0000000020000000 DR1: 0000000020000020 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000600
Call Trace:
af_alg_free_resources+0x36/0x80 crypto/af_alg.c:1030
_skcipher_recvmsg crypto/algif_skcipher.c:152 [inline]
skcipher_recvmsg+0x8e5/0xf20 crypto/algif_skcipher.c:165
sock_recvmsg_nosec net/socket.c:805 [inline]
sock_recvmsg+0xc9/0x110 net/socket.c:812
sock_read_iter+0x361/0x560 net/socket.c:889
call_read_iter include/linux/fs.h:1766 [inline]
aio_read+0x2b0/0x3a0 fs/aio.c:1501
io_submit_one fs/aio.c:1611 [inline]
do_io_submit+0xf99/0x14f0 fs/aio.c:1682
SYSC_io_submit fs/aio.c:1707 [inline]
SyS_io_submit+0x27/0x30 fs/aio.c:1704
entry_SYSCALL_64_fastpath+0x1f/0x96
RIP: 0033:0x4529d9
RSP: 002b:00007f197dd47c58 EFLAGS: 00000212 ORIG_RAX: 00000000000000d1
RAX: ffffffffffffffda RBX: 0000000000758020 RCX: 00000000004529d9
RDX: 0000000020bd9fe0 RSI: 0000000000000001 RDI: 00007f197dd27000
RBP: 0000000000000086 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000212 R12: 00000000004b759b
R13: 00007f197dd47bc8 R14: 00000000004b759b R15: 0000000000000000
Code: 00 00 48 8d 45 98 48 bb 00 00 00 00 00 fc ff df 48 89 85 48 fe ff ff
48 c1 e8 03 4c 8d 3c 18 e8 a6 fe 35 ff 4c 89 e0 48 c1 e8 03 <80> 3c 18 00
0f 85 4b 03 00 00 49 8b 04 24 48 83 e0 fc 48 89 85
RIP: compound_head include/linux/page-flags.h:147 [inline] RSP:
ffff8801cf0b7690
RIP: put_page include/linux/mm.h:850 [inline] RSP: ffff8801cf0b7690
RIP: af_alg_free_areq_sgls+0x5d1/0xab0 crypto/af_alg.c:678 RSP:
ffff8801cf0b7690
---[ end trace 108559a7b630c51e ]---
Kernel panic - not syncing: Fatal exception
Dumping ftrace buffer:
(ftrace buffer empty)
Kernel Offset: disabled


---
This bug is generated by a dumb bot. It may contain errors.
See https://goo.gl/tpsmEJ for details.
Direct all questions to syzk...@googlegroups.com.
Please credit me with: Reported-by: syzbot <syzk...@googlegroups.com>

syzbot will keep track of this bug report.
Once a fix for this bug is committed, please reply to this email with:
#syz fix: exact-commit-title
To mark this as a duplicate of another syzbot report, please reply with:
#syz dup: exact-subject-of-another-report
If it's a one-off invalid bug report, please reply with:
#syz invalid
Note: if the crash happens again, it will cause creation of a new bug
report.
Note: all commands must start from beginning of the line in the email body.

[Eric Biggers]

On Mon, Nov 27, 2017 at 10:56:47AM -0800, syzbot wrote:
> Hello,
>
> syzkaller hit the following crash on
> b0a84f19a5161418d4360cd57603e94ed489915e
> git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/master
> compiler: gcc (GCC) 7.1.1 20170620
> .config is attached
> Raw console output is attached.
>
> Unfortunately, I don't have any reproducer for this bug yet.
>

This was probably caused by taking the path where areq->tsgl could not be
allocated. (syzkaller probably reached it after injecting a memory allocation
failure.) The following should fix it:

---8<---

From 1a7a7f86f09c50652f1fff75b8d3a32712826b32 Mon Sep 17 00:00:00 2001
From: Eric Biggers <ebig...@google.com>
Date: Tue, 28 Nov 2017 00:46:24 -0800
Subject: [PATCH] crypto: af_alg - fix NULL pointer dereference in
af_alg_free_areq_sgls()

If allocating the ->tsgl member of 'struct af_alg_async_req' failed,
during cleanup we dereferenced the NULL ->tsgl pointer in
af_alg_free_areq_sgls(), because ->tsgl_entries was nonzero.

Fix it by only freeing the ->tsgl list if it is non-NULL.

This affected both algif_skcipher and algif_aead.

Fixes: e870456d8e7c ("crypto: algif_skcipher - overhaul memory management")
Fixes: d887c52d6ae4 ("crypto: algif_aead - overhaul memory management")
Reported-by: syzbot <syzk...@googlegroups.com>
Cc: <sta...@vger.kernel.org> # v4.14+
Signed-off-by: Eric Biggers <ebig...@google.com>
---
crypto/af_alg.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/crypto/af_alg.c b/crypto/af_alg.c
index 358749c38894..415a54ced4d6 100644
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c
@@ -672,14 +672,15 @@ void af_alg_free_areq_sgls(struct af_alg_async_req *areq)
}

tsgl = areq->tsgl;
- for_each_sg(tsgl, sg, areq->tsgl_entries, i) {
- if (!sg_page(sg))
- continue;
- put_page(sg_page(sg));
- }
+ if (tsgl) {
+ for_each_sg(tsgl, sg, areq->tsgl_entries, i) {
+ if (!sg_page(sg))
+ continue;
+ put_page(sg_page(sg));
+ }

- if (areq->tsgl && areq->tsgl_entries)
sock_kfree_s(sk, tsgl, areq->tsgl_entries * sizeof(*tsgl));
+ }
}
EXPORT_SYMBOL_GPL(af_alg_free_areq_sgls);

--
2.15.0

[Stephan Mueller]

Am Dienstag, 28. November 2017, 10:02:52 CET schrieb Eric Biggers:

Hi Eric,

> ---
> crypto/af_alg.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/crypto/af_alg.c b/crypto/af_alg.c
> index 358749c38894..415a54ced4d6 100644
> --- a/crypto/af_alg.c
> +++ b/crypto/af_alg.c
> @@ -672,14 +672,15 @@ void af_alg_free_areq_sgls(struct af_alg_async_req
> *areq) }
>
> tsgl = areq->tsgl;
> - for_each_sg(tsgl, sg, areq->tsgl_entries, i) {
> - if (!sg_page(sg))
> - continue;
> - put_page(sg_page(sg));
> - }
> + if (tsgl) {
> + for_each_sg(tsgl, sg, areq->tsgl_entries, i) {
> + if (!sg_page(sg))
> + continue;
> + put_page(sg_page(sg));
> + }
>
> - if (areq->tsgl && areq->tsgl_entries)

Why do you want to remove the check for areq->tsgl_entries? I know in the
current code that cannot happen. But it should be caught in case of a
programming error.

Thus, should we add a BUG_ON(!areq->tsgl_entries)?

Otherwise:

Reviewed-by: Stephan Mueller <smue...@chronox.de>

Ciao
Stephan

[Eric Biggers]

On Tue, Nov 28, 2017 at 10:10:55AM +0100, Stephan Mueller wrote:
> > diff --git a/crypto/af_alg.c b/crypto/af_alg.c
> > index 358749c38894..415a54ced4d6 100644
> > --- a/crypto/af_alg.c
> > +++ b/crypto/af_alg.c
> > @@ -672,14 +672,15 @@ void af_alg_free_areq_sgls(struct af_alg_async_req
> > *areq) }
> >
> > tsgl = areq->tsgl;
> > - for_each_sg(tsgl, sg, areq->tsgl_entries, i) {
> > - if (!sg_page(sg))
> > - continue;
> > - put_page(sg_page(sg));
> > - }
> > + if (tsgl) {
> > + for_each_sg(tsgl, sg, areq->tsgl_entries, i) {
> > + if (!sg_page(sg))
> > + continue;
> > + put_page(sg_page(sg));
> > + }
> >
> > - if (areq->tsgl && areq->tsgl_entries)
>
> Why do you want to remove the check for areq->tsgl_entries? I know in the
> current code that cannot happen. But it should be caught in case of a
> programming error.
>
> Thus, should we add a BUG_ON(!areq->tsgl_entries)?
>

sock_kfree_s() works even if the size is 0. So there's no reason to check.

Eric

[Herbert Xu]

On Tue, Nov 28, 2017 at 01:02:52AM -0800, Eric Biggers wrote:
>
> >From 1a7a7f86f09c50652f1fff75b8d3a32712826b32 Mon Sep 17 00:00:00 2001
> From: Eric Biggers <ebig...@google.com>
> Date: Tue, 28 Nov 2017 00:46:24 -0800
> Subject: [PATCH] crypto: af_alg - fix NULL pointer dereference in
> af_alg_free_areq_sgls()
>
> If allocating the ->tsgl member of 'struct af_alg_async_req' failed,
> during cleanup we dereferenced the NULL ->tsgl pointer in
> af_alg_free_areq_sgls(), because ->tsgl_entries was nonzero.
>
> Fix it by only freeing the ->tsgl list if it is non-NULL.
>
> This affected both algif_skcipher and algif_aead.
>
> Fixes: e870456d8e7c ("crypto: algif_skcipher - overhaul memory management")
> Fixes: d887c52d6ae4 ("crypto: algif_aead - overhaul memory management")
> Reported-by: syzbot <syzk...@googlegroups.com>
> Cc: <sta...@vger.kernel.org> # v4.14+
> Signed-off-by: Eric Biggers <ebig...@google.com>

Patch applied. Thanks.
--
Email: Herbert Xu <her...@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[Eric Biggers]

On Wed, Nov 29, 2017 at 04:23:53PM +1100, Herbert Xu wrote:
> On Tue, Nov 28, 2017 at 01:02:52AM -0800, Eric Biggers wrote:
> >
> > >From 1a7a7f86f09c50652f1fff75b8d3a32712826b32 Mon Sep 17 00:00:00 2001
> > From: Eric Biggers <ebig...@google.com>
> > Date: Tue, 28 Nov 2017 00:46:24 -0800
> > Subject: [PATCH] crypto: af_alg - fix NULL pointer dereference in
> > af_alg_free_areq_sgls()
> >
> > If allocating the ->tsgl member of 'struct af_alg_async_req' failed,
> > during cleanup we dereferenced the NULL ->tsgl pointer in
> > af_alg_free_areq_sgls(), because ->tsgl_entries was nonzero.
> >
> > Fix it by only freeing the ->tsgl list if it is non-NULL.
> >
> > This affected both algif_skcipher and algif_aead.
> >
> > Fixes: e870456d8e7c ("crypto: algif_skcipher - overhaul memory management")
> > Fixes: d887c52d6ae4 ("crypto: algif_aead - overhaul memory management")
> > Reported-by: syzbot <syzk...@googlegroups.com>
> > Cc: <sta...@vger.kernel.org> # v4.14+
> > Signed-off-by: Eric Biggers <ebig...@google.com>
>
> Patch applied. Thanks.

Herbert, if it's not too late can you fix the subject? It got split into two
lines:

commit 887207ed9e5812ed9239b6d07185a2d35dda91db
Author: Eric Biggers <ebig...@google.com>
Date: Tue Nov 28 00:46:24 2017 -0800

crypto: af_alg - fix NULL pointer dereference in

af_alg_free_areq_sgls()

If allocating the ->tsgl member of 'struct af_alg_async_req' failed,
during cleanup we dereferenced the NULL ->tsgl pointer in
af_alg_free_areq_sgls(), because ->tsgl_entries was nonzero.

Fix it by only freeing the ->tsgl list if it is non-NULL.

This affected both algif_skcipher and algif_aead.

Fixes: e870456d8e7c ("crypto: algif_skcipher - overhaul memory management")
Fixes: d887c52d6ae4 ("crypto: algif_aead - overhaul memory management")
Reported-by: syzbot <syzk...@googlegroups.com>
Cc: <sta...@vger.kernel.org> # v4.14+
Signed-off-by: Eric Biggers <ebig...@google.com>

Reviewed-by: Stephan Mueller <smue...@chronox.de>
Signed-off-by: Herbert Xu <her...@gondor.apana.org.au>

[Herbert Xu]

On Wed, Nov 29, 2017 at 11:51:09AM -0800, Eric Biggers wrote:
>
> Herbert, if it's not too late can you fix the subject? It got split into two
> lines:

Sorry, it's already pushed out with other patches sitting on top
of it.

Cheers,

[syzbot]

syzkaller has found reproducer for the following crash on
3c1c4ddffb58b9e10b3365764fe59546130b3f32
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/master

compiler: gcc (GCC) 7.1.1 20170620
.config is attached
Raw console output is attached.

C reproducer is attached
syzkaller reproducer is attached. See https://goo.gl/kgGztJ
for information about syzkaller reproducers

kasan: CONFIG_KASAN_INLINE enabled
kasan: GPF could be caused by NULL-ptr deref or user memory access
general protection fault: 0000 [#1] SMP KASAN
Dumping ftrace buffer:
(ftrace buffer empty)
Modules linked in:

CPU: 1 PID: 3084 Comm: syzkaller426369 Not tainted 4.15.0-rc1+ #203

Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011

task: 000000009887e938 task.stack: 00000000ae5f34d5

RIP: 0010:compound_head include/linux/page-flags.h:147 [inline]
RIP: 0010:put_page include/linux/mm.h:850 [inline]
RIP: 0010:af_alg_free_areq_sgls+0x5d1/0xab0 crypto/af_alg.c:678

RSP: 0018:ffff8801cc2d7790 EFLAGS: 00010246
RAX: 0000000000000000 RBX: dffffc0000000000 RCX: ffffffff8239cc1a
RDX: 0000000000000000 RSI: ffffffff85f44540 RDI: 0000000000000000
RBP: ffff8801cc2d7988 R08: 1ffff1003985ae76 R09: 0000000000000000

R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000

R13: 0000000000000000 R14: 00000000fffffff4 R15: ffffed003985af24
FS: 00000000021ae940(0000) GS:ffff8801db500000(0000) knlGS:0000000000000000

CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033

CR2: 0000000020233fd0 CR3: 00000001cb97b000 CR4: 00000000001406e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400

Call Trace:
af_alg_free_resources+0x36/0x80 crypto/af_alg.c:1030

_aead_recvmsg crypto/algif_aead.c:316 [inline]
aead_recvmsg+0x14e1/0x1bc0 crypto/algif_aead.c:329

sock_recvmsg_nosec net/socket.c:805 [inline]
sock_recvmsg+0xc9/0x110 net/socket.c:812

___sys_recvmsg+0x29b/0x630 net/socket.c:2207
__sys_recvmsg+0xe2/0x210 net/socket.c:2252
SYSC_recvmsg net/socket.c:2264 [inline]
SyS_recvmsg+0x2d/0x50 net/socket.c:2259
entry_SYSCALL_64_fastpath+0x1f/0x96
RIP: 0033:0x440f09
RSP: 002b:00007fff6ddf3098 EFLAGS: 00000246 ORIG_RAX: 000000000000002f
RAX: ffffffffffffffda RBX: 0000000000000001 RCX: 0000000000440f09
RDX: 0000000000000000 RSI: 00000000209c6fc8 RDI: 0000000000000004
RBP: 0000000000000005 R08: 0000000000000001 R09: 00000000021a0032
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000402830
R13: 00000000004028c0 R14: 0000000000000000 R15: 0000000000000000

Code: 00 00 48 8d 45 98 48 bb 00 00 00 00 00 fc ff df 48 89 85 48 fe ff ff

48 c1 e8 03 4c 8d 3c 18 e8 16 2c 36 ff 4c 89 e0 48 c1 e8 03 <80> 3c 18 00

0f 85 4b 03 00 00 49 8b 04 24 48 83 e0 fc 48 89 85
RIP: compound_head include/linux/page-flags.h:147 [inline] RSP:

ffff8801cc2d7790
RIP: put_page include/linux/mm.h:850 [inline] RSP: ffff8801cc2d7790

RIP: af_alg_free_areq_sgls+0x5d1/0xab0 crypto/af_alg.c:678 RSP:

ffff8801cc2d7790
---[ end trace de1b188df438eb0f ]---

Kernel panic - not syncing: Fatal exception
Dumping ftrace buffer:
(ftrace buffer empty)
Kernel Offset: disabled

Rebooting in 86400 seconds..

[Eric Biggers]

#syz fix: crypto: af_alg - fix NULL pointer dereference in