On Fri, 19 Jul 2024 09:12:58 +0000
Konstantin Ananyev <konstantin.anan...@huawei.com> wrote:
> bpf_convert_filter() uses standard approach with XOR-ing itself:
> xor r0, r0, r0
> to reset some register values.
> Unfortunately linux verifier seems way too strict here and
> doesn't allow access to register with undefined value.
> It generates error log like that for this op:
> Failed to verify program: Permission denied (13)
> LOG: func#0 @0
> 0: R1=ctx(id=0,off=0,imm=0) R10=fp0
> 0: (af) r0 ^= r0
> R0 !read_ok
> processed 1 insns (limit 1000000) max_states_per_insn 0 total_states 0
> peak_states 0 mark_read 0
>
> To overcome that, simply replace XOR with itself to explicit
> mov32 r0, #0x0
>
> Signed-off-by: Konstantin Ananyev <konstantin.anan...@huawei.com>
> ---
> lib/bpf/bpf_convert.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/lib/bpf/bpf_convert.c b/lib/bpf/bpf_convert.c
> index d7ff2b4325..eceaa19c76 100644
> --- a/lib/bpf/bpf_convert.c
> +++ b/lib/bpf/bpf_convert.c
> @@ -267,8 +267,11 @@ static int bpf_convert_filter(const struct bpf_insn
> *prog, size_t len,
> /* Classic BPF expects A and X to be reset first. These need
> * to be guaranteed to be the first two instructions.
> */
> - *new_insn++ = EBPF_ALU64_REG(BPF_XOR, BPF_REG_A, BPF_REG_A);
> - *new_insn++ = EBPF_ALU64_REG(BPF_XOR, BPF_REG_X, BPF_REG_X);
> + //*new_insn++ = EBPF_ALU64_REG(BPF_XOR, BPF_REG_A, BPF_REG_A);
> + //*new_insn++ = EBPF_ALU64_REG(BPF_XOR, BPF_REG_X, BPF_REG_X);
> +
> + *new_insn++ = BPF_MOV32_IMM(BPF_REG_A, 0);
> + *new_insn++ = BPF_MOV32_IMM(BPF_REG_X, 0);
>
> /* All programs must keep CTX in callee saved BPF_REG_CTX.
> * In eBPF case it's done by the compiler, here we need to
This was taken from how kernel converts cBPF and the prologue it generates.
Surprising that verifier gags?
see net/core/filter.c
/* Classic BPF related prologue emission. */
if (new_prog) {
/* Classic BPF expects A and X to be reset first. These need
* to be guaranteed to be the first two instructions.
*/
*new_insn++ = BPF_ALU32_REG(BPF_XOR, BPF_REG_A, BPF_REG_A);
*new_insn++ = BPF_ALU32_REG(BPF_XOR, BPF_REG_X, BPF_REG_X);
/* All programs must keep CTX in callee saved BPF_REG_CTX.
* In eBPF case it's done by the compiler, here we need to
* do this ourself. Initial CTX is present in BPF_REG_ARG1.
*/
*new_insn++ = BPF_MOV64_REG(BPF_REG_CTX, BPF_REG_ARG1);
if (*seen_ld_abs) {
/* For packet access in classic BPF, cache skb->data
* in callee-saved BPF R8 and skb->len - skb->data_len
* (headlen) in BPF R9. Since classic BPF is read-only
* on CTX, we only need to cache it once.
*/
*new_insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF(struct
sk_buff, data),
BPF_REG_D, BPF_REG_CTX,
offsetof(struct sk_buff,
data));
*new_insn++ = BPF_LDX_MEM(BPF_W, BPF_REG_H, BPF_REG_CTX,
offsetof(struct sk_buff,
len));
*new_insn++ = BPF_LDX_MEM(BPF_W, BPF_REG_TMP,
BPF_REG_CTX,
offsetof(struct sk_buff,
data_len));
*new_insn++ = BPF_ALU32_REG(BPF_SUB, BPF_REG_H,
BPF_REG_TMP);
}
