On 12/5/23 4:13 PM, Alexei Starovoitov wrote:
On Mon, Dec 4, 2023 at 11:01 PM Yonghong Song <yonghong.song@xxxxxxxxx> wrote:
Er, it is not what I want, although I have written a similar patch in
which bpf_map_put() will call btf_put() and set map->btf as NULL if
there is no BPF_LIST_HEAD and BPF_RB_ROOT fields in map->record,
otherwise calling bpf_put() in bpf_put_free_deferred(). What I have
suggested is to optionally pin btf in graph_root.btf just like
btf_field_kptr does.
Okay, I see what you mean. This is actually what I kind of think
as well in below to identify *all* cases btf data might be accessed.
I didn't explicitly mention this approach in detail but the idea is
to get a reference count for btf and later release it during btf_record_free.
I think this should work. I need to do an audit then to find other potential
places, if exists, to do similar things. The current approach
is simpler but looks like we can do better with existing
btf_field_kptr approach.
imo that would be the only correct way to fix it.
we btf_get(kptr_btf) before saving it kptr.btf in btf_parse_kptr() and
btf_put() it eventually in btf_record_free().
graph_root looks buggy.
It saved the btf pointer in btf_parse_graph_root() without taking refcnt.
Agreed. Just send v3 patch:
https://lore.kernel.org/bpf/20231205224812.813224-1-yonghong.song@xxxxxxxxx/
