On 12/8/23 6:34 AM, Eduard Zingerman wrote:
On Thu, 2023-12-07 at 18:28 -0800, Yonghong Song wrote:
[...]
All context types are defined in include/linux/bpf_types.h.
The context type bpf_nf_ctx is missing.
convert_ctx_access() is not applied for bpf_nf_ctx. Searching through
kernel code shows that BPF programs access this structure directly
(net/netfilter/nf_bpf_link.c):
static unsigned int nf_hook_run_bpf(void *bpf_prog, struct sk_buff *skb,
const struct nf_hook_state *s)
{
const struct bpf_prog *prog = bpf_prog;
struct bpf_nf_ctx ctx = {
.state = s,
.skb = skb,
};
return bpf_prog_run(prog, &ctx);
}
I added __bpf_ctx only for types that are subject to convert_ctx_access()
transformation. On the other hand, applying it to each context type
should not hurt either. Which way would you prefer?
[...]
The error message should happen here:
check_mem_access
...
} else if (reg->type == PTR_TO_CTX) {
check_ptr_off_reg
__check_ptr_off_reg
if (!fixed_off_ok && reg->off) {
verbose(env, "dereference of modified %s ptr R%d off=%d disallowed\n",
reg_type_str(env, reg->type), regno, reg->off);
return -EACCES;
}
...
So the verification error message will be emitted earlier, before convert_ctx_access.
Could you double check?
How to add the same definitions in vmlinux.h is an open question,
and most likely requires bpftool modification:
- Hard code generation of __bpf_ctx based on type names?
- Mark context types with some special
__attribute__((btf_decl_tag("preserve_static_offset")))
and convert it to __attribute__((preserve_static_offset))?
The number of context types is limited, I would just go through
the first approach with hard coding the list of ctx types and
mark them with preserve_static_offset attribute in vmlinux.h.
Tbh, I'm with Alan here, generic approach seems a tad nicer.
Lets collect some more votes :)
