Re: [PATCH RFC bpf-next 3/7] libbpf: Support BPF_MAP_TYPE_STRUCT_OPS in bpf_gen__map_create()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] libbpf: double byte-swap of BPF_F_VTYPE_BTF_OBJ_FD in
  bpf_gen__map_create()
--

commit 504179d2d83634dc1591a224d0263ff3b4a9b9cf
Author: Siddharth Nayyar <sidnayyar@google.com>

libbpf: Support BPF_MAP_TYPE_STRUCT_OPS in bpf_gen__map_create()

This commit adds support for BPF_MAP_TYPE_STRUCT_OPS maps in
bpf_gen__map_create() when generating a BPF light skeleton, including
handling kernel module BTF object FDs.

> diff --git a/tools/lib/bpf/gen_loader.c b/tools/lib/bpf/gen_loader.c
> index 09713c39f71ec..6027a2762c352 100644
> --- a/tools/lib/bpf/gen_loader.c
> +++ b/tools/lib/bpf/gen_loader.c

[ ... ]

> @@ -543,6 +543,27 @@ void bpf_gen__map_create(struct bpf_gen *gen,
>  				stack_off(inner_map_fd));
>  		close_inner_map_fd = true;
>  		break;
> +	case BPF_MAP_TYPE_STRUCT_OPS:
> +		move_blob2blob(gen, attr_field(map_create_attr, value_size), 4,
> +			       value_size);
> +		emit2(gen, BPF_LD_IMM64_RAW_FULL(BPF_REG_1, BPF_PSEUDO_MAP_IDX_VALUE,
> +						 0, 0, 0, map_attr->btf_vmlinux_value_type_id));
> +		emit(gen, BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_1, 0));
> +		emit2(gen, BPF_LD_IMM64_RAW_FULL(BPF_REG_1, BPF_PSEUDO_MAP_IDX_VALUE,
> +						 0, 0, 0, map_create_attr));
> +		emit(gen, BPF_STX_MEM(BPF_W, BPF_REG_1, BPF_REG_0,
> +				      offsetof(union bpf_attr, btf_vmlinux_value_type_id)));
> +		emit(gen, BPF_ALU64_IMM(BPF_RSH, BPF_REG_0, 32));
> +		emit(gen, BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 4));
> +		emit(gen, BPF_STX_MEM(BPF_W, BPF_REG_1, BPF_REG_0,
> +				      offsetof(union bpf_attr, value_type_btf_obj_fd)));
> +		emit(gen, BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_1,
> +				      offsetof(union bpf_attr, map_flags)));
> +		emit(gen, BPF_ALU32_IMM(BPF_OR, BPF_REG_0,
> +					tgt_endian(BPF_F_VTYPE_BTF_OBJ_FD)));
> +		emit(gen, BPF_STX_MEM(BPF_W, BPF_REG_1, BPF_REG_0,
> +				      offsetof(union bpf_attr, map_flags)));
> +		break;

[Severity: Medium]
Does wrapping BPF_F_VTYPE_BTF_OBJ_FD in tgt_endian() double-swap the
instruction immediate when generating a cross-endian light skeleton?

Unlike data blob fields, emitted BPF instructions are built in host byte
order and later byte-swapped in bpf_gen__finish() via bpf_insn_bswap()
when gen->swapped_endian is set:

tools/lib/bpf/gen_loader.c:bpf_gen__finish() {
	...
		if (gen->swapped_endian) {
			struct bpf_insn *insn = (struct bpf_insn *)opts->insns;
			int insn_cnt = opts->insns_sz / sizeof(struct bpf_insn);

			for (i = 0; i < insn_cnt; i++)
				bpf_insn_bswap(insn++);
		}
	...
}

Pre-swapping the immediate here causes bpf_insn_bswap() to swap insn->imm
back to the wrong byte order, setting 0x00800000 instead of
BPF_F_VTYPE_BTF_OBJ_FD (0x00008000) in attr.map_flags on the target and
causing BPF_MAP_CREATE for kernel module struct_ops maps to fail with
-EINVAL.  Passing BPF_F_VTYPE_BTF_OBJ_FD directly without tgt_endian()
avoids the double swap.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106-lskel-struct-ops-v1-0-a4486ffc79cb@google.com?part=3