Skip to content

fix(hiding_ci): race condition in write syscall #5338

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
+71 −86
Closed

fix(hiding_ci): race condition in write syscall #5338

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
149 changes: 67 additions & 82 deletions ...i/linux_patches/20-gmem-write/0039-KVM-guest_memfd-add-generic-population-via-write.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
From 859e9756eb70cfc8cf4fb4f0870908e55521cc7f Mon Sep 17 00:00:00 2001
From 77bdbb5b4541b7737d2b9a52de04ba2c47613bf0 Mon Sep 17 00:00:00 2001
From: Nikita Kalyazin <[email protected]>
Date: Mon, 3 Mar 2025 13:08:37 +0000
Subject: [PATCH 39/46] KVM: guest_memfd: add generic population via write
Subject: [PATCH] KVM: guest_memfd: add generic population via write

write syscall populates guest_memfd with user-supplied data in a generic
way, ie no vendor-specific preparation is performed. This is supposed
Expand All @@ -18,109 +18,94 @@ The following behaviour is implemented:

Signed-off-by: Nikita Kalyazin <[email protected]>
---
virt/kvm/guest_memfd.c | 88 +++++++++++++++++++++++++++++++++++++++++-
1 file changed, 87 insertions(+), 1 deletion(-)
virt/kvm/guest_memfd.c | 60 +++++++++++++++++++++++++++++++++++++++++-
1 file changed, 59 insertions(+), 1 deletion(-)

diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
index d70ee66bb96d..809da2a2fb37 100644
index d70ee66bb96d..984b32752b27 100644
--- a/virt/kvm/guest_memfd.c
+++ b/virt/kvm/guest_memfd.c
@@ -392,8 +392,93 @@ static int kvm_gmem_mmap(struct file *file, struct vm_area_struct *vma)
return 0;
@@ -393,7 +393,9 @@ static int kvm_gmem_mmap(struct file *file, struct vm_area_struct *vma)
}

+static ssize_t kvm_kmem_gmem_write(struct file *file, const char __user *buf,
+ size_t count, loff_t *offset)
static struct file_operations kvm_gmem_fops = {
- .mmap = kvm_gmem_mmap,
+ .mmap = kvm_gmem_mmap,
+ .llseek = default_llseek,
+ .write_iter = generic_perform_write,
.open = generic_file_open,
.release = kvm_gmem_release,
.fallocate = kvm_gmem_fallocate,
@@ -404,6 +406,59 @@ void kvm_gmem_init(struct module *module)
kvm_gmem_fops.owner = module;
}

+static int kvm_kmem_gmem_write_begin(struct file *file, struct address_space *mapping,
+ loff_t pos, unsigned len, struct folio **foliop,
+ void **fsdata)
+{
+ pgoff_t start, end, index;
+ ssize_t ret = 0;
+ pgoff_t index = pos >> PAGE_SHIFT;
+ struct folio *folio;
+
+ if (!PAGE_ALIGNED(*offset) || !PAGE_ALIGNED(count))
+ if (!PAGE_ALIGNED(pos) || len != PAGE_SIZE)
+ return -EINVAL;
+
+ if (*offset + count > i_size_read(file_inode(file)))
+ if (pos + len > i_size_read(file_inode(file)))
+ return -EINVAL;
+
+ if (!buf)
+ return -EINVAL;
+
+ start = *offset >> PAGE_SHIFT;
+ end = (*offset + count) >> PAGE_SHIFT;
+
+ filemap_invalidate_lock_shared(file->f_mapping);
+
+ for (index = start; index < end; ) {
+ struct folio *folio;
+ void *vaddr;
+ pgoff_t buf_offset = (index - start) << PAGE_SHIFT;
+
+ if (signal_pending(current)) {
+ ret = -EINTR;
+ goto out;
+ }
+ folio = kvm_gmem_get_folio(file_inode(file), index);
+ if (IS_ERR(folio))
+ return -EFAULT;
+
+ folio = kvm_gmem_get_folio(file_inode(file), index);
+ if (IS_ERR(folio)) {
+ ret = -EFAULT;
+ goto out;
+ }
+
+ if (folio_test_hwpoison(folio)) {
+ folio_unlock(folio);
+ folio_put(folio);
+ ret = -EFAULT;
+ goto out;
+ }
+
+ /* No support for huge pages. */
+ if (WARN_ON_ONCE(folio_test_large(folio))) {
+ folio_unlock(folio);
+ folio_put(folio);
+ ret = -EFAULT;
+ goto out;
+ }
+
+ if (folio_test_uptodate(folio)) {
+ folio_unlock(folio);
+ folio_put(folio);
+ ret = -ENOSPC;
+ goto out;
+ }
+ if (WARN_ON_ONCE(folio_test_large(folio))) {
+ folio_unlock(folio);
+ folio_put(folio);
+ return -EFAULT;
+ }
+
+ if (folio_test_uptodate(folio)) {
+ folio_unlock(folio);
+ folio_put(folio);
+ return -ENOSPC;
+ }
+
+ vaddr = kmap_local_folio(folio, 0);
+ ret = copy_from_user(vaddr, buf + buf_offset, PAGE_SIZE);
+ kunmap_local(vaddr);
+ if (ret) {
+ ret = -EINVAL;
+ folio_put(folio);
+ goto out;
+ }
+ *foliop = folio;
+ return 0;
+}
+
+ kvm_gmem_mark_prepared(folio);
+ folio_put(folio);
+static int kvm_kmem_gmem_write_end(struct file *file, struct address_space *mapping,
+ loff_t pos, unsigned len, unsigned copied,
+ struct folio *folio, void *fsdata)
+{
+ int ret;
+
+ index = folio_next_index(folio);
+ *offset += PAGE_SIZE;
+ if (copied == len) {
+ kvm_gmem_mark_prepared(folio);
+ ret = copied;
+ } else {
+ filemap_remove_folio(folio);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this scenario, I think we need to return 0 instead of copied, because we're throwing away the data written

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point, fixed, thanks!

+ ret = 0;
+ }
+
+out:
+ filemap_invalidate_unlock_shared(file->f_mapping);
+ folio_unlock(folio);
+ folio_put(folio);
+
+ return ret && start == (*offset >> PAGE_SHIFT) ?
+ ret : *offset - (start << PAGE_SHIFT);
+ return ret;
+}
+
static struct file_operations kvm_gmem_fops = {
- .mmap = kvm_gmem_mmap,
+ .mmap = kvm_gmem_mmap,
+ .llseek = default_llseek,
+ .write = kvm_kmem_gmem_write,
.open = generic_file_open,
.release = kvm_gmem_release,
.fallocate = kvm_gmem_fallocate,
@@ -514,6 +599,7 @@ static int __kvm_gmem_create(struct kvm *kvm, loff_t size, u64 flags)
static int kvm_gmem_migrate_folio(struct address_space *mapping,
struct folio *dst, struct folio *src,
enum migrate_mode mode)
@@ -463,6 +518,8 @@ static void kvm_gmem_free_folio(struct address_space *mapping,

static const struct address_space_operations kvm_gmem_aops = {
.dirty_folio = noop_dirty_folio,
+ .write_begin = kvm_kmem_gmem_write_begin,
+ .write_end = kvm_kmem_gmem_write_end,
.migrate_folio = kvm_gmem_migrate_folio,
.error_remove_folio = kvm_gmem_error_folio,
.free_folio = kvm_gmem_free_folio,
@@ -514,6 +571,7 @@ static int __kvm_gmem_create(struct kvm *kvm, loff_t size, u64 flags)
}

file->f_flags |= O_LARGEFILE;
Expand Down
8 changes: 4 additions & 4 deletions ...ng_ci/linux_patches/20-gmem-write/0040-KVM-selftests-update-guest_memfd-write-tests.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
From 205f30ad3c9fa142c34ab2ac6a3f6a762f6fc6b1 Mon Sep 17 00:00:00 2001
From 5d2885cd8a7904642d0c92d4464a74fedb167ddd Mon Sep 17 00:00:00 2001
From: Nikita Kalyazin <[email protected]>
Date: Mon, 3 Mar 2025 13:08:38 +0000
Subject: [PATCH 40/46] KVM: selftests: update guest_memfd write tests
Subject: [PATCH 2/2] KVM: selftests: update guest_memfd write tests

This is to reflect that the write syscall is now implemented for
guest_memfd.
Expand All @@ -12,7 +12,7 @@ Signed-off-by: Nikita Kalyazin <[email protected]>
1 file changed, 79 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/kvm/guest_memfd_test.c b/tools/testing/selftests/kvm/guest_memfd_test.c
index 1252e74fbb8f..3965a842896e 100644
index 1252e74fbb8f..4c8eccd751d9 100644
--- a/tools/testing/selftests/kvm/guest_memfd_test.c
+++ b/tools/testing/selftests/kvm/guest_memfd_test.c
@@ -22,18 +22,90 @@
Expand Down Expand Up @@ -59,7 +59,7 @@ index 1252e74fbb8f..3965a842896e 100644
+
+ ret = pwrite(fd, NULL, page_size, 0);
+ TEST_ASSERT(ret == -1, "supplying a NULL buffer when writing a guest_mem fd should fail");
+ TEST_ASSERT_EQ(errno, EINVAL);
+ TEST_ASSERT_EQ(errno, EFAULT);
+
+ /* Check double population is not allowed */
+
Expand Down