lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Fri, 28 Oct 2022 16:19:37 -0700 From: Kees Cook <keescook@...omium.org> To: Stanislav Fomichev <sdf@...gle.com> Cc: Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>, John Fastabend <john.fastabend@...il.com>, Andrii Nakryiko <andrii@...nel.org>, Martin KaFai Lau <martin.lau@...ux.dev>, Song Liu <song@...nel.org>, Yonghong Song <yhs@...com>, KP Singh <kpsingh@...nel.org>, Hao Luo <haoluo@...gle.com>, Jiri Olsa <jolsa@...nel.org>, bpf@...r.kernel.org, linux-kernel@...r.kernel.org, linux-hardening@...r.kernel.org Subject: Re: [PATCH] bpf: Use kmalloc_size_roundup() to match ksize() usage On Tue, Oct 18, 2022 at 01:07:45PM -0700, Stanislav Fomichev wrote: > On Tue, Oct 18, 2022 at 11:19 AM Kees Cook <keescook@...omium.org> wrote: > > > > On Tue, Oct 18, 2022 at 11:07:38AM -0700, sdf@...gle.com wrote: > > > On 10/18, Kees Cook wrote: > > > > Round up allocations with kmalloc_size_roundup() so that the verifier's > > > > use of ksize() is always accurate and no special handling of the memory > > > > is needed by KASAN, UBSAN_BOUNDS, nor FORTIFY_SOURCE. Pass the new size > > > > information back up to callers so they can use the space immediately, > > > > so array resizing to happen less frequently as well. Explicitly zero > > > > any trailing bytes in new allocations. > > > > > > > Additionally fix a memory allocation leak: if krealloc() fails, "arr" > > > > wasn't freed, but NULL was return to the caller of realloc_array() would > > > > be writing NULL to the lvalue, losing the reference to the original > > > > memory. > [...] > > > > - arr = krealloc_array(arr, new_n, size, GFP_KERNEL); > > > > - if (!arr) > > > > + alloc_size = kmalloc_size_roundup(size_mul(*new_n, size)); > > > > + arr = krealloc(old_arr, alloc_size, GFP_KERNEL); > > > > + if (!arr) { > > > > + kfree(old_arr); > > > > return NULL; > > > > + } > > > > > > Any reason not do hide this complexity behind krealloc_array? Why can't > > > it take care of those roundup details? > > > > It might be possible to do this with a macro, yes, but then callers > > aren't in a position to take advantage of the new size. Maybe we need > > something like: > > > > arr = krealloc_up(old_arr, alloc_size, &new_size, GFP_KERNEL); > > Maybe even krealloc_array_up(arr, &new_n, size, flags) or similar > where we return a new size? > Though I don't know if there are any other places in the kernel to > reuse it and warrant a new function.. Yeah, and it explicitly can't be a function, since GCC has broken attribute handling[1] for inlines. :( Regardless, I'll respin this with a macro and see how it looks. -Kees [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96503 -- Kees Cook
Powered by blists - more mailing lists