Re: [PATCH 5/5] selftests/bpf: Test two-way arena access with two guests

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] selftests/bpf: unaligned cmsghdr buffer causes undefined behavior and SIGBUS
- [Medium] bpf: virtme-ng --numa argument risks unit mismatch in QEMU memory allocation
--

commit f89b47f73364f408524cec1a644bd3003fa751ab
Author: Andrea Righi <arighi@nvidia.com>

selftests/bpf: Test two-way arena access with two guests

This patch adds a selftest to verify two-way memory access between
two simultaneous virtme-ng guests using a shared pinned host arena.
It establishes mutual memory exchanges using direct BPF loads and stores.

> diff --git a/tools/testing/selftests/bpf/arena_kvm.py b/tools/testing/selftests/bpf/arena_kvm.py
> --- a/tools/testing/selftests/bpf/arena_kvm.py
> +++ b/tools/testing/selftests/bpf/arena_kvm.py
[ ... ]
> +def start_guest(pin, build, kernel, index, vng, env, guests):
> +    node0_size = GUEST_MEMORY - SLICE_SIZE
> +    base = GUEST_RAM_BASES[os.uname().machine] + node0_size
> +    progress(f"Starting guest {index}: {SLICE_SIZE // (1024 * 1024)} MiB "
> +             f"slice at file offset {index * SLICE_SIZE:#x}, "
> +             f"NUMA node 1 physical base {base:#x}")
> +    qemu_opts = (
> +        f"-object memory-backend-file,id=signal,size={SLICE_SIZE},share=on,"
> +        f"offset={index * SLICE_SIZE},mem-path={pin} "
> +        "-numa node,nodeid=1,memdev=signal"
> +    )
> +    guest_command = [str(build / "arena_kvm_guest-init"),
> +                     str(build / "arena_kvm_guest.bpf.o")]
> +    command = [
> +        vng, "--verbose", "--run", str(kernel), "--cpus", "2",
> +        "--memory", f"{GUEST_MEMORY // (1024 * 1024)}M",
> +        "--numa", str(node0_size),

[Severity: Medium]
Will this --numa argument cause a unit mismatch in QEMU's memory allocation?

The --numa argument is passed a raw byte count (node0_size), while --memory
is explicitly passed in megabytes with an M suffix. Since QEMU's legacy
-numa node,mem= option defaults to megabytes if no unit is specified,
passing a large byte count could be interpreted as terabytes. This could
potentially cause the virtme-ng VM launch to fail immediately.

[ ... ]
> diff --git a/tools/testing/selftests/bpf/arena_kvm_host.c b/tools/testing/selftests/bpf/arena_kvm_host.c
> --- a/tools/testing/selftests/bpf/arena_kvm_host.c
> +++ b/tools/testing/selftests/bpf/arena_kvm_host.c
[ ... ]
> +static int create_arena(const char *pin, int socket_fd, unsigned int pages)
> +{
> +	LIBBPF_OPTS(bpf_map_create_opts, opts,
> +		.map_flags = BPF_F_MMAPABLE | BPF_F_ARENA_NO_FREE |
> +			     BPF_F_ARENA_EXPORT);
> +	char control[CMSG_SPACE(sizeof(int))] = {};
> +	char byte = 0;
> +	struct iovec iov = { .iov_base = &byte, .iov_len = 1 };
> +	struct msghdr msg = {
> +		.msg_iov = &iov,
> +		.msg_iovlen = 1,
> +		.msg_control = control,
> +		.msg_controllen = sizeof(control),
> +	};
> +	struct cmsghdr *cmsg = CMSG_FIRSTHDR(&msg);

[Severity: Medium]
Does declaring the control buffer as a plain char array risk alignment faults?

The control array defaults to 1-byte alignment, but CMSG_FIRSTHDR casts it
to a struct cmsghdr pointer. Accessing its members (such as cmsg_level and
cmsg_type) requires the pointer to be aligned to the architecture's word
boundary.

If the compiler places the char array at an unaligned address, this could
invoke undefined behavior and cause a SIGBUS on architectures with strict
alignment requirements. Using a union or proper alignment attributes would
satisfy POSIX and C standards here.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261010224902.1232178-1-arighi@nvidia.com?part=5