[PATCH] bpf: check size before calling kvmalloc

* [PATCH] bpf: check size before calling kvmalloc
@ 2021-12-17 18:48 George Kennedy
  2021-12-17 22:45 ` Daniel Borkmann
  0 siblings, 1 reply; 4+ messages in thread
From: George Kennedy @ 2021-12-17 18:48 UTC (permalink / raw)
  To: sdf, ast, daniel; +Cc: george.kennedy, netdev, bpf, linux-kernel

ZERO_SIZE_PTR ((void *)16) is returned by kvmalloc() instead of NULL
if size is zero. Currently, return values from kvmalloc() are only
checked for NULL. Before calling kvmalloc() check for size of zero
and return error if size is zero to avoid the following crash.

BUG: kernel NULL pointer dereference, address: 0000000000000000
PGD 1030bd067 P4D 1030bd067 PUD 103497067 PMD 0
Oops: 0010 [#1] PREEMPT SMP KASAN NOPTI
CPU: 1 PID: 15094 Comm: syz-executor344 Not tainted 5.16.0-rc1-syzk #1
Hardware name: Red Hat KVM, BIOS
RIP: 0010:0x0
Code: Unable to access opcode bytes at RIP 0xffffffffffffffd6.
RSP: 0018:ffff888017627b78 EFLAGS: 00010246
RAX: 0000000000000000 RBX: ffff8880215d0780 RCX: ffffffff81b63c60
RDX: 0000000000000010 RSI: 0000000000000000 RDI: ffff8881035db400
RBP: ffff888017627f08 R08: ffffed1003697209 R09: ffffed1003697209
R10: ffff88801b4b9043 R11: ffffed1003697208 R12: ffffffff8f15d580
R13: 1ffff11002ec4f77 R14: ffff8881035db400 R15: 0000000000000000
FS:  00007f62bca78740(0000) GS:ffff888107880000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: ffffffffffffffd6 CR3: 000000002282a000 CR4: 00000000000006e0
Call Trace:
 <TASK>
 map_get_next_key kernel/bpf/syscall.c:1279 [inline]
 __sys_bpf+0x384d/0x5b30 kernel/bpf/syscall.c:4612
 __do_sys_bpf kernel/bpf/syscall.c:4722 [inline]
 __se_sys_bpf kernel/bpf/syscall.c:4720 [inline]
 __x64_sys_bpf+0x7a/0xc0 kernel/bpf/syscall.c:4720
 do_syscall_x64 arch/x86/entry/common.c:50 [inline]
 do_syscall_64+0x3a/0x80 arch/x86/entry/common.c:80
 entry_SYSCALL_64_after_hwframe+0x44/0xae

Reported-by: syzkaller <syzkaller@googlegroups.com>
Signed-off-by: George Kennedy <george.kennedy@oracle.com>
---
 kernel/bpf/syscall.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 1033ee8..9873723 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -1278,10 +1278,18 @@ static int map_get_next_key(union bpf_attr *attr)
 		key = NULL;
 	}
 
+	if (!map->key_size) {
+		err = -EINVAL;
+		goto err_put;
+	}
+
 	err = -ENOMEM;
 	next_key = kvmalloc(map->key_size, GFP_USER);
-	if (!next_key)
-		goto free_key;
+	if (!next_key) {
+		if (key)
+			goto free_key;
+		goto err_put;
+	}
 
 	if (bpf_map_is_dev_bound(map)) {
 		err = bpf_map_offload_get_next_key(map, key, next_key);
@@ -1331,6 +1339,8 @@ int generic_map_delete_batch(struct bpf_map *map,
 	if (!max_count)
 		return 0;
 
+	if (!map->key_size)
+		return -EINVAL;
 	key = kvmalloc(map->key_size, GFP_USER | __GFP_NOWARN);
 	if (!key)
 		return -ENOMEM;
@@ -1388,6 +1398,8 @@ int generic_map_update_batch(struct bpf_map *map,
 	if (!max_count)
 		return 0;
 
+	if (!map->key_size)
+		return -EINVAL;
 	key = kvmalloc(map->key_size, GFP_USER | __GFP_NOWARN);
 	if (!key)
 		return -ENOMEM;
@@ -1452,6 +1464,8 @@ int generic_map_lookup_batch(struct bpf_map *map,
 	if (put_user(0, &uattr->batch.count))
 		return -EFAULT;
 
+	if (!map->key_size)
+		return -EINVAL;
 	buf_prevkey = kvmalloc(map->key_size, GFP_USER | __GFP_NOWARN);
 	if (!buf_prevkey)
 		return -ENOMEM;
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread