net: user-controllable kmalloc size in __sctp_setsockopt_connectx

[Dmitry Vyukov]

Hello,

The following program triggers WARNING in kmalloc:

// autogenerated by syzkaller (http://github.com/google/syzkaller)
#include <unistd.h>
#include <sys/syscall.h>
#include <string.h>
#include <stdint.h>

int main()
{
long r0 = syscall(SYS_mmap, 0x20000000ul, 0x4000ul, 0x3ul,
0x32ul, 0xfffffffffffffffful, 0x0ul);
long r1 = syscall(SYS_socket, 0x2ul, 0x80801ul, 0x84ul, 0, 0, 0);
*(uint32_t*)0x20002fb0 = (uint32_t)0x5fb;
*(uint32_t*)0x20002fb4 = (uint32_t)0xffffffffffffffff;
*(uint32_t*)0x20002fb8 = (uint32_t)0x0;
*(uint32_t*)0x20002fbc = (uint32_t)0x0;
*(uint32_t*)0x20002fc0 = (uint32_t)0xffffffffffffffff;
*(uint16_t*)0x20002fc4 = (uint16_t)0x7;
*(uint16_t*)0x20002fc6 = (uint16_t)0x8;
*(uint64_t*)0x20002fc8 = (uint64_t)0xa1d;
*(uint64_t*)0x20002fd0 = (uint64_t)0xd775;
*(uint64_t*)0x20002fd8 = (uint64_t)0x9;
*(uint64_t*)0x20002fe0 = (uint64_t)0x26;
*(uint64_t*)0x20002fe8 = (uint64_t)0x2;
*(uint64_t*)0x20002ff0 = (uint64_t)0x997;
*(uint32_t*)0x20002ff8 = (uint32_t)0x0;
*(uint32_t*)0x20002ffc = (uint32_t)0xffffffffffffffff;
long r17 = syscall(SYS_msgctl, 0xfffffffffffffffful, 0xbul,
0x20002fb0ul, 0, 0, 0);
memcpy((void*)0x20001000,
"\xc4\xcb\x30\xad\x58\x07\xa7\x93\x4f\xba\x75\x75\x33\x9a\x9b\x14\x36\x28\x6d\xc6\x57\x57\xc0\x17\x3b\x03\x6e\xe8\xbd\x31\x99\x17\x1b\x18\xcb\x05\x31\x3b\xc5\x39\xda\xdf\x1f\x9f\x1f\xd0\x1c\xd4\xce\x04\x1c\x00\xa0\x0b\xf8\x13\xd6\x93\xbd\x43\x33\xcb\x6d\x18\x8f\xab\x15\x59\x79\x63\x0d\x3d\x8d\x11\xd4\xd5\x07",
77);
long r19 = syscall(SYS_msgsnd, 0x0ul, 0x20001000ul, 0x800ul, 0, 0, 0);
long r20 = syscall(SYS_getsockopt, r1, 0x84ul, 0x6ful,
0x20002fcbul, 0x20000ffful, 0);
return 0;
}

------------[ cut here ]------------
WARNING: CPU: 3 PID: 6724 at mm/page_alloc.c:2989
__alloc_pages_nodemask+0x771/0x15f0()
Modules linked in:
CPU: 3 PID: 6724 Comm: a.out Not tainted 4.4.0-rc6+ #173
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
00000000ffffffff ffff880069a276d0 ffffffff82899ffd 0000000000000000
ffff880064124680 ffffffff85deedc0 ffff880069a27710 ffffffff812ebbb9
ffffffff815f8f81 ffffffff85deedc0 0000000000000bad ffff880069a27998
Call Trace:
[< inline >] __dump_stack lib/dump_stack.c:15
[<ffffffff82899ffd>] dump_stack+0x6f/0xa2 lib/dump_stack.c:50
[<ffffffff812ebbb9>] warn_slowpath_common+0xd9/0x140 kernel/panic.c:460
[<ffffffff812ebde9>] warn_slowpath_null+0x29/0x30 kernel/panic.c:493
[< inline >] __alloc_pages_slowpath mm/page_alloc.c:2989
[<ffffffff815f8f81>] __alloc_pages_nodemask+0x771/0x15f0 mm/page_alloc.c:3235
[<ffffffff816ba01e>] alloc_pages_current+0xee/0x340 mm/mempolicy.c:2055
[< inline >] alloc_pages include/linux/gfp.h:451
[<ffffffff815f5136>] alloc_kmem_pages+0x16/0xf0 mm/page_alloc.c:3414
[<ffffffff81644cff>] kmalloc_order+0x1f/0x80 mm/slab_common.c:1007
[<ffffffff81644d7f>] kmalloc_order_trace+0x1f/0x140 mm/slab_common.c:1018
[< inline >] kmalloc_large include/linux/slab.h:390
[<ffffffff816c97de>] __kmalloc+0x2de/0x330 mm/slub.c:3555
[< inline >] kmalloc include/linux/slab.h:463
[<ffffffff8576a326>] __sctp_setsockopt_connectx+0xc6/0x150
net/sctp/socket.c:1318
[< inline >] sctp_getsockopt_connectx3 net/sctp/socket.c:1410
[<ffffffff8577a22e>] sctp_getsockopt+0x25ee/0x3e00 net/sctp/socket.c:6007
[<ffffffff84ba2d65>] sock_common_getsockopt+0x95/0xd0 net/core/sock.c:2601
[< inline >] SYSC_getsockopt net/socket.c:1782
[<ffffffff84ba0512>] SyS_getsockopt+0x142/0x230 net/socket.c:1764
[<ffffffff85cce8b6>] entry_SYSCALL_64_fastpath+0x16/0x7a
arch/x86/entry/entry_64.S:185
---[ end trace 142fd9e8ed8bda1f ]---

On commit 4ef7675344d687a0ef5b0d7c0cee12da005870c0 (Dec 20).

[Marcelo Ricardo Leitner]

Hi,

On Tue, Dec 22, 2015 at 09:13:54PM +0100, Dmitry Vyukov wrote:
> Hello,
...

>
> [<ffffffff8576a326>] __sctp_setsockopt_connectx+0xc6/0x150
> net/sctp/socket.c:1318
> [< inline >] sctp_getsockopt_connectx3 net/sctp/socket.c:1410
> [<ffffffff8577a22e>] sctp_getsockopt+0x25ee/0x3e00 net/sctp/socket.c:6007
> [<ffffffff84ba2d65>] sock_common_getsockopt+0x95/0xd0 net/core/sock.c:2601
> [< inline >] SYSC_getsockopt net/socket.c:1782
> [<ffffffff84ba0512>] SyS_getsockopt+0x142/0x230 net/socket.c:1764
> [<ffffffff85cce8b6>] entry_SYSCALL_64_fastpath+0x16/0x7a
> arch/x86/entry/entry_64.S:185

This is similar to that other one. I'll send a patch for it tomorrow.

Thanks,
Marcelo

[Marcelo Ricardo Leitner]

Commit cacc06215271 ("sctp: use GFP_USER for user-controlled kmalloc")
missed two other spots.

For connectx, as it's more likely to be used by kernel users of the API,
it detects if GFP_USER should be used or not.

Fixes: cacc06215271 ("sctp: use GFP_USER for user-controlled kmalloc")
Reported-by: Dmitry Vyukov <dvy...@google.com>
Signed-off-by: Marcelo Ricardo Leitner <marcelo...@gmail.com>
---
net/sctp/socket.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 2a1e8ba2808cab3bb29d670f155bcef243f85922..400a14d744834c7a503b338bc68f5f8b5b5dae8e 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -1301,8 +1301,9 @@ static int __sctp_setsockopt_connectx(struct sock *sk,
int addrs_size,
sctp_assoc_t *assoc_id)
{
- int err = 0;
struct sockaddr *kaddrs;
+ gfp_t gfp = GFP_KERNEL;
+ int err = 0;

pr_debug("%s: sk:%p addrs:%p addrs_size:%d\n",
__func__, sk, addrs, addrs_size);
@@ -1315,7 +1316,9 @@ static int __sctp_setsockopt_connectx(struct sock *sk,
return -EFAULT;

/* Alloc space for the address array in kernel memory. */
- kaddrs = kmalloc(addrs_size, GFP_KERNEL);
+ if (sk->sk_socket->file)
+ gfp = GFP_USER | __GFP_NOWARN;
+ kaddrs = kmalloc(addrs_size, gfp);
if (unlikely(!kaddrs))
return -ENOMEM;

@@ -5773,7 +5776,7 @@ static int sctp_getsockopt_assoc_ids(struct sock *sk, int len,

len = sizeof(struct sctp_assoc_ids) + sizeof(sctp_assoc_t) * num;

- ids = kmalloc(len, GFP_KERNEL);
+ ids = kmalloc(len, GFP_USER | __GFP_NOWARN);
if (unlikely(!ids))
return -ENOMEM;

--
2.5.0

[David Miller]

From: Marcelo Ricardo Leitner <marcelo...@gmail.com>
Date: Wed, 23 Dec 2015 16:28:40 -0200

> Commit cacc06215271 ("sctp: use GFP_USER for user-controlled kmalloc")
> missed two other spots.
>
> For connectx, as it's more likely to be used by kernel users of the API,
> it detects if GFP_USER should be used or not.
>
> Fixes: cacc06215271 ("sctp: use GFP_USER for user-controlled kmalloc")
> Reported-by: Dmitry Vyukov <dvy...@google.com>
> Signed-off-by: Marcelo Ricardo Leitner <marcelo...@gmail.com>

Applied.