Skip to content

Commit

Permalink
bpf: Support refcounted local kptrs in existing semantics
Browse files Browse the repository at this point in the history
A local kptr is considered 'refcounted' when it is of a type that has a
bpf_refcount field. When such a kptr is created, its refcount should be
initialized to 1; when destroyed, the object should be free'd only if a
refcount decr results in 0 refcount.

Existing logic always frees the underlying memory when destroying a
local kptr, and 0-initializes all btf_record fields. This patch adds
checks for "is local kptr refcounted?" and new logic for that case in
the appropriate places.

This patch focuses on changing existing semantics and thus conspicuously
does _not_ provide a way for BPF programs in increment refcount. That
follows later in the series.

__bpf_obj_drop_impl is modified to do the right thing when it sees a
refcounted type. Container types for graph nodes (list, tree, stashed in
map) are migrated to use __bpf_obj_drop_impl as a destructor for their
nodes instead of each having custom destruction code in their _free
paths. Now that "drop" isn't a synonym for "free" when the type is
refcounted it makes sense to centralize this logic.

Signed-off-by: Dave Marchevsky <davemarchevsky@fb.com>
Link: https://lore.kernel.org/r/20230415201811.343116-4-davemarchevsky@fb.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
  • Loading branch information
davemarchevsky authored and Alexei Starovoitov committed Apr 16, 2023
1 parent d54730b commit 1512217
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 8 deletions.
3 changes: 3 additions & 0 deletions include/linux/bpf.h
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,9 @@ static inline void bpf_obj_init(const struct btf_record *rec, void *obj)
return;
for (i = 0; i < rec->cnt; i++)
memset(obj + rec->fields[i].offset, 0, rec->fields[i].size);

if (rec->refcount_off >= 0)
refcount_set((refcount_t *)(obj + rec->refcount_off), 1);
}

/* 'dst' must be a temporary buffer and should not point to memory that is being
Expand Down
21 changes: 13 additions & 8 deletions kernel/bpf/helpers.c
Original file line number Diff line number Diff line change
Expand Up @@ -1798,6 +1798,8 @@ bpf_base_func_proto(enum bpf_func_id func_id)
}
}

void __bpf_obj_drop_impl(void *p, const struct btf_record *rec);

void bpf_list_head_free(const struct btf_field *field, void *list_head,
struct bpf_spin_lock *spin_lock)
{
Expand Down Expand Up @@ -1828,13 +1830,8 @@ void bpf_list_head_free(const struct btf_field *field, void *list_head,
/* The contained type can also have resources, including a
* bpf_list_head which needs to be freed.
*/
bpf_obj_free_fields(field->graph_root.value_rec, obj);
/* bpf_mem_free requires migrate_disable(), since we can be
* called from map free path as well apart from BPF program (as
* part of map ops doing bpf_obj_free_fields).
*/
migrate_disable();
bpf_mem_free(&bpf_global_ma, obj);
__bpf_obj_drop_impl(obj, field->graph_root.value_rec);
migrate_enable();
}
}
Expand Down Expand Up @@ -1871,10 +1868,9 @@ void bpf_rb_root_free(const struct btf_field *field, void *rb_root,
obj = pos;
obj -= field->graph_root.node_offset;

bpf_obj_free_fields(field->graph_root.value_rec, obj);

migrate_disable();
bpf_mem_free(&bpf_global_ma, obj);
__bpf_obj_drop_impl(obj, field->graph_root.value_rec);
migrate_enable();
}
}
Expand All @@ -1897,8 +1893,17 @@ __bpf_kfunc void *bpf_obj_new_impl(u64 local_type_id__k, void *meta__ign)
return p;
}

/* Must be called under migrate_disable(), as required by bpf_mem_free */
void __bpf_obj_drop_impl(void *p, const struct btf_record *rec)
{
if (rec && rec->refcount_off >= 0 &&
!refcount_dec_and_test((refcount_t *)(p + rec->refcount_off))) {
/* Object is refcounted and refcount_dec didn't result in 0
* refcount. Return without freeing the object
*/
return;
}

if (rec)
bpf_obj_free_fields(rec, p);
bpf_mem_free(&bpf_global_ma, p);
Expand Down

0 comments on commit 1512217

Please sign in to comment.