Skip to content

Commit

Permalink
bpf/verifier: Use kmalloc_size_roundup() to match ksize() usage
Browse files Browse the repository at this point in the history
Most allocation sites in the kernel want an explicitly sized allocation
(and not "more"), and that dynamic runtime analysis tools (e.g. KASAN,
UBSAN_BOUNDS, FORTIFY_SOURCE, etc) are looking for precise bounds checking
(i.e. not something that is rounded up). A tiny handful of allocations
were doing an implicit alloc/realloc loop that actually depended on
ksize(), and didn't actually always call realloc. This has created a
long series of bugs and problems over many years related to the runtime
bounds checking, so these callers are finally being adjusted to _not_
depend on the ksize() side-effect, by doing one of several things:

- tracking the allocation size precisely and just never calling ksize()
  at all [1].

- always calling realloc and not using ksize() at all. (This solution
  ends up actually be a subset of the next solution.)

- using kmalloc_size_roundup() to explicitly round up the desired
  allocation size immediately [2].

The bpf/verifier case is this another of this latter case, and is the
last outstanding case to be fixed in the kernel.

Because some of the dynamic bounds checking depends on the size being an
_argument_ to an allocator function (i.e. see the __alloc_size attribute),
the ksize() users are rare, and it could waste local variables, it
was been deemed better to explicitly separate the rounding up from the
allocation itself [3].

Round up allocations with kmalloc_size_roundup() so that the verifier's
use of ksize() is always accurate.

[1] e.g.:
    https://git.kernel.org/linus/712f210a457d
    https://git.kernel.org/linus/72c08d9f4c72

[2] e.g.:
    https://git.kernel.org/netdev/net-next/c/12d6c1d3a2ad
    https://git.kernel.org/netdev/net-next/c/ab3f7828c979
    https://git.kernel.org/netdev/net-next/c/d6dd508080a3

[3] https://lore.kernel.org/lkml/0ea1fc165a6c6117f982f4f135093e69cb884930.camel@redhat.com/

Signed-off-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Stanislav Fomichev <sdf@google.com>
Link: https://lore.kernel.org/bpf/20221118183409.give.387-kees@kernel.org
  • Loading branch information
Kees Cook authored and Daniel Borkmann committed Nov 21, 2022
1 parent 35ffb1d commit ceb35b6
Showing 1 changed file with 8 additions and 4 deletions.
12 changes: 8 additions & 4 deletions kernel/bpf/verifier.c
Original file line number Diff line number Diff line change
Expand Up @@ -1022,9 +1022,9 @@ 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) < bytes) {
if (ksize(dst) < ksize(src)) {
kfree(dst);
dst = kmalloc_track_caller(bytes, flags);
dst = kmalloc_track_caller(kmalloc_size_roundup(bytes), flags);
if (!dst)
return NULL;
}
Expand All @@ -1041,12 +1041,14 @@ 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)
{
size_t alloc_size;
void *new_arr;

if (!new_n || old_n == new_n)
goto out;

new_arr = krealloc_array(arr, new_n, size, GFP_KERNEL);
alloc_size = kmalloc_size_roundup(size_mul(new_n, size));
new_arr = krealloc(arr, alloc_size, GFP_KERNEL);
if (!new_arr) {
kfree(arr);
return NULL;
Expand Down Expand Up @@ -2519,9 +2521,11 @@ static int push_jmp_history(struct bpf_verifier_env *env,
{
u32 cnt = cur->jmp_history_cnt;
struct bpf_idx_pair *p;
size_t alloc_size;

cnt++;
p = krealloc(cur->jmp_history, cnt * sizeof(*p), GFP_USER);
alloc_size = kmalloc_size_roundup(size_mul(cnt, sizeof(*p)));
p = krealloc(cur->jmp_history, alloc_size, GFP_USER);
if (!p)
return -ENOMEM;
p[cnt - 1].idx = env->insn_idx;
Expand Down

0 comments on commit ceb35b6

Please sign in to comment.