Skip to contentSkip to conversation

bpf: Always use maximal size for copy_array()

PATCH Applied Kees Cook 2 participants 2 messages Dec 23, 2022 – Dec 28, 2022

Subsystems (2)
BPF BPF core
lore
Share & tools
lore

Kees Cook

author
#0 lore raw

PATCH 1/1 bpf: Always use maximal size for copy_array()

Instead of counting on prior allocations to have sized allocations to the next kmalloc bucket size, always perform a krealloc that is at least ksize(dst) in size (which is a no-op), so the size can be correctly tracked by all the various allocation size trackers (KASAN, __alloc_size, etc).

Reported-by Hyunwoo Kim <[email protected]>
Link https://lore.kernel.org/bpf/20221223094551.GA1439509@ubuntu open here
Fixes ceb35b666d42 ("bpf/verifier: Use kmalloc_size_roundup() to match ksize() usage")
Cc Alexei Starovoitov <[email protected]>
Cc Daniel Borkmann <[email protected]>
Cc John Fastabend <[email protected]>
Cc Andrii Nakryiko <[email protected]>
Cc Martin KaFai Lau <[email protected]>
Cc Song Liu <[email protected]>
Cc Yonghong Song <[email protected]>
Cc KP Singh <[email protected]>
Cc Stanislav Fomichev <[email protected]>
Cc Hao Luo <[email protected]>
Cc Jiri Olsa <[email protected]>
Cc [email protected]
Signed-off-by Kees Cook <[email protected]>
---
 kernel/bpf/verifier.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)
kernel/bpf/verifier.c +7 -5
index a5255a0dcbb6..969e61bd6d39 100644
@@ -1054,6 +1054,8 @@ static void print_insn_state(struct bpf_verifier_env *env,
  */
 static void *copy_array(void *dst, const void *src, size_t n, size_t size, gfp_t flags)
 {
+	size_t alloc_bytes;
+	void *orig = dst;
 	size_t bytes;
 
 	if (ZERO_OR_NULL_PTR(src))

@@ -1062,11 +1064,11 @@ static void *copy_array(void *dst, const void *src, size_t n, size_t size, gfp_t
 	if (unlikely(check_mul_overflow(n, size, &bytes)))
 		return NULL;
 
-	if (ksize(dst) < ksize(src)) {
-		kfree(dst);
-		dst = kmalloc_track_caller(kmalloc_size_roundup(bytes), flags);
-		if (!dst)
-			return NULL;
+	alloc_bytes = max(ksize(orig), kmalloc_size_roundup(bytes));
+	dst = krealloc(orig, alloc_bytes, flags);
+	if (!dst) {
+		kfree(orig);
+		return NULL;
 	}
 
 	memcpy(dst, src, bytes);
Signature · 1 line
2.34.1
CI Applied
#1 lore raw
This patch was applied to bpf/bpf.git (master) by Alexei Starovoitov <[email protected]>:

Hello:

This patch was applied to bpf/bpf.git (master)
by Alexei Starovoitov <[email protected]>:

Quoted 23 lines · On Fri, 23 Dec 2022 10:28:44 -0800 you wrote:

Instead of counting on prior allocations to have sized allocations to the next kmalloc bucket size, always perform a krealloc that is at least ksize(dst) in size (which is a no-op), so the size can be correctly tracked by all the various allocation size trackers (KASAN, __alloc_size, etc).

Reported-by Hyunwoo Kim <[email protected]>
Link https://lore.kernel.org/bpf/20221223094551.GA1439509@ubuntu open here
Fixes ceb35b666d42 ("bpf/verifier: Use kmalloc_size_roundup() to match ksize() usage")
Cc Alexei Starovoitov <[email protected]>
Cc Daniel Borkmann <[email protected]>
Cc John Fastabend <[email protected]>
Cc Andrii Nakryiko <[email protected]>
Cc Martin KaFai Lau <[email protected]>
Cc Song Liu <[email protected]>
Cc Yonghong Song <[email protected]>
Cc KP Singh <[email protected]>
Cc Stanislav Fomichev <[email protected]>
Cc Hao Luo <[email protected]>
Cc Jiri Olsa <[email protected]>
Cc [email protected]
Signed-off-by Kees Cook <[email protected]>
Quoted 1 line

[...]

Here is the summary with links:
- bpf: Always use maximal size for copy_array()

    https://git.kernel.org/bpf/bpf/c/45435d8da71f

You are awesome, thank you!

Signature · 2 lines
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html