BUG: bpf: freplace of iterator global function requires bpf_iter_<type> name

[email protected]
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.
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.