Messages in this thread | | | Subject | Re: [PATCH bpf-next v2 1/3] bpf/verifier: Fix potential memory leak in array reallocation | From | Daniel Borkmann <> | Date | Tue, 1 Nov 2022 14:46:54 +0100 |
| |
[ +Lorenz ]
On 10/31/22 9:16 PM, Bill Wendling wrote: > On Fri, Oct 28, 2022 at 7:55 PM Kees Cook <keescook@chromium.org> wrote: >> >> If an error (NULL) is returned by krealloc(), callers of realloc_array() >> were setting their allocation pointers to NULL, but on error krealloc() >> does not touch the original allocation. This would result in a memory >> resource leak. Instead, free the old allocation on the error handling >> path. >> >> Cc: Alexei Starovoitov <ast@kernel.org> >> Cc: Daniel Borkmann <daniel@iogearbox.net> >> Cc: John Fastabend <john.fastabend@gmail.com> >> Cc: Andrii Nakryiko <andrii@kernel.org> >> Cc: Martin KaFai Lau <martin.lau@linux.dev> >> Cc: Song Liu <song@kernel.org> >> Cc: Yonghong Song <yhs@fb.com> >> Cc: KP Singh <kpsingh@kernel.org> >> Cc: Stanislav Fomichev <sdf@google.com> >> Cc: Hao Luo <haoluo@google.com> >> Cc: Jiri Olsa <jolsa@kernel.org> >> Cc: bpf@vger.kernel.org >> Signed-off-by: Kees Cook <keescook@chromium.org> > > Reviewed-by: Bill Wendling <morbo@google.com> > >> --- >> kernel/bpf/verifier.c | 9 +++++++-- >> 1 file changed, 7 insertions(+), 2 deletions(-) >> >> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c >> index 014ee0953dbd..eb8c34db74c7 100644 >> --- a/kernel/bpf/verifier.c >> +++ b/kernel/bpf/verifier.c >> @@ -1027,12 +1027,17 @@ static void *copy_array(void *dst, const void *src, size_t n, size_t size, gfp_t >> */ >> static void *realloc_array(void *arr, size_t old_n, size_t new_n, size_t size) >> { >> + void *new_arr; >> + >> if (!new_n || old_n == new_n) >> goto out; >> >> - arr = krealloc_array(arr, new_n, size, GFP_KERNEL); >> - if (!arr) >> + new_arr = krealloc_array(arr, new_n, size, GFP_KERNEL); >> + if (!new_arr) { >> + kfree(arr); >> return NULL; >> + } >> + arr = new_arr;
Fyi, I took this fix into bpf tree and improved commit log a bit with the one from Zhengchao [0] given yours came in first. Fixes tag would have been nice, I added the c69431aab67a to the commit message while applying.
[0] https://patchwork.kernel.org/project/netdevbpf/patch/20221031070812.339883-1-shaozhengchao@huawei.com/
>> if (new_n > old_n) >> memset(arr + old_n * size, 0, (new_n - old_n) * size); >> -- >> 2.34.1 >>
| |