BUG: bpf: freplace of iterator global function requires bpf_iter_<type> name
| Newsgroups | org.kernel.vger.bpf |
|---|---|
| Message-ID | <AyAj3bTqZ30lo9rs1fqBKO0Vk7CJqGfxcmHqHhmmEBpxlQxf0M4lfKOmSE_MbTVLhhxeFUx1PbyX6oZKq8Jh54h0pzjEFtXlRSqi4m6DcQA=@pm.me> |
Hi,
I believe there's an unintended behavior when an EXT prog attaches to a BPF global function used by an ITER prog. The global function name is required to match a registered iter type (e.g. `bpf_iter_task`, etc.) in order to pass the verifier. However the freplace target is just a BPF global function, so I don't think its name should be subject to the name validation in `bpf_iter_prog_supported()`.
For example, an ITER prog that uses:
`__noinline int foo(struct task_struct *t) { /* .. */ return 0; }`
an EXT prog freplace'ing `foo` is rejected unless `foo` is instead named something like `bpf_iter_task`, `bpf_iter_tcp`, etc.
It looks like this happens bc the verifier path for EXT prog sets `expected_attach_type` to the target prog's attach type:
if (tgt_prog && prog->type == BPF_PROG_TYPE_EXT) {
env->ops = bpf_verifier_ops[tgt_prog->type];
prog->expected_attach_type = tgt_prog->expected_attach_type;
}
in this case `BPF_TRACE_ITER`, which funnels into `bpf_iter_prog_supported()`:
} else if (prog->expected_attach_type == BPF_TRACE_ITER) {
if (!bpf_iter_prog_supported(prog))
return -EINVAL;
return 0;
}
`bpf_iter_prog_supported()` then does checks on the attach function as if the EXT prog were a full ITER prog.
I'm not entirely sure if the EXT prog should inherit all of the ITER-specific verifier checks, but I believe the global function name shouldn't need to match a `bpf_iter_<type>` name.
I also noticed another related issue involving `cache_btf_id()`. After using a valid `bpf_iter_<type>` name, subsequent changes to the global function name only appear to require the `BPF_ITER_FUNC_PREFIX` prefix. So for example, after using `bpf_iter_task` as name, changing name to `bpf_iter_foo` or even just `bpf_iter_` now passes verification when it previously did not.
I'm not completely sure I understand what `cache_btf_id()` does, but it appears to make passing/failing verifier dependent on whether a "valid" `bpf_iter_<type>` name was loaded and cached first.
If what I'm observing is a bug, I can put together a patch and selftest.
Thanks.