Re: [PATCH bpf-next v10 3/4] bpf: Add kfunc bpf_rcu_read_lock/unlock()

Yonghong Song
2022-11-24T23:44:32Z

On 11/24/22 12:34 PM, Alexei Starovoitov wrote:
> On Wed, Nov 23, 2022 at 9:32 PM Yonghong Song <yhs@fb.com> wrote:
>>
>> @@ -16580,6 +16682,8 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr, bpfptr_t uattr)
>> env->bypass_spec_v1 = bpf_bypass_spec_v1();
>> env->bypass_spec_v4 = bpf_bypass_spec_v4();
>> env->bpf_capable = bpf_capable();
>> + env->rcu_tag_supported =
>> + btf_find_by_name_kind(btf_vmlinux, "rcu", BTF_KIND_TYPE_TAG) > 0;
>
> It needs btf_vmlinux != NULL check as well,
> since we error earlier only on IS_ERR(btf_vmlinux).
> btf_vmlinux can be NULL at this point when CONFIG_DEBUG_INFO_BTF is not set.

I checked the code and it looks like btf_find_by_name_kind can handle
btf_vmlinux = NULL properly. Consider this is a unlikely case so
I did not add btf_vmlinux checking here.

u32 btf_nr_types(const struct btf *btf)
{
u32 total = 0;

while (btf) {
total += btf->nr_types;
btf = btf->base_btf;
}

return total;
}

s32 btf_find_by_name_kind(const struct btf *btf, const char *name, u8 kind)
{
const struct btf_type *t;
const char *tname;
u32 i, total;

total = btf_nr_types(btf);
for (i = 1; i < total; i++) {
t = btf_type_by_id(btf, i);
if (BTF_INFO_KIND(t->info) != kind)
continue;

tname = btf_name_by_offset(btf, t->name_off);
if (!strcmp(tname, name))
return i;
}

return -ENOENT;
}

If btf_vmlinux is NULL, then btf_nr_types(...) will return 0, so
btf_find_by_name_kind will return -ENOENT.

Certainly it does not hurt by adding explicit btf_vmlinux checking
before doing btf_find_by_name_kind(...)

>
> In the previous discussion I thought we agreed to
> fix convert_ctx_accesses() vs incorrect application of
> BPF_PROBE_MEM for PTR_TRUSTED pointers.
> But I didn't find it in this patch.
> So I'm fixing both issues and planning to apply after testing.

Thanks. I didn't do that since it is not really related to
bpf_rcu_read_lock(). I plan to do it as a followup. But
saw you just fixed the issue.