* [PATCH bpf v2 1/3] libbpf: Validate struct_ops member offsets before data access
2026-09-13 14:52 [PATCH bpf v2 0/3] libbpf: Validate struct_ops member offsets Mingpei CAO
@ 2026-09-13 14:52 ` Mingpei CAO
2026-09-13 15:49 ` bot+bpf-ci
2026-09-13 14:52 ` [PATCH bpf v2 2/3] bpftool: Keep struct_ops field offsets when skipping bitfields Mingpei CAO
2026-09-13 14:52 ` [PATCH bpf v2 3/3] selftests/bpf: Cover struct_ops bitfield and offset validation Mingpei CAO
2 siblings, 1 reply; 9+ messages in thread
From: Mingpei CAO @ 2026-09-13 14:52 UTC (permalink / raw)
To: bpf; +Cc: andrii, eddyz87, ameryhung, qmo, Mingpei CAO
A local struct_ops type can contain a bitfield absent from the
corresponding kernel BTF type. libbpf checks whether data for an absent
local member is zero.
For a bitfield, member->offset contains the bit position and field width.
bpf_map__init_kern_struct_ops() used the full value to create a data
pointer before checking the member range. libbpf_is_mem_zeroed() could
then read outside the local struct_ops data.
bpf_map__init_kern_struct_ops() also failed to check offsets for
non-bitfield members.
Reject local and kernel bitfields before reading member data. Check every
local and kernel member range before creating a data pointer.
Fixes: c911fc61a7ce ("libbpf: Skip zeroed or null fields if not found in the kernel type.")
Assisted-by: LLM
Signed-off-by: Mingpei CAO <caomingpei@gmail.com>
---
tools/lib/bpf/libbpf.c | 80 +++++++++++++++++++++++++++++++++++-------
1 file changed, 68 insertions(+), 12 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index b749c01742ee0..2791b48585939 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -1175,6 +1175,7 @@ static int bpf_map__init_kern_struct_ops(struct bpf_map *map)
const struct btf_member *member, *kern_member, *kern_data_member;
const struct btf_type *type, *kern_type, *kern_vtype;
__u32 i, kern_type_id, kern_vtype_id, kern_data_off;
+ __u32 kern_data_bit_off, kern_data_member_idx;
struct bpf_object *obj = map->obj;
const struct btf *btf = obj->btf;
struct bpf_struct_ops *st_ops;
@@ -1195,6 +1196,25 @@ static int bpf_map__init_kern_struct_ops(struct bpf_map *map)
return err;
kern_btf = mod_btf ? mod_btf->btf : obj->btf_vmlinux;
+ kern_data_member_idx = kern_data_member - btf_members(kern_vtype);
+ if (btf_member_bitfield_size(kern_vtype, kern_data_member_idx)) {
+ pr_warn("struct_ops init_kern %s: kernel data member is a bitfield\n",
+ map->name);
+ return -ENOTSUP;
+ }
+ kern_data_bit_off = btf_member_bit_offset(kern_vtype, kern_data_member_idx);
+ if (kern_data_bit_off % 8) {
+ pr_warn("struct_ops init_kern %s: kernel data member has a non-byte-aligned offset\n",
+ map->name);
+ return -EINVAL;
+ }
+ kern_data_off = kern_data_bit_off / 8;
+ if (kern_data_off > kern_vtype->size ||
+ kern_type->size > kern_vtype->size - kern_data_off) {
+ pr_warn("struct_ops init_kern %s: kernel data member is outside the %u-byte kernel value type\n",
+ map->name, kern_vtype->size);
+ return -EINVAL;
+ }
pr_debug("struct_ops init_kern %s: type_id:%u kern_type_id:%u kern_vtype_id:%u\n",
map->name, st_ops->type_id, kern_type_id, kern_vtype_id);
@@ -1208,29 +1228,50 @@ static int bpf_map__init_kern_struct_ops(struct bpf_map *map)
return -ENOMEM;
data = st_ops->data;
- kern_data_off = kern_data_member->offset / 8;
kern_data = st_ops->kern_vdata + kern_data_off;
member = btf_members(type);
for (i = 0; i < btf_vlen(type); i++, member++) {
const struct btf_type *mtype, *kern_mtype;
- __u32 mtype_id, kern_mtype_id;
+ __u32 bit_offset, mtype_id, kern_mtype_id;
void *mdata, *kern_mdata;
struct bpf_program *prog;
__s64 msize, kern_msize;
+ __u64 maccess_size;
__u32 moff, kern_moff;
__u32 kern_member_idx;
const char *mname;
mname = btf__name_by_offset(btf, member->name_off);
- moff = member->offset / 8;
- mdata = data + moff;
+ if (btf_member_bitfield_size(type, i)) {
+ pr_warn("struct_ops init_kern %s: local bitfield %s is not supported\n",
+ map->name, mname);
+ return -ENOTSUP;
+ }
+
msize = btf__resolve_size(btf, member->type);
if (msize < 0) {
pr_warn("struct_ops init_kern %s: failed to resolve the size of member %s\n",
map->name, mname);
return msize;
}
+ mtype = skip_mods_and_typedefs(btf, member->type, &mtype_id);
+ maccess_size = msize;
+ if (btf_is_ptr(mtype) && maccess_size < sizeof(prog))
+ maccess_size = sizeof(prog);
+ bit_offset = btf_member_bit_offset(type, i);
+ if (bit_offset % 8) {
+ pr_warn("struct_ops init_kern %s: member %s has a non-byte-aligned offset\n",
+ map->name, mname);
+ return -EINVAL;
+ }
+ moff = bit_offset / 8;
+ if (moff > type->size || maccess_size > type->size - moff) {
+ pr_warn("struct_ops init_kern %s: member %s is outside the %u-byte local struct_ops type\n",
+ map->name, mname, type->size);
+ return -EINVAL;
+ }
+ mdata = data + moff;
kern_member = find_member_by_name(kern_btf, kern_type, mname);
if (!kern_member) {
@@ -1259,17 +1300,33 @@ static int bpf_map__init_kern_struct_ops(struct bpf_map *map)
}
kern_member_idx = kern_member - btf_members(kern_type);
- if (btf_member_bitfield_size(type, i) ||
- btf_member_bitfield_size(kern_type, kern_member_idx)) {
- pr_warn("struct_ops init_kern %s: bitfield %s is not supported\n",
+ if (btf_member_bitfield_size(kern_type, kern_member_idx)) {
+ pr_warn("struct_ops init_kern %s: kernel bitfield %s is not supported\n",
map->name, mname);
return -ENOTSUP;
}
- kern_moff = kern_member->offset / 8;
+ kern_msize = btf__resolve_size(kern_btf, kern_member->type);
+ if (kern_msize < 0) {
+ pr_warn("struct_ops init_kern %s: failed to resolve the kernel size of member %s\n",
+ map->name, mname);
+ return kern_msize;
+ }
+ bit_offset = btf_member_bit_offset(kern_type, kern_member_idx);
+ if (bit_offset % 8) {
+ pr_warn("struct_ops init_kern %s: kernel member %s has a non-byte-aligned offset\n",
+ map->name, mname);
+ return -EINVAL;
+ }
+ kern_moff = bit_offset / 8;
+ if (kern_moff > kern_type->size ||
+ (__u64)kern_msize > kern_type->size - kern_moff) {
+ pr_warn("struct_ops init_kern %s: kernel member %s is outside the %u-byte kernel struct_ops type\n",
+ map->name, mname, kern_type->size);
+ return -EINVAL;
+ }
kern_mdata = kern_data + kern_moff;
- mtype = skip_mods_and_typedefs(btf, member->type, &mtype_id);
kern_mtype = skip_mods_and_typedefs(kern_btf, kern_member->type,
&kern_mtype_id);
if (BTF_INFO_KIND(mtype->info) !=
@@ -1281,7 +1338,7 @@ static int bpf_map__init_kern_struct_ops(struct bpf_map *map)
}
if (btf_is_ptr(mtype)) {
- prog = *(void **)mdata;
+ memcpy(&prog, mdata, sizeof(prog));
/* just like for !kern_member case above, reset declaratively
* set (at compile time) program's autload to false,
* if user replaced it with another program or NULL
@@ -1352,8 +1409,7 @@ static int bpf_map__init_kern_struct_ops(struct bpf_map *map)
continue;
}
- kern_msize = btf__resolve_size(kern_btf, kern_mtype_id);
- if (kern_msize < 0 || msize != kern_msize) {
+ if (msize != kern_msize) {
pr_warn("struct_ops init_kern %s: Error in size of member %s: %zd != %zd(kernel)\n",
map->name, mname, (ssize_t)msize,
(ssize_t)kern_msize);
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH bpf v2 1/3] libbpf: Validate struct_ops member offsets before data access
2026-09-13 14:52 ` [PATCH bpf v2 1/3] libbpf: Validate struct_ops member offsets before data access Mingpei CAO
@ 2026-09-13 15:49 ` bot+bpf-ci
0 siblings, 0 replies; 9+ messages in thread
From: bot+bpf-ci @ 2026-09-13 15:49 UTC (permalink / raw)
To: caomingpei, bpf
Cc: andrii, eddyz87, ameryhung, qmo, caomingpei, ast, andrii, daniel,
martin.lau, eddyz87, yonghong.song, mason, ihor.solodrai
[-- Attachment #1: Type: text/plain, Size: 4008 bytes --]
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index b749c01742ee..2791b4858593 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
[ ... ]
> @@ -1208,29 +1228,50 @@ static int bpf_map__init_kern_struct_ops(struct bpf_map *map)
> return -ENOMEM;
>
> data = st_ops->data;
> - kern_data_off = kern_data_member->offset / 8;
> kern_data = st_ops->kern_vdata + kern_data_off;
>
> member = btf_members(type);
> for (i = 0; i < btf_vlen(type); i++, member++) {
> const struct btf_type *mtype, *kern_mtype;
> - __u32 mtype_id, kern_mtype_id;
> + __u32 bit_offset, mtype_id, kern_mtype_id;
> void *mdata, *kern_mdata;
> struct bpf_program *prog;
> __s64 msize, kern_msize;
> + __u64 maccess_size;
> __u32 moff, kern_moff;
> __u32 kern_member_idx;
> const char *mname;
>
> mname = btf__name_by_offset(btf, member->name_off);
> - moff = member->offset / 8;
> - mdata = data + moff;
> + if (btf_member_bitfield_size(type, i)) {
> + pr_warn("struct_ops init_kern %s: local bitfield %s is not supported\n",
> + map->name, mname);
> + return -ENOTSUP;
> + }
> +
> msize = btf__resolve_size(btf, member->type);
> if (msize < 0) {
> pr_warn("struct_ops init_kern %s: failed to resolve the size of member %s\n",
> map->name, mname);
> return msize;
> }
> + mtype = skip_mods_and_typedefs(btf, member->type, &mtype_id);
> + maccess_size = msize;
> + if (btf_is_ptr(mtype) && maccess_size < sizeof(prog))
> + maccess_size = sizeof(prog);
> + bit_offset = btf_member_bit_offset(type, i);
> + if (bit_offset % 8) {
> + pr_warn("struct_ops init_kern %s: member %s has a non-byte-aligned offset\n",
> + map->name, mname);
> + return -EINVAL;
> + }
> + moff = bit_offset / 8;
> + if (moff > type->size || maccess_size > type->size - moff) {
> + pr_warn("struct_ops init_kern %s: member %s is outside the %u-byte local struct_ops type\n",
> + map->name, mname, type->size);
> + return -EINVAL;
> + }
> + mdata = data + moff;
This isn't a bug, but could the commit message mention the pointer load
change and the maccess_size widening? The changelog describes rejecting
bitfields and checking ranges, but doesn't explain why the punned load
prog = *(void **)mdata becomes memcpy(&prog, mdata, sizeof(prog)) or why
maccess_size is widened to sizeof(prog) for pointer members. Both look
deliberate - once an arbitrary moff is accepted, mdata need not be
pointer-aligned, and the read is sizeof(struct bpf_program *) wide rather
than msize wide - but a reader has to reconstruct that rationale from the
code.
[ ... ]
> @@ -1281,7 +1338,7 @@ static int bpf_map__init_kern_struct_ops(struct bpf_map *map)
> }
>
> if (btf_is_ptr(mtype)) {
> - prog = *(void **)mdata;
> + memcpy(&prog, mdata, sizeof(prog));
> /* just like for !kern_member case above, reset declaratively
> * set (at compile time) program's autload to false,
> * if user replaced it with another program or NULL
[ ... ]
> @@ -1352,8 +1409,7 @@ static int bpf_map__init_kern_struct_ops(struct bpf_map *map)
> continue;
> }
>
> - kern_msize = btf__resolve_size(kern_btf, kern_mtype_id);
> - if (kern_msize < 0 || msize != kern_msize) {
> + if (msize != kern_msize) {
> pr_warn("struct_ops init_kern %s: Error in size of member %s: %zd != %zd(kernel)\n",
> map->name, mname, (ssize_t)msize,
> (ssize_t)kern_msize);
This isn't a bug, but with the btf__resolve_size(kern_btf, kern_mtype_id)
call gone, is kern_mtype_id still read anywhere? The patch writes it via
skip_mods_and_typedefs() calls but the last consumer is removed here.
skip_mods_and_typedefs() accepts NULL for res_id, so both call sites could
pass NULL and the local could be dropped.
---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/34765254213
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH bpf v2 2/3] bpftool: Keep struct_ops field offsets when skipping bitfields
2026-09-13 14:52 [PATCH bpf v2 0/3] libbpf: Validate struct_ops member offsets Mingpei CAO
2026-09-13 14:52 ` [PATCH bpf v2 1/3] libbpf: Validate struct_ops member offsets before data access Mingpei CAO
@ 2026-09-13 14:52 ` Mingpei CAO
2026-09-13 15:02 ` sashiko-bot
2026-09-13 15:49 ` bot+bpf-ci
2026-09-13 14:52 ` [PATCH bpf v2 3/3] selftests/bpf: Cover struct_ops bitfield and offset validation Mingpei CAO
2 siblings, 2 replies; 9+ messages in thread
From: Mingpei CAO @ 2026-09-13 14:52 UTC (permalink / raw)
To: bpf; +Cc: andrii, eddyz87, ameryhung, qmo, Mingpei CAO
For a bitfield, member->offset contains the bit position and field width.
The struct_ops skeleton generator used the full value as the bit position.
The generated C structure could contain excessive padding and place later
fields outside the map value.
Represent unsupported bitfield storage as padding in the generated
skeleton. This preserves the struct_ops map and keeps supported fields at
their declared BTF offsets. Validate every member range and alignment
before generating the C structure.
Fixes: a7b0fa352eaf ("bpftool: Generated shadow variables for struct_ops maps.")
Assisted-by: LLM
Signed-off-by: Mingpei CAO <caomingpei@gmail.com>
---
tools/bpf/bpftool/gen.c | 147 ++++++++++++++++++++++++++--------------
1 file changed, 97 insertions(+), 50 deletions(-)
diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
index a50540ef6521c..d93073d5e7e32 100644
--- a/tools/bpf/bpftool/gen.c
+++ b/tools/bpf/bpftool/gen.c
@@ -1044,62 +1044,79 @@ codegen_progs_skeleton(struct bpf_object *obj, size_t prog_cnt, bool populate_li
}
static int walk_st_ops_shadow_vars(struct btf *btf, const char *ident,
- const struct btf_type *map_type, __u32 map_type_id)
+ const struct btf_type *map_type)
{
+ enum st_ops_shadow_kind {
+ ST_OPS_SHADOW_SCALAR,
+ ST_OPS_SHADOW_FUNC_PTR,
+ ST_OPS_SHADOW_OPAQUE,
+ };
LIBBPF_OPTS(btf_dump_emit_type_decl_opts, opts, .indent_level = 3);
const struct btf_type *member_type;
- __u32 offset, next_offset = 0;
+ __u64 bitfield_end = 0, map_bits;
+ __u32 bit_offset, bitfield_size;
+ __u32 map_size, offset, next_offset = 0;
const struct btf_member *m;
struct btf_dump *d = NULL;
const char *member_name;
__u32 member_type_id;
- int i, err = 0, n;
- int size;
+ int align, err = 0, i, max_align = 1, n, size;
+ enum st_ops_shadow_kind shadow_kind;
d = btf_dump__new(btf, codegen_btf_dump_printf, NULL, NULL);
if (!d)
return -errno;
+ map_size = map_type->size;
+ map_bits = (__u64)map_size * 8;
n = btf_vlen(map_type);
for (i = 0, m = btf_members(map_type); i < n; i++, m++) {
- member_type = skip_mods_and_typedefs(btf, m->type, &member_type_id);
member_name = btf__name_by_offset(btf, m->name_off);
+ bit_offset = btf_member_bit_offset(map_type, i);
+ bitfield_size = btf_member_bitfield_size(map_type, i);
+ if (bitfield_size) {
+ if ((__u64)bit_offset < bitfield_end ||
+ (__u64)bit_offset < (__u64)next_offset * 8 ||
+ bit_offset > map_bits ||
+ bitfield_size > map_bits - bit_offset) {
+ p_err("Invalid bitfield layout for struct_ops member %s",
+ member_name);
+ err = -EINVAL;
+ goto out;
+ }
+ bitfield_end = (__u64)bit_offset + bitfield_size;
+ continue;
+ }
- offset = m->offset / 8;
- if (next_offset < offset)
- printf("\t\t\tchar __padding_%d[%u];\n", i, offset - next_offset);
+ if (bit_offset % 8 || bit_offset < bitfield_end) {
+ p_err("Invalid offset for struct_ops member %s", member_name);
+ err = -EINVAL;
+ goto out;
+ }
+
+ offset = bit_offset / 8;
+ if (offset < next_offset || offset > map_size) {
+ p_err("Invalid offset for struct_ops member %s", member_name);
+ err = -EINVAL;
+ goto out;
+ }
+ member_type = skip_mods_and_typedefs(btf, m->type, &member_type_id);
switch (btf_kind(member_type)) {
case BTF_KIND_INT:
case BTF_KIND_FLOAT:
case BTF_KIND_ENUM:
case BTF_KIND_ENUM64:
- /* scalar type */
- printf("\t\t\t");
- opts.field_name = member_name;
- err = btf_dump__emit_type_decl(d, member_type_id, &opts);
- if (err) {
- p_err("Failed to emit type declaration for %s: %d", member_name, err);
- goto out;
- }
- printf(";\n");
-
+ shadow_kind = ST_OPS_SHADOW_SCALAR;
size = btf__resolve_size(btf, member_type_id);
- if (size < 0) {
- p_err("Failed to resolve size of %s: %d\n", member_name, size);
- err = size;
- goto out;
- }
-
- next_offset = offset + size;
+ align = btf__align_of(btf, member_type_id);
break;
case BTF_KIND_PTR:
if (resolve_func_ptr(btf, m->type, NULL)) {
- /* Function pointer */
- printf("\t\t\tstruct bpf_program *%s;\n", member_name);
-
- next_offset = offset + sizeof(void *);
+ shadow_kind = ST_OPS_SHADOW_FUNC_PTR;
+ size = sizeof(void *);
+ align = __alignof__(void *);
break;
}
/* All pointer types are unsupported except for
@@ -1108,34 +1125,64 @@ static int walk_st_ops_shadow_vars(struct btf *btf, const char *ident,
fallthrough;
default:
- /* Unsupported types
- *
- * Types other than scalar types and function
- * pointers are currently not supported in order to
- * prevent conflicts in the generated code caused
- * by multiple definitions. For instance, if the
- * struct type FOO is used in a struct_ops map,
- * bpftool has to generate definitions for FOO,
- * which may result in conflicts if FOO is defined
- * in different skeleton files.
- */
+ shadow_kind = ST_OPS_SHADOW_OPAQUE;
size = btf__resolve_size(btf, member_type_id);
- if (size < 0) {
- p_err("Failed to resolve size of %s: %d\n", member_name, size);
- err = size;
+ align = 1;
+ break;
+ }
+
+ if (size < 0 || align < 0) {
+ err = size < 0 ? size : align;
+ p_err("Failed to resolve layout of %s: %d", member_name, err);
+ goto out;
+ }
+ if ((__u32)size > map_size - offset ||
+ (align > 1 && offset % align)) {
+ p_err("Invalid layout for struct_ops member %s", member_name);
+ err = -EINVAL;
+ goto out;
+ }
+
+ if (next_offset < offset)
+ printf("\t\t\tchar __padding_%d[%u];\n", i, offset - next_offset);
+
+ switch (shadow_kind) {
+ case ST_OPS_SHADOW_SCALAR:
+ printf("\t\t\t");
+ opts.field_name = member_name;
+ err = btf_dump__emit_type_decl(d, member_type_id, &opts);
+ if (err) {
+ p_err("Failed to emit type declaration for %s: %d",
+ member_name, err);
goto out;
}
+ printf(";\n");
+ break;
+ case ST_OPS_SHADOW_FUNC_PTR:
+ printf("\t\t\tstruct bpf_program *%s;\n", member_name);
+ break;
+ case ST_OPS_SHADOW_OPAQUE:
+ /* Avoid emitting named types that might conflict with
+ * definitions from other skeletons.
+ */
printf("\t\t\tchar __unsupported_%d[%d];\n", i, size);
-
- next_offset = offset + size;
break;
+ default:
+ __builtin_unreachable();
}
+
+ next_offset = offset + size;
+ max_align = max(max_align, align);
+ bitfield_end = 0;
}
- /* Cannot fail since it must be a struct type */
- size = btf__resolve_size(btf, map_type_id);
- if (next_offset < (__u32)size)
- printf("\t\t\tchar __padding_end[%u];\n", size - next_offset);
+ if (map_size % max_align) {
+ p_err("Invalid size for struct_ops type %s", ident);
+ err = -EINVAL;
+ goto out;
+ }
+ if (next_offset < map_size)
+ printf("\t\t\tchar __padding_end[%u];\n", map_size - next_offset);
out:
btf_dump__free(d);
@@ -1177,7 +1224,7 @@ static int gen_st_ops_shadow_type(const char *obj_name, struct btf *btf, const c
printf("\t\tstruct %s__%s__%s {\n", obj_name, ident, type_name);
- err = walk_st_ops_shadow_vars(btf, ident, map_type, map_type_id);
+ err = walk_st_ops_shadow_vars(btf, ident, map_type);
if (err)
return err;
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH bpf v2 2/3] bpftool: Keep struct_ops field offsets when skipping bitfields
2026-09-13 14:52 ` [PATCH bpf v2 2/3] bpftool: Keep struct_ops field offsets when skipping bitfields Mingpei CAO
@ 2026-09-13 15:02 ` sashiko-bot
2026-09-13 15:49 ` bot+bpf-ci
1 sibling, 0 replies; 9+ messages in thread
From: sashiko-bot @ 2026-09-13 15:02 UTC (permalink / raw)
To: Mingpei CAO; +Cc: bpf
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Multi-line comment does not follow the preferred BPF subsystem comment style.
--
commit 76b7f5ec797f551d88f419af56f14122daae2704
Author: Mingpei CAO <caomingpei@gmail.com>
bpftool: Keep struct_ops field offsets when skipping bitfields
This commit modifies the struct_ops skeleton generator to correctly handle
bitfields by representing unsupported bitfield storage as padding. This
prevents excessive padding generation and keeps supported fields at their
declared BTF offsets.
> diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
> --- a/tools/bpf/bpftool/gen.c
> +++ b/tools/bpf/bpftool/gen.c
[ ... ]
> @@ -1108,34 +1125,64 @@ static int walk_st_ops_shadow_vars(struct btf *btf, const char *ident,
[ ... ]
> + case ST_OPS_SHADOW_OPAQUE:
> + /* Avoid emitting named types that might conflict with
> + * definitions from other skeletons.
> + */
[Severity: Low]
This isn't a bug, but does this multi-line comment in
walk_st_ops_shadow_vars() adhere to the BPF subsystem guidelines? The
preferred style for multi-line comments under BPF-related paths requires
the opening /* to be on its own line, rather than on the same line as the
first line of text.
> printf("\t\t\tchar __unsupported_%d[%d];\n", i, size);
> break;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260913145233.1446643-1-caomingpei@gmail.com?part=2
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH bpf v2 2/3] bpftool: Keep struct_ops field offsets when skipping bitfields
2026-09-13 14:52 ` [PATCH bpf v2 2/3] bpftool: Keep struct_ops field offsets when skipping bitfields Mingpei CAO
2026-09-13 15:02 ` sashiko-bot
@ 2026-09-13 15:49 ` bot+bpf-ci
1 sibling, 0 replies; 9+ messages in thread
From: bot+bpf-ci @ 2026-09-13 15:49 UTC (permalink / raw)
To: caomingpei, bpf
Cc: andrii, eddyz87, ameryhung, qmo, caomingpei, ast, andrii, daniel,
martin.lau, eddyz87, yonghong.song, mason, ihor.solodrai
[-- Attachment #1: Type: text/plain, Size: 8051 bytes --]
> diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
> index a50540ef6521..d93073d5e7e3 100644
> --- a/tools/bpf/bpftool/gen.c
> +++ b/tools/bpf/bpftool/gen.c
[ ... ]
> @@ -1044,62 +1044,79 @@ codegen_progs_skeleton(struct bpf_object *obj, size_t prog_cnt, bool populate_li
> }
>
> static int walk_st_ops_shadow_vars(struct btf *btf, const char *ident,
> - const struct btf_type *map_type, __u32 map_type_id)
> + const struct btf_type *map_type)
> {
> + enum st_ops_shadow_kind {
> + ST_OPS_SHADOW_SCALAR,
> + ST_OPS_SHADOW_FUNC_PTR,
> + ST_OPS_SHADOW_OPAQUE,
> + };
> LIBBPF_OPTS(btf_dump_emit_type_decl_opts, opts, .indent_level = 3);
> const struct btf_type *member_type;
> - __u32 offset, next_offset = 0;
> + __u64 bitfield_end = 0, map_bits;
> + __u32 bit_offset, bitfield_size;
> + __u32 map_size, offset, next_offset = 0;
> const struct btf_member *m;
> struct btf_dump *d = NULL;
> const char *member_name;
> __u32 member_type_id;
> - int i, err = 0, n;
> - int size;
> + int align, err = 0, i, max_align = 1, n, size;
> + enum st_ops_shadow_kind shadow_kind;
>
> d = btf_dump__new(btf, codegen_btf_dump_printf, NULL, NULL);
> if (!d)
> return -errno;
>
> + map_size = map_type->size;
> + map_bits = (__u64)map_size * 8;
> n = btf_vlen(map_type);
> for (i = 0, m = btf_members(map_type); i < n; i++, m++) {
> - member_type = skip_mods_and_typedefs(btf, m->type, &member_type_id);
> member_name = btf__name_by_offset(btf, m->name_off);
> + bit_offset = btf_member_bit_offset(map_type, i);
> + bitfield_size = btf_member_bitfield_size(map_type, i);
> + if (bitfield_size) {
> + if ((__u64)bit_offset < bitfield_end ||
> + (__u64)bit_offset < (__u64)next_offset * 8 ||
> + bit_offset > map_bits ||
> + bitfield_size > map_bits - bit_offset) {
> + p_err("Invalid bitfield layout for struct_ops member %s",
> + member_name);
> + err = -EINVAL;
> + goto out;
> + }
> + bitfield_end = (__u64)bit_offset + bitfield_size;
> + continue;
> + }
>
> - offset = m->offset / 8;
> - if (next_offset < offset)
> - printf("\t\t\tchar __padding_%d[%u];\n", i, offset - next_offset);
> + if (bit_offset % 8 || bit_offset < bitfield_end) {
> + p_err("Invalid offset for struct_ops member %s", member_name);
> + err = -EINVAL;
> + goto out;
> + }
> +
> + offset = bit_offset / 8;
> + if (offset < next_offset || offset > map_size) {
> + p_err("Invalid offset for struct_ops member %s", member_name);
> + err = -EINVAL;
> + goto out;
> + }
This isn't a bug, but the two error blocks above print the same message
and take the same error path.
Would folding the four conditions into one guard (or differentiating the
messages) make it clearer which check failed during debugging?
>
> member_type = skip_mods_and_typedefs(btf, m->type, &member_type_id);
> switch (btf_kind(member_type)) {
> case BTF_KIND_INT:
> case BTF_KIND_FLOAT:
> case BTF_KIND_ENUM:
> case BTF_KIND_ENUM64:
> - /* scalar type */
> - printf("\t\t\t");
> - opts.field_name = member_name;
> - err = btf_dump__emit_type_decl(d, member_type_id, &opts);
> - if (err) {
> - p_err("Failed to emit type declaration for %s: %d", member_name, err);
> - goto out;
> - }
> - printf(";\n");
> -
> + shadow_kind = ST_OPS_SHADOW_SCALAR;
> size = btf__resolve_size(btf, member_type_id);
> - if (size < 0) {
> - p_err("Failed to resolve size of %s: %d\n", member_name, size);
> - err = size;
> - goto out;
> - }
> -
> - next_offset = offset + size;
> + align = btf__align_of(btf, member_type_id);
> break;
>
> case BTF_KIND_PTR:
> if (resolve_func_ptr(btf, m->type, NULL)) {
> - /* Function pointer */
> - printf("\t\t\tstruct bpf_program *%s;\n", member_name);
> -
> - next_offset = offset + sizeof(void *);
> + shadow_kind = ST_OPS_SHADOW_FUNC_PTR;
> + size = sizeof(void *);
> + align = __alignof__(void *);
> break;
> }
> /* All pointer types are unsupported except for
> @@ -1108,34 +1125,64 @@ static int walk_st_ops_shadow_vars(struct btf *btf, const char *ident,
> fallthrough;
>
> default:
> - /* Unsupported types
> - *
> - * Types other than scalar types and function
> - * pointers are currently not supported in order to
> - * prevent conflicts in the generated code caused
> - * by multiple definitions. For instance, if the
> - * struct type FOO is used in a struct_ops map,
> - * bpftool has to generate definitions for FOO,
> - * which may result in conflicts if FOO is defined
> - * in different skeleton files.
> - */
> + shadow_kind = ST_OPS_SHADOW_OPAQUE;
> size = btf__resolve_size(btf, member_type_id);
> - if (size < 0) {
> - p_err("Failed to resolve size of %s: %d\n", member_name, size);
> - err = size;
> + align = 1;
> + break;
> + }
> +
> + if (size < 0 || align < 0) {
> + err = size < 0 ? size : align;
> + p_err("Failed to resolve layout of %s: %d", member_name, err);
> + goto out;
> + }
> + if ((__u32)size > map_size - offset ||
> + (align > 1 && offset % align)) {
> + p_err("Invalid layout for struct_ops member %s", member_name);
> + err = -EINVAL;
> + goto out;
> + }
> +
> + if (next_offset < offset)
> + printf("\t\t\tchar __padding_%d[%u];\n", i, offset - next_offset);
> +
> + switch (shadow_kind) {
> + case ST_OPS_SHADOW_SCALAR:
> + printf("\t\t\t");
> + opts.field_name = member_name;
> + err = btf_dump__emit_type_decl(d, member_type_id, &opts);
> + if (err) {
> + p_err("Failed to emit type declaration for %s: %d",
> + member_name, err);
> goto out;
> }
> + printf(";\n");
> + break;
> + case ST_OPS_SHADOW_FUNC_PTR:
> + printf("\t\t\tstruct bpf_program *%s;\n", member_name);
> + break;
> + case ST_OPS_SHADOW_OPAQUE:
> + /* Avoid emitting named types that might conflict with
> + * definitions from other skeletons.
> + */
> printf("\t\t\tchar __unsupported_%d[%d];\n", i, size);
> -
> - next_offset = offset + size;
> break;
> + default:
> + __builtin_unreachable();
^^^^
> }
This isn't a bug, but the emission switch covers all three values of the
st_ops_shadow_kind enum, and shadow_kind is assigned on every path of
the classifying switch above.
Since the default arm can't be reached and this is the only
__builtin_unreachable() in tools/bpf/, would dropping it (or using
unreachable() from linux/compiler.h) keep -Wswitch coverage intact?
[ ... ]
> @@ -1177,7 +1224,7 @@ static int gen_st_ops_shadow_type(const char *obj_name, struct btf *btf, const c
>
> printf("\t\tstruct %s__%s__%s {\n", obj_name, ident, type_name);
>
> - err = walk_st_ops_shadow_vars(btf, ident, map_type, map_type_id);
> + err = walk_st_ops_shadow_vars(btf, ident, map_type);
> if (err)
> return err;
>
The patch adds a new member-type mapping: bitfield storage becomes part
of __padding_* instead of __unsupported_*. The comment before
gen_st_ops_shadow_type() explains the member-type mappings:
In gen.c:gen_st_ops_shadow_type():
/*
* A shadow type includes not all members, only members of some types.
* They are scalar types and function pointers. The function pointers are
* translated to the pointer of the struct bpf_program. The scalar types
* are translated to the original type without any modifiers.
*
* Unsupported types will be translated to a char array to occupy the same
* space as the original field, being renamed as __unsupported_*. The user
* should treat these fields as opaque data.
*/
This isn't a bug, but now that bitfield members disappear into __padding_*
instead of appearing as __unsupported_*, would adding a line about that
keep the documented member mapping complete?
---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/34765254213
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH bpf v2 3/3] selftests/bpf: Cover struct_ops bitfield and offset validation
2026-09-13 14:52 [PATCH bpf v2 0/3] libbpf: Validate struct_ops member offsets Mingpei CAO
2026-09-13 14:52 ` [PATCH bpf v2 1/3] libbpf: Validate struct_ops member offsets before data access Mingpei CAO
2026-09-13 14:52 ` [PATCH bpf v2 2/3] bpftool: Keep struct_ops field offsets when skipping bitfields Mingpei CAO
@ 2026-09-13 14:52 ` Mingpei CAO
2026-09-13 15:03 ` sashiko-bot
2026-09-13 15:49 ` bot+bpf-ci
2 siblings, 2 replies; 9+ messages in thread
From: Mingpei CAO @ 2026-09-13 14:52 UTC (permalink / raw)
To: bpf; +Cc: andrii, eddyz87, ameryhung, qmo, Mingpei CAO
Add two adjacent bitfield members between two non-bitfield members. Check
the size of the generated C structure and the offsets of both non-bitfield
members. Require libbpf to reject the first bitfield member and print the
expected message.
Move a non-bitfield member to the first byte after a four-byte local
struct_ops type. Require an -EINVAL result without a memory error.
Assisted-by: LLM
Signed-off-by: Mingpei CAO <caomingpei@gmail.com>
---
.../bpf/prog_tests/test_struct_ops_module.c | 91 ++++++++++++++++++-
.../selftests/bpf/progs/struct_ops_module.c | 17 ++++
2 files changed, 107 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/test_struct_ops_module.c b/tools/testing/selftests/bpf/prog_tests/test_struct_ops_module.c
index 75a0dea511b3f..41029ebb22f01 100644
--- a/tools/testing/selftests/bpf/prog_tests/test_struct_ops_module.c
+++ b/tools/testing/selftests/bpf/prog_tests/test_struct_ops_module.c
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: GPL-2.0
/* Copyright (c) 2024 Meta Platforms, Inc. and affiliates. */
#include <test_progs.h>
+#include <bpf/btf.h>
#include <time.h>
#include <sys/epoll.h>
@@ -150,6 +151,91 @@ static void test_struct_ops_not_zeroed(void)
struct_ops_module__destroy(skel);
}
+static void test_struct_ops_local_bitfield(void)
+{
+ struct struct_ops_module *skel;
+ char *log = NULL;
+ int err;
+
+ skel = struct_ops_module__open();
+ if (!ASSERT_OK_PTR(skel, "struct_ops_module_open_local_bitfield"))
+ return;
+
+ if (!ASSERT_EQ(offsetof(typeof(*skel->struct_ops.testmod_local_bitfield), data),
+ 0, "local_bitfield_data_offset") ||
+ !ASSERT_EQ(offsetof(typeof(*skel->struct_ops.testmod_local_bitfield), onebyte),
+ 8, "local_bitfield_onebyte_offset"))
+ goto cleanup;
+ if (!ASSERT_EQ(sizeof(*skel->struct_ops.testmod_local_bitfield),
+ bpf_map__value_size(skel->maps.testmod_local_bitfield),
+ "local_bitfield_shadow_size"))
+ goto cleanup;
+
+ /* Normal fields around the unsupported bitfield remain accessible. */
+ skel->struct_ops.testmod_local_bitfield->data = 1;
+ skel->struct_ops.testmod_local_bitfield->onebyte = 1;
+
+ err = bpf_map__set_autocreate(skel->maps.testmod_local_bitfield, true);
+ if (!ASSERT_OK(err, "enable_local_bitfield_map"))
+ goto cleanup;
+
+ if (start_libbpf_log_capture())
+ goto cleanup;
+ err = struct_ops_module__load(skel);
+ log = stop_libbpf_log_capture();
+ if (!ASSERT_EQ(err, -ENOTSUP, "struct_ops_module_load_local_bitfield"))
+ goto cleanup;
+ ASSERT_HAS_SUBSTR(log, "local bitfield extra_bitfield is not supported",
+ "local_bitfield_rejection_log");
+
+cleanup:
+ free(log);
+ struct_ops_module__destroy(skel);
+}
+
+static void test_struct_ops_bad_member_offset(void)
+{
+ struct struct_ops_module *skel;
+ struct btf_member *member;
+ struct btf_type *type;
+ struct btf *btf;
+ char *log = NULL;
+ int err, type_id;
+
+ skel = struct_ops_module__open();
+ if (!ASSERT_OK_PTR(skel, "struct_ops_module_open_bad_offset"))
+ return;
+
+ btf = bpf_object__btf(skel->obj);
+ type_id = btf__find_by_name_kind(btf, "bpf_testmod_ops___bad_offset",
+ BTF_KIND_STRUCT);
+ if (!ASSERT_GT(type_id, 0, "find_bad_offset_type"))
+ goto cleanup;
+ type = (struct btf_type *)btf__type_by_id(btf, type_id);
+ if (!ASSERT_OK_PTR(type, "get_bad_offset_type"))
+ goto cleanup;
+
+ member = btf_members(type);
+ member->offset = type->size * 8;
+
+ err = bpf_map__set_autocreate(skel->maps.testmod_bad_offset, true);
+ if (!ASSERT_OK(err, "enable_bad_offset_map"))
+ goto cleanup;
+
+ if (start_libbpf_log_capture())
+ goto cleanup;
+ err = struct_ops_module__load(skel);
+ log = stop_libbpf_log_capture();
+ if (!ASSERT_EQ(err, -EINVAL, "struct_ops_module_load_bad_offset"))
+ goto cleanup;
+ ASSERT_HAS_SUBSTR(log, "member extra is outside the 4-byte local struct_ops type",
+ "bad_offset_rejection_log");
+
+cleanup:
+ free(log);
+ struct_ops_module__destroy(skel);
+}
+
/* The signature of an implementation might not match the signature of the
* function pointer prototype defined in the BPF program. This mismatch
* should be allowed as long as the behavior of the operator program
@@ -304,6 +390,10 @@ void serial_test_struct_ops_module(void)
test_struct_ops_load();
if (test__start_subtest("struct_ops_not_zeroed"))
test_struct_ops_not_zeroed();
+ if (test__start_subtest("struct_ops_local_bitfield"))
+ test_struct_ops_local_bitfield();
+ if (test__start_subtest("struct_ops_bad_member_offset"))
+ test_struct_ops_bad_member_offset();
if (test__start_subtest("struct_ops_incompatible"))
test_struct_ops_incompatible();
if (test__start_subtest("struct_ops_null_out_cb"))
@@ -314,4 +404,3 @@ void serial_test_struct_ops_module(void)
test_detach_link();
RUN_TESTS(unsupported_ops);
}
-
diff --git a/tools/testing/selftests/bpf/progs/struct_ops_module.c b/tools/testing/selftests/bpf/progs/struct_ops_module.c
index 71c420c3a5a6c..846b5a934dc83 100644
--- a/tools/testing/selftests/bpf/progs/struct_ops_module.c
+++ b/tools/testing/selftests/bpf/progs/struct_ops_module.c
@@ -76,6 +76,23 @@ struct bpf_testmod_ops___zeroed testmod_zeroed = {
.zeroed_op = (void *)zeroed_op,
};
+struct bpf_testmod_ops___local_bitfield {
+ int data;
+ unsigned int extra_bitfield:31;
+ unsigned int extra_bitfield2:1;
+ char onebyte;
+};
+
+SEC("?.struct_ops.link")
+struct bpf_testmod_ops___local_bitfield testmod_local_bitfield;
+
+struct bpf_testmod_ops___bad_offset {
+ unsigned int extra;
+};
+
+SEC("?.struct_ops.link")
+struct bpf_testmod_ops___bad_offset testmod_bad_offset;
+
struct bpf_testmod_ops___incompatible {
int (*test_1)(void);
void (*test_2)(int *a);
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH bpf v2 3/3] selftests/bpf: Cover struct_ops bitfield and offset validation
2026-09-13 14:52 ` [PATCH bpf v2 3/3] selftests/bpf: Cover struct_ops bitfield and offset validation Mingpei CAO
@ 2026-09-13 15:03 ` sashiko-bot
2026-09-13 15:49 ` bot+bpf-ci
1 sibling, 0 replies; 9+ messages in thread
From: sashiko-bot @ 2026-09-13 15:03 UTC (permalink / raw)
To: Mingpei CAO; +Cc: bpf
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The newly added tests `test_struct_ops_local_bitfield` and `test_struct_ops_bad_member_offset` fail during skeleton load because they forget to disable autocreation of the `testmod_zeroed` map.
--
commit 69d278fe659a33375aee90852a40642cb2fe6e03
Author: Mingpei CAO <caomingpei@gmail.com>
selftests/bpf: Cover struct_ops bitfield and offset validation
This commit adds selftests to verify libbpf's handling of struct_ops bitfield
members and incorrect member offsets. It ensures that unsupported local
bitfields are rejected and bad offsets yield an EINVAL error.
> diff --git a/tools/testing/selftests/bpf/prog_tests/test_struct_ops_module.c b/tools/testing/selftests/bpf/prog_tests/test_struct_ops_module.c
> index 75a0dea511b3f..41029ebb22f01 100644
> --- a/tools/testing/selftests/bpf/prog_tests/test_struct_ops_module.c
> +++ b/tools/testing/selftests/bpf/prog_tests/test_struct_ops_module.c
[ ... ]
> @@ -150,6 +151,91 @@ static void test_struct_ops_not_zeroed(void)
> struct_ops_module__destroy(skel);
> }
>
> +static void test_struct_ops_local_bitfield(void)
> +{
> + struct struct_ops_module *skel;
> + char *log = NULL;
> + int err;
[ ... ]
> + err = bpf_map__set_autocreate(skel->maps.testmod_local_bitfield, true);
> + if (!ASSERT_OK(err, "enable_local_bitfield_map"))
> + goto cleanup;
> +
> + if (start_libbpf_log_capture())
> + goto cleanup;
> + err = struct_ops_module__load(skel);
[Severity: Medium]
Will this skeleton load fail prematurely due to testmod_zeroed?
When struct_ops_module__load() is called, libbpf iterates over all
autocreate maps in the BPF object and initializes them. The testmod_zeroed
map is defined first in the BPF object and is set to autocreate by default.
Since its zeroed_op member is initialized to a non-NULL program pointer but
does not exist in the kernel BTF, bpf_map__init_kern_struct_ops() returns
-ENOTSUP.
This causes the skeleton load to abort early before testmod_local_bitfield
is fully evaluated, leading to false negatives in the test suite. Does
this test need to disable autocreation for the testmod_zeroed map before
calling struct_ops_module__load()?
> + log = stop_libbpf_log_capture();
> + if (!ASSERT_EQ(err, -ENOTSUP, "struct_ops_module_load_local_bitfield"))
> + goto cleanup;
> + ASSERT_HAS_SUBSTR(log, "local bitfield extra_bitfield is not supported",
> + "local_bitfield_rejection_log");
> +
> +cleanup:
> + free(log);
> + struct_ops_module__destroy(skel);
> +}
> +
> +static void test_struct_ops_bad_member_offset(void)
> +{
> + struct struct_ops_module *skel;
> + struct btf_member *member;
[ ... ]
> + err = bpf_map__set_autocreate(skel->maps.testmod_bad_offset, true);
> + if (!ASSERT_OK(err, "enable_bad_offset_map"))
> + goto cleanup;
> +
> + if (start_libbpf_log_capture())
> + goto cleanup;
> + err = struct_ops_module__load(skel);
[Severity: Medium]
Does this suffer from the same early skeleton load failure?
Since testmod_zeroed is still set to autocreate, the load will return
-ENOTSUP instead of the expected -EINVAL, causing the subsequent ASSERT_EQ
check to fail unconditionally. Should autocreation of testmod_zeroed be
disabled in this function as well?
> + log = stop_libbpf_log_capture();
> + if (!ASSERT_EQ(err, -EINVAL, "struct_ops_module_load_bad_offset"))
> + goto cleanup;
> + ASSERT_HAS_SUBSTR(log, "member extra is outside the 4-byte local struct_ops type",
> + "bad_offset_rejection_log");
> +
> +cleanup:
> + free(log);
> + struct_ops_module__destroy(skel);
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260913145233.1446643-1-caomingpei@gmail.com?part=3
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH bpf v2 3/3] selftests/bpf: Cover struct_ops bitfield and offset validation
2026-09-13 14:52 ` [PATCH bpf v2 3/3] selftests/bpf: Cover struct_ops bitfield and offset validation Mingpei CAO
2026-09-13 15:03 ` sashiko-bot
@ 2026-09-13 15:49 ` bot+bpf-ci
1 sibling, 0 replies; 9+ messages in thread
From: bot+bpf-ci @ 2026-09-13 15:49 UTC (permalink / raw)
To: caomingpei, bpf
Cc: andrii, eddyz87, ameryhung, qmo, caomingpei, ast, andrii, daniel,
martin.lau, eddyz87, yonghong.song, mason, ihor.solodrai
[-- Attachment #1: Type: text/plain, Size: 3760 bytes --]
> diff --git a/tools/testing/selftests/bpf/progs/struct_ops_module.c b/tools/testing/selftests/bpf/progs/struct_ops_module.c
> index 71c420c3a5a6..846b5a934dc8 100644
> --- a/tools/testing/selftests/bpf/progs/struct_ops_module.c
> +++ b/tools/testing/selftests/bpf/progs/struct_ops_module.c
[ ... ]
> @@ -76,6 +76,23 @@ struct bpf_testmod_ops___zeroed testmod_zeroed = {
> .zeroed_op = (void *)zeroed_op,
> };
>
> +struct bpf_testmod_ops___local_bitfield {
> + int data;
> + unsigned int extra_bitfield:31;
> + unsigned int extra_bitfield2:1;
> + char onebyte;
> +};
> +
> +SEC("?.struct_ops.link")
> +struct bpf_testmod_ops___local_bitfield testmod_local_bitfield;
> +
> +struct bpf_testmod_ops___bad_offset {
> + unsigned int extra;
> +};
> +
> +SEC("?.struct_ops.link")
> +struct bpf_testmod_ops___bad_offset testmod_bad_offset;
This is the first place in the tree that puts two struct_ops variables
into the same '?'-prefixed section. Does libbpf handle this correctly?
Looking at init_struct_ops_maps() in tools/lib/bpf/libbpf.c, the code
receives one sec_name for the whole section and advances it inside the
per-variable loop:
for (i = 0; i < btf_vlen(datasec); i++, vsi++) {
...
map = bpf_object__add_map(obj);
...
if (sec_name[0] == '?') {
map->autocreate = false;
sec_name++; /* persists across iterations */
}
Both new variables live in the same ELF section "?.struct_ops.link", so
they are handled by a single init_struct_ops_maps() call. libbpf sorts
DATASEC var_secinfos by offset (btf_fixup_datasec()), so iteration 0 is
testmod_local_bitfield (offset 0, size 12): sec_name[0] == '?',
autocreate is set to false and sec_name is advanced to ".struct_ops.link".
On iteration 1 (testmod_bad_offset, offset 12) sec_name[0] is now '.', the
branch is not taken, and testmod_bad_offset keeps the default
autocreate == true from bpf_object__add_map().
As a result:
1. The SEC("?...") annotation on testmod_bad_offset is inert.
bpf_map__set_autocreate(skel->maps.testmod_bad_offset, true) in the new
test_struct_ops_bad_member_offset() is a no-op, so the test's apparent
opt-in isolation is not real.
2. testmod_bad_offset is now initialised and created during every other
subtest that loads this shared skeleton: struct_ops_load,
struct_ops_not_zeroed, struct_ops_incompatible, struct_ops_null_out_cb,
struct_ops_forgotten_cb, test_detach_link, plus test_cpp.cpp:127
(struct_ops_module__open_and_load()). Those loads happen to still
succeed today only because 'extra' is absent from the kernel struct
bpf_testmod_ops and the zero-filled variable passes the
libbpf_is_mem_zeroed() fallback in bpf_map__init_kern_struct_ops() -
i.e. the negative-test map is now silently registered as a real
struct_ops map in unrelated tests.
3. The outcome depends on which variable LLVM emits first in the section.
If testmod_bad_offset were emitted at offset 0 and testmod_local_bitfield
after it, then testmod_local_bitfield would be the one with
autocreate == true, and every other subtest plus test_cpp would fail
their load with -ENOTSUP "local bitfield extra_bitfield is not
supported". The patch works only by relying on unspecified
global-emission ordering.
Giving each optional map its own section name (e.g. distinct sections or
fixing libbpf's init_struct_ops_maps() to not advance the shared sec_name
inside the loop) removes the coupling.
---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/34765254213
^ permalink raw reply [flat|nested] 9+ messages in thread