Skip to content

Commit

Permalink
bpf: Zeroing allocated object from slab in bpf memory allocator
Browse files Browse the repository at this point in the history
Currently the freed element in bpf memory allocator may be immediately
reused, for htab map the reuse will reinitialize special fields in map
value (e.g., bpf_spin_lock), but lookup procedure may still access
these special fields, and it may lead to hard-lockup as shown below:

 NMI backtrace for cpu 16
 CPU: 16 PID: 2574 Comm: htab.bin Tainted: G             L     6.1.0+ #1
 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
 RIP: 0010:queued_spin_lock_slowpath+0x283/0x2c0
 ......
 Call Trace:
  <TASK>
  copy_map_value_locked+0xb7/0x170
  bpf_map_copy_value+0x113/0x3c0
  __sys_bpf+0x1c67/0x2780
  __x64_sys_bpf+0x1c/0x20
  do_syscall_64+0x30/0x60
  entry_SYSCALL_64_after_hwframe+0x46/0xb0
 ......
  </TASK>

For htab map, just like the preallocated case, these is no need to
initialize these special fields in map value again once these fields
have been initialized. For preallocated htab map, these fields are
initialized through __GFP_ZERO in bpf_map_area_alloc(), so do the
similar thing for non-preallocated htab in bpf memory allocator. And
there is no need to use __GFP_ZERO for per-cpu bpf memory allocator,
because __alloc_percpu_gfp() does it implicitly.

Fixes: 0fd7c5d ("bpf: Optimize call_rcu in non-preallocated hash map.")
Signed-off-by: Hou Tao <houtao1@huawei.com>
Link: https://lore.kernel.org/r/20230215082132.3856544-2-houtao@huaweicloud.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
  • Loading branch information
Hou Tao authored and Alexei Starovoitov committed Feb 15, 2023
1 parent b2d9002 commit 997849c
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 3 deletions.
7 changes: 7 additions & 0 deletions include/linux/bpf.h
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,13 @@ static inline void bpf_obj_init(const struct btf_field_offs *foffs, void *obj)
memset(obj + foffs->field_off[i], 0, foffs->field_sz[i]);
}

/* 'dst' must be a temporary buffer and should not point to memory that is being
* used in parallel by a bpf program or bpf syscall, otherwise the access from
* the bpf program or bpf syscall may be corrupted by the reinitialization,
* leading to weird problems. Even 'dst' is newly-allocated from bpf memory
* allocator, it is still possible for 'dst' to be used in parallel by a bpf
* program or bpf syscall.
*/
static inline void check_and_init_map_value(struct bpf_map *map, void *dst)
{
bpf_obj_init(map->field_offs, dst);
Expand Down
4 changes: 2 additions & 2 deletions kernel/bpf/hashtab.c
Original file line number Diff line number Diff line change
Expand Up @@ -1004,8 +1004,6 @@ static struct htab_elem *alloc_htab_elem(struct bpf_htab *htab, void *key,
l_new = ERR_PTR(-ENOMEM);
goto dec_count;
}
check_and_init_map_value(&htab->map,
l_new->key + round_up(key_size, 8));
}

memcpy(l_new->key, key, key_size);
Expand Down Expand Up @@ -1592,6 +1590,7 @@ static int __htab_map_lookup_and_delete_elem(struct bpf_map *map, void *key,
else
copy_map_value(map, value, l->key +
roundup_key_size);
/* Zeroing special fields in the temp buffer */
check_and_init_map_value(map, value);
}

Expand Down Expand Up @@ -1792,6 +1791,7 @@ __htab_map_lookup_and_delete_batch(struct bpf_map *map,
true);
else
copy_map_value(map, dst_val, value);
/* Zeroing special fields in the temp buffer */
check_and_init_map_value(map, dst_val);
}
if (do_delete) {
Expand Down
2 changes: 1 addition & 1 deletion kernel/bpf/memalloc.c
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ static void *__alloc(struct bpf_mem_cache *c, int node)
return obj;
}

return kmalloc_node(c->unit_size, flags, node);
return kmalloc_node(c->unit_size, flags | __GFP_ZERO, node);
}

static struct mem_cgroup *get_memcg(const struct bpf_mem_cache *c)
Expand Down

0 comments on commit 997849c

Please sign in to comment.