Re: [PATCH bpf-next 05/16] libbpf: Add the arena percpu chunk attribute to map creation options

[bot+bpf-ci@kernel.org]

[-- Attachment #1: Type: text/plain, Size: 3701 bytes --]

> libbpf: Add the arena percpu chunk attribute to map creation options

TL;DR: New fields inserted before log_opts in struct bpf_map_create_opts
appear to break libbpf ABI: binaries built against 1.8 may have log_opts
ignored or map creation fail with -EINVAL. They should likely be appended
after log_opts.

> diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
> --- a/tools/lib/bpf/bpf.h
> +++ b/tools/lib/bpf/bpf.h
> @@ -70,6 +70,10 @@ struct bpf_map_create_opts {
>  	const void *excl_prog_hash;
>  	__u32 excl_prog_hash_size;
>
> +	/* offset and size of an arena percpu chunk */
> +	__u32 arena_percpu_chunk_off;
> +	__u32 arena_percpu_chunk_sz;
> +
>  	struct bpf_log_opts *log_opts;
                              ^^^^

Does inserting these two fields ahead of log_opts break the layout of
struct bpf_map_create_opts for existing users?

log_opts was added by 702259006f93 ("libbpf: Add syscall common attributes
support for map_create") and is already in a released libbpf (3ead4ead078f
("libbpf: Start v1.9 development cycle") says libbpf 1.8.0 has been
released, and 702259006f93 is an ancestor of it).

The OPTS_HAS(opts, field) scheme relies on the caller-supplied sz and
offsetofend(type, field), which only works if new fields are appended at the
end.  On x86_64 the layout changes like this:

  old (1.8): excl_prog_hash_size @64, log_opts @72, sizeof == 80
  new:       excl_prog_hash_size @64, arena_percpu_chunk_off @68,
             arena_percpu_chunk_sz @72, log_opts @80, sizeof == 88

Consider an application built against the 1.8 headers that sets
opts.log_opts (opts.sz == 80), running against a shared libbpf that includes
this patch.  In bpf_map_create():

  attr.arena_percpu_chunk_off = OPTS_GET(opts, arena_percpu_chunk_off, 0);
      /* 80 >= 72: reads old padding */
  attr.arena_percpu_chunk_sz = OPTS_GET(opts, arena_percpu_chunk_sz, 0);
      /* 80 >= 76: reads low 32 bits of the old log_opts pointer */
  log_opts = OPTS_GET(opts, log_opts, NULL);
      /* 80 < 88: NULL */

So the caller's log_opts is silently ignored, no log buffer is filled, and
true_size is never written back.  The kernel also receives a non-zero
arena_percpu_chunk_sz.

On kernels without the arena percpu attr, BPF_MAP_CREATE_LAST_FIELD is still
excl_prog_hash_size in kernel/bpf/syscall.c at this commit, so
CHECK_ATTR(BPF_MAP_CREATE) sees non-zero bytes past that field and returns
-EINVAL.  Does that mean every map creation that passes log_opts, which used
to succeed, now fails?

On kernels with the later d7935093885b, an ARENA map would get a bogus chunk
size (rejected as unaligned, or accepted with a wrong layout), and other map
types would silently lose their log.

The reverse case also looks problematic.  An application built with the new
header sets arena_percpu_chunk_sz but not log_opts, then runs against libbpf
1.8.  The 1.8 library reads offset 72 (arena_percpu_chunk_sz plus padding) as
a struct bpf_log_opts pointer, and OPTS_VALID(log_opts, bpf_log_opts)
dereferences it.

Nothing later in this series (a6e6ab7a4b2f..08224ba2de19) touches
tools/lib/bpf/bpf.h, so this is not fixed by a following patch.

Should the two new fields be appended after log_opts instead, with the
last-field marker updated?

	struct bpf_log_opts *log_opts;

	/* offset and size of an arena percpu chunk */
	__u32 arena_percpu_chunk_off;
	__u32 arena_percpu_chunk_sz;

	size_t :0;
};
#define bpf_map_create_opts__last_field arena_percpu_chunk_sz


---
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/39434884362