Re: [PATCH v2 4/4] KVM: selftests: use allowed NUMA nodes in guest_memfd_test

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Stack buffer overflow in test_mbind when retrieving memory policy on 32-bit architectures.
--

commit c29a637608e8b3ee37278728fd941b62df184a8b
Author: Shivank Garg <shivankg@amd.com>

KVM: selftests: use allowed NUMA nodes in guest_memfd_test

guest_memfd_test assumes that nodes 0 and 1 exist and have memory. Select
the required nodes from MPOL_F_MEMS_ALLOWED instead. The sysfs helpers for
finding maxnode are no longer needed.

> diff --git a/tools/testing/selftests/kvm/guest_memfd_test.c b/tools/testing/selftests/kvm/guest_memfd_test.c
> index 2233d871a38f4..aee80dda62295 100644
> --- a/tools/testing/selftests/kvm/guest_memfd_test.c
> +++ b/tools/testing/selftests/kvm/guest_memfd_test.c
[ ... ]
>  static void test_mbind(int fd, size_t total_size)
>  {
> -	const unsigned long nodemask_0 = 1; /* nid: 0 */
> -	unsigned long nodemask = 0;
> -	unsigned long maxnode = BITS_PER_TYPE(nodemask);
> +	unsigned long nodemask, bind_nodemask;
> +	unsigned long maxnode = BITS_PER_TYPE(nodemask) + 1;

[Severity: Medium]
Can this cause a stack buffer overflow in test_mbind() on 32-bit architectures?

The nodemask variable is declared as a single unsigned long, which is
4 bytes on 32-bit architectures. However, maxnode is set to
BITS_PER_TYPE(nodemask) + 1, which evaluates to 33.

>  	int policy;
>  	char *mem;
> +	int nid;
>  	int ret;
>  
> -	if (!is_multi_numa_node_system())
> +	if (!get_numa_node_ids(&nid, 1))
>  		return;
>  
> +	bind_nodemask = 1UL << nid;
>  	mem = kvm_mmap(total_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd);
>  
>  	/* Test MPOL_INTERLEAVE policy */
> -	kvm_mbind(mem, page_size * 2, MPOL_INTERLEAVE, &nodemask_0, maxnode, 0);
> +	kvm_mbind(mem, page_size * 2, MPOL_INTERLEAVE, &bind_nodemask, maxnode, 0);
>  	kvm_get_mempolicy(&policy, &nodemask, maxnode, mem, MPOL_F_ADDR);

When maxnode is passed to kvm_get_mempolicy() here, the kernel must
write at least 5 bytes to return 33 bits of node status. This rounds up to
8 bytes (two 32-bit words).

Will this overwrite the 4-byte nodemask buffer and corrupt adjacent
variables on the stack?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260901-gmem-selftests-fix-v2-0-5a273153354c@amd.com?part=4