mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v4 0/3] KVM: guest_memfd: Fix signed offset+size goof
@ 2026-06-02 17:09 Sean Christopherson
  2026-06-02 17:09 ` [PATCH v4 1/3] KVM: guest_memfd: Treat memslot binding offset+size as unsigned values Sean Christopherson
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Sean Christopherson @ 2026-06-02 17:09 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: kvm, linux-kernel, Ackerley Tng, Michael Roth, Sean Christopherson

Fix a bug where KVM fails to reject a comically large offset into guest_memfd
if offset+size results in a signed, negative value.  Add a testcase to prove
the bug, and to serve as a regression test.

Note, v1 and v2 was part of larger series.

v4:
 - Collect tags. [Mike, Ackerley]
 - Use "INT64_MAX - page_size" instead of hardcoded literal. [Sashiko, Ackerley]

v3:
 - Use uoff_t, not u64. [the combined might of Sean and Ackerley]
 - Explaining exactly what is broken. [Ackerley]
 - Add a regression test.

v2: https://lore.kernel.org/all/20260522-fix-sev-gmem-post-populate-v2-0-3f196bfad5a1@google.com
v1: https://lore.kernel.org/r/20260522-fix-sev-gmem-post-populate-v1-0-9fc8d6437b65@google.com

Sean Christopherson (3):
  KVM: guest_memfd: Treat memslot binding offset+size as unsigned values
  KVM: selftests: Expand the guest_memfd test macros to allow passing
    the VM
  KVM: selftests: Add guest_memfd regression test signed offset+size bug

 .../testing/selftests/kvm/guest_memfd_test.c  | 24 +++++++++++++++++--
 virt/kvm/guest_memfd.c                        |  8 +++----
 virt/kvm/kvm_mm.h                             |  7 ++++--
 3 files changed, 31 insertions(+), 8 deletions(-)


base-commit: d1568b1332b6b3b36b222c2868fc102727c12a34
-- 
2.54.0.929.g9b7fa37559-goog


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v4 1/3] KVM: guest_memfd: Treat memslot binding offset+size as unsigned values
  2026-06-02 17:09 [PATCH v4 0/3] KVM: guest_memfd: Fix signed offset+size goof Sean Christopherson
@ 2026-06-02 17:09 ` Sean Christopherson
  2026-06-02 17:09 ` [PATCH v4 2/3] KVM: selftests: Expand the guest_memfd test macros to allow passing the VM Sean Christopherson
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Sean Christopherson @ 2026-06-02 17:09 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: kvm, linux-kernel, Ackerley Tng, Michael Roth, Sean Christopherson

When binding a memslot to a guest_memfd file, treat the offset and size as
unsigned values to fix a bug where the sum of the two can result in a false
negative when checking for overflow against the size of the file.  Passing
unsigned values also avoids relying on somewhat obscure checks in other
flows for safety, and tracks the offset and size as they are intended to be
tracked, as unsigned values.

On 64-bit kernels, the number of pages a memslot contains and thus the size
(and offset) of its guest_memfd binding are unsigned 64-bit values.  Taking
the offset+size as an loff_t instead of a uoff_t inadvertently converts
the unsigned value to a signed value if the offset and/or size is massive.

Locally storing the offset and size as signed values is benign in and of
itself (though even that is *extremely* difficult to discern), but
operating on their sum is not.

For the offset, KVM explicitly checks against a negative value, which might
seem like a bug as KVM could incorrectly reject a legitimate binding, but
that's not actually the case as KVM_CREATE_GUEST_MEMFD takes a signed value
for its size, i.e. a would-be-negative offset is also greater than the
maximum possible size of any guest_memfd file.

Regarding the size, while KVM lacks an explicit check for a negative value,
i.e. seemingly has a flawed overflow check, KVM restricts the number of
pages in a single memslot to the largest positive signed 32-bit value:

        if (id < KVM_USER_MEM_SLOTS &&
            (mem->memory_size >> PAGE_SHIFT) > KVM_MEM_MAX_NR_PAGES)
                return -EINVAL;

and so that maximum "size" will ever be is 0x7fffffff000.

The sum of the two is, however, problematic.  While the size is restricted
by KVM's memslot logic, the offset is not, i.e. the offset is completely
unchecked until the "offset + size > i_size_read(inode)" check.  If the
offset is the (nearly) largest possible _positive_ value, then adding size
to the offset can result in a signed, negative 64-bit value.  When compared
against the size of the file (guaranteed to be positive), the negative sum
is always smaller, and KVM incorrectly allows the absurd offset.

Opportunistically add missing includes in kvm_mm.h (instead of relying on
its parents).

Fixes: a7800aa80ea4 ("KVM: Add KVM_CREATE_GUEST_MEMFD ioctl() for guest-specific backing memory")
Cc: stable@vger.kernel.org
Cc: Ackerley Tng <ackerleytng@google.com>
Reviewed-by: Michael Roth <michael.roth@amd.com>
Reviewed-by: Ackerley Tng <ackerleytng@google.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 virt/kvm/guest_memfd.c | 8 ++++----
 virt/kvm/kvm_mm.h      | 7 +++++--
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
index bf9659a7b0f6..a1cb72e66288 100644
--- a/virt/kvm/guest_memfd.c
+++ b/virt/kvm/guest_memfd.c
@@ -640,15 +640,16 @@ int kvm_gmem_create(struct kvm *kvm, struct kvm_create_guest_memfd *args)
 }
 
 int kvm_gmem_bind(struct kvm *kvm, struct kvm_memory_slot *slot,
-		  unsigned int fd, loff_t offset)
+		  unsigned int fd, uoff_t offset)
 {
-	loff_t size = slot->npages << PAGE_SHIFT;
+	uoff_t size = slot->npages << PAGE_SHIFT;
 	unsigned long start, end;
 	struct gmem_file *f;
 	struct inode *inode;
 	struct file *file;
 	int r = -EINVAL;
 
+	BUILD_BUG_ON(sizeof(gpa_t) != sizeof(offset));
 	BUILD_BUG_ON(sizeof(gfn_t) != sizeof(slot->gmem.pgoff));
 
 	file = fget(fd);
@@ -664,8 +665,7 @@ int kvm_gmem_bind(struct kvm *kvm, struct kvm_memory_slot *slot,
 
 	inode = file_inode(file);
 
-	if (offset < 0 || !PAGE_ALIGNED(offset) ||
-	    offset + size > i_size_read(inode))
+	if (!PAGE_ALIGNED(offset) || offset + size > i_size_read(inode))
 		goto err;
 
 	filemap_invalidate_lock(inode->i_mapping);
diff --git a/virt/kvm/kvm_mm.h b/virt/kvm/kvm_mm.h
index 9fcc5d5b7f8d..7510ca915dd1 100644
--- a/virt/kvm/kvm_mm.h
+++ b/virt/kvm/kvm_mm.h
@@ -3,6 +3,9 @@
 #ifndef __KVM_MM_H__
 #define __KVM_MM_H__ 1
 
+#include <linux/kvm.h>
+#include <linux/kvm_types.h>
+
 /*
  * Architectures can choose whether to use an rwlock or spinlock
  * for the mmu_lock.  These macros, for use in common code
@@ -72,7 +75,7 @@ int kvm_gmem_init(struct module *module);
 void kvm_gmem_exit(void);
 int kvm_gmem_create(struct kvm *kvm, struct kvm_create_guest_memfd *args);
 int kvm_gmem_bind(struct kvm *kvm, struct kvm_memory_slot *slot,
-		  unsigned int fd, loff_t offset);
+		  unsigned int fd, uoff_t offset);
 void kvm_gmem_unbind(struct kvm_memory_slot *slot);
 #else
 static inline int kvm_gmem_init(struct module *module)
@@ -82,7 +85,7 @@ static inline int kvm_gmem_init(struct module *module)
 static inline void kvm_gmem_exit(void) {};
 static inline int kvm_gmem_bind(struct kvm *kvm,
 					 struct kvm_memory_slot *slot,
-					 unsigned int fd, loff_t offset)
+					 unsigned int fd, uoff_t offset)
 {
 	WARN_ON_ONCE(1);
 	return -EIO;
-- 
2.54.0.929.g9b7fa37559-goog


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v4 2/3] KVM: selftests: Expand the guest_memfd test macros to allow passing the VM
  2026-06-02 17:09 [PATCH v4 0/3] KVM: guest_memfd: Fix signed offset+size goof Sean Christopherson
  2026-06-02 17:09 ` [PATCH v4 1/3] KVM: guest_memfd: Treat memslot binding offset+size as unsigned values Sean Christopherson
@ 2026-06-02 17:09 ` Sean Christopherson
  2026-06-02 17:09 ` [PATCH v4 3/3] KVM: selftests: Add guest_memfd regression test signed offset+size bug Sean Christopherson
  2026-06-05 18:31 ` [PATCH v4 0/3] KVM: guest_memfd: Fix signed offset+size goof Sean Christopherson
  3 siblings, 0 replies; 5+ messages in thread
From: Sean Christopherson @ 2026-06-02 17:09 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: kvm, linux-kernel, Ackerley Tng, Michael Roth, Sean Christopherson

Expand the gmem test macros to allow passing the VM to testcases, without
needing to plumb the VM into _every_ testcase, as the vast majority of
testcases only need the fd and size.

No functional change intended.

Reviewed-by: Ackerley Tng <ackerleytng@google.com>
Tested-by: Ackerley Tng <ackerleytng@google.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 tools/testing/selftests/kvm/guest_memfd_test.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/kvm/guest_memfd_test.c b/tools/testing/selftests/kvm/guest_memfd_test.c
index 832ef4dfb99f..246bb408ecc0 100644
--- a/tools/testing/selftests/kvm/guest_memfd_test.c
+++ b/tools/testing/selftests/kvm/guest_memfd_test.c
@@ -408,17 +408,26 @@ static void test_guest_memfd_flags(struct kvm_vm *vm)
 	}
 }
 
-#define __gmem_test(__test, __vm, __flags, __gmem_size)			\
+#define ____gmem_test(__test, __vm, __flags, __gmem_size, args...)	\
 do {									\
 	int fd = vm_create_guest_memfd(__vm, __gmem_size, __flags);	\
 									\
-	test_##__test(fd, __gmem_size);					\
+	test_##__test(args);						\
 	close(fd);							\
 } while (0)
 
+#define __gmem_test(__test, __vm, __flags, __gmem_size)			\
+	____gmem_test(__test, __vm, __flags, __gmem_size, fd, __gmem_size)
+
 #define gmem_test(__test, __vm, __flags)				\
 	__gmem_test(__test, __vm, __flags, page_size * 4)
 
+#define __gmem_test_vm(__test, __vm, __flags, __gmem_size)		\
+	____gmem_test(__test, __vm, __flags, __gmem_size, __vm, fd, __gmem_size)
+
+#define gmem_test_vm(__test, __vm, __flags)				\
+	__gmem_test_vm(__test, __vm, __flags, page_size * 4)
+
 static void __test_guest_memfd(struct kvm_vm *vm, u64 flags)
 {
 	test_create_guest_memfd_multiple(vm);
-- 
2.54.0.929.g9b7fa37559-goog


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v4 3/3] KVM: selftests: Add guest_memfd regression test signed offset+size bug
  2026-06-02 17:09 [PATCH v4 0/3] KVM: guest_memfd: Fix signed offset+size goof Sean Christopherson
  2026-06-02 17:09 ` [PATCH v4 1/3] KVM: guest_memfd: Treat memslot binding offset+size as unsigned values Sean Christopherson
  2026-06-02 17:09 ` [PATCH v4 2/3] KVM: selftests: Expand the guest_memfd test macros to allow passing the VM Sean Christopherson
@ 2026-06-02 17:09 ` Sean Christopherson
  2026-06-05 18:31 ` [PATCH v4 0/3] KVM: guest_memfd: Fix signed offset+size goof Sean Christopherson
  3 siblings, 0 replies; 5+ messages in thread
From: Sean Christopherson @ 2026-06-02 17:09 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: kvm, linux-kernel, Ackerley Tng, Michael Roth, Sean Christopherson

Add a regression (and proof-of-bug) testcase to ensure KVM rejects an
offset+size that would result in a negative value when computed as a signed
64-bit value.  KVM had a flaw where it would allow binding a memslot to a
guest_memfd instance even with a wildly out-of-range offset, if the offset
and size were both positive values, but the combined offset+size was
negative.

Use "0x7fffffffffffffffull - page_size", i.e. "INT64_MAX - page_size", for
the offset as the size of the guest_memfd file must be at least page_size
(KVM requires memslots and gmem files to be host page-size aligned).  I.e.
"INT64_MAX - page_size + size" is guaranteed to generate an offset+size
that is negative when converted to a signed 64-bit value *and* honors KVM's
alignment requirements.

Reviewed-by: Ackerley Tng <ackerleytng@google.com>
Tested-by: Ackerley Tng <ackerleytng@google.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 tools/testing/selftests/kvm/guest_memfd_test.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/tools/testing/selftests/kvm/guest_memfd_test.c b/tools/testing/selftests/kvm/guest_memfd_test.c
index 246bb408ecc0..2233d871a38f 100644
--- a/tools/testing/selftests/kvm/guest_memfd_test.c
+++ b/tools/testing/selftests/kvm/guest_memfd_test.c
@@ -345,6 +345,16 @@ static void test_invalid_punch_hole(int fd, size_t total_size)
 	}
 }
 
+static void test_invalid_binding(struct kvm_vm *vm, int fd, size_t size)
+{
+	int r;
+
+	r = __vm_set_user_memory_region2(vm, 0, KVM_MEM_GUEST_MEMFD, 0, size, 0,
+					 fd, ALIGN_DOWN(INT64_MAX, page_size));
+	TEST_ASSERT(r && errno == EINVAL,
+		    "Memslot with out-of-range offset+size should fail");
+}
+
 static void test_create_guest_memfd_invalid_sizes(struct kvm_vm *vm,
 						  u64 guest_memfd_flags)
 {
@@ -456,6 +466,7 @@ static void __test_guest_memfd(struct kvm_vm *vm, u64 flags)
 	gmem_test(file_size, vm, flags);
 	gmem_test(fallocate, vm, flags);
 	gmem_test(invalid_punch_hole, vm, flags);
+	gmem_test_vm(invalid_binding, vm, flags);
 }
 
 static void test_guest_memfd(unsigned long vm_type)
-- 
2.54.0.929.g9b7fa37559-goog


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v4 0/3] KVM: guest_memfd: Fix signed offset+size goof
  2026-06-02 17:09 [PATCH v4 0/3] KVM: guest_memfd: Fix signed offset+size goof Sean Christopherson
                   ` (2 preceding siblings ...)
  2026-06-02 17:09 ` [PATCH v4 3/3] KVM: selftests: Add guest_memfd regression test signed offset+size bug Sean Christopherson
@ 2026-06-05 18:31 ` Sean Christopherson
  3 siblings, 0 replies; 5+ messages in thread
From: Sean Christopherson @ 2026-06-05 18:31 UTC (permalink / raw)
  To: Sean Christopherson, Paolo Bonzini
  Cc: kvm, linux-kernel, Ackerley Tng, Michael Roth

On Tue, 02 Jun 2026 10:09:18 -0700, Sean Christopherson wrote:
> Fix a bug where KVM fails to reject a comically large offset into guest_memfd
> if offset+size results in a signed, negative value.  Add a testcase to prove
> the bug, and to serve as a regression test.
> 
> Note, v1 and v2 was part of larger series.
> 
> v4:
>  - Collect tags. [Mike, Ackerley]
>  - Use "INT64_MAX - page_size" instead of hardcoded literal. [Sashiko, Ackerley]
> 
> [...]

Applied to kvm-x86 gmem, thanks!

[1/3] KVM: guest_memfd: Treat memslot binding offset+size as unsigned values
      https://github.com/kvm-x86/linux/commit/eba85fee7fc6
[2/3] KVM: selftests: Expand the guest_memfd test macros to allow passing the VM
      https://github.com/kvm-x86/linux/commit/b7a23fb0ed7e
[3/3] KVM: selftests: Add guest_memfd regression test signed offset+size bug
      https://github.com/kvm-x86/linux/commit/b408b52e7111

--
https://github.com/kvm-x86/linux/tree/next

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-06-05 18:31 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-06-02 17:09 [PATCH v4 0/3] KVM: guest_memfd: Fix signed offset+size goof Sean Christopherson
2026-06-02 17:09 ` [PATCH v4 1/3] KVM: guest_memfd: Treat memslot binding offset+size as unsigned values Sean Christopherson
2026-06-02 17:09 ` [PATCH v4 2/3] KVM: selftests: Expand the guest_memfd test macros to allow passing the VM Sean Christopherson
2026-06-02 17:09 ` [PATCH v4 3/3] KVM: selftests: Add guest_memfd regression test signed offset+size bug Sean Christopherson
2026-06-05 18:31 ` [PATCH v4 0/3] KVM: guest_memfd: Fix signed offset+size goof Sean Christopherson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®