Re: [PATCH 1/2] KVM: selftests: Cast guest_memfd fd to a signed int when checking for >= 0

[Ackerley Tng]

Sean Christopherson <seanjc@xxxxxxxxxx> writes:

> When conditionally closing a memory region's guest_memfd file descriptor,
> cast the field to a signed it so that negative values are correctly
> detected.  Because selftests reuse "struct kvm_userspace_memory_region2"
> instead of providing custom storage, they pick up the kernel uAPI's __u32
> definition of the file descriptor, not the more common "int" definition,
> e.g. that's used for userspace_mem_region.fd.
>

I wonder if the better way to handle this would be to have a clearer
userspace_mem_region vs memslot separation, where only the final
function actually making the SET_MEMORY_ATTRIBUTES ioctl builds the
struct and passes it to the ioctl. userspace_mem_region should store
userspace-facing or userspace-specific parameters.

Anyway this is the faster fix :)

Reviewed-by: Ackerley Tng <ackerleytng@xxxxxxxxxx>

> Fixes: bb2968ad6c33 ("KVM: selftests: Add support for creating private memslots")
> Reported-by: Bibo Mao <maobibo@xxxxxxxxxxx>
> Closes: https://lore.kernel.org/all/20260508015013.4108345-1-maobibo@xxxxxxxxxxx
> Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> ---
>  tools/testing/selftests/kvm/lib/kvm_util.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
> index e08967ef7b7b..4ad015c6c44f 100644
> --- a/tools/testing/selftests/kvm/lib/kvm_util.c
> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c
> @@ -817,7 +817,7 @@ static void __vm_mem_region_delete(struct kvm_vm *vm,
>  		kvm_munmap(region->mmap_alias, region->mmap_size);
>  		close(region->fd);
>  	}
> -	if (region->region.guest_memfd >= 0)
> +	if ((int)region->region.guest_memfd >= 0)
>  		close(region->region.guest_memfd);
>
>  	free(region);
> --
> 2.54.0.794.g4f17f83d09-goog