Skip to contentSkip to conversation

mshv_vtl: fix poll waitqueue handling

PATCH Applied Daehyeon Ko 3 participants 5 messages Oct 19 – Oct 21

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

v2 cover mshv_vtl: fix poll waitqueue handling

MSHV_SET_POLL_FILE retains one embedded wait entry per CPU after vfs_poll() returns. Two independent bugs affect that state.

First, a provider such as signalfd may retire a non-file-owned waitqueue with POLLFREE. The MSHV callback leaves its entry linked and retains the queue pointer, allowing a later replacement to access freed provider memory. Patch 1 detaches on POLLFREE and introduces an RCU-safe removal helper.

Second, a poll callback may register more than one waitqueue. MSHV inserts the same embedded list node into each queue, corrupting the first list. Patch 2 rejects a second distinct queue and uses the common helper to unwind the first registration.

The roots, triggers, effects, and runtime oracles remain independent. Source-equivalent KASAN/LOCKDEP tests reproduced both bugs and validated the fixes. The local system is not a Hyper-V VTL2 environment, so the series has not been exercised through the real MSHV ioctl. OpenVMM does not exercise these paths in its current use of the interface.

An LLM assisted the source audits, fix development, and validation. Each patch notes that assistance in its changelog.

Changes in v2:
- combine the two previously standalone fixes into one series;
- rebase onto current Torvalds master;
- introduce the removal helper in patch 1 and reuse it in patch 2;
- replace the obsolete Assisted-by trailer with changelog prose.

Previous standalone submissions:
- POLLFREE v1: <[email protected]>
- multi-waitqueue v2, now withdrawn:
<[email protected]>

Daehyeon Ko (2):
mshv_vtl: detach poll waiter on POLLFREE
mshv_vtl: reject poll files with multiple waitqueues

 drivers/hv/mshv_vtl_main.c | 51 ++++++++++++++++++++++++++++++++++----
 1 file changed, 46 insertions(+), 5 deletions(-)

base-commit: c03c1a14507dfaf8b856c2608f9ac216021fc222

Signature · 1 line
2.55.0
#1 lore raw

PATCH v2 1/2 mshv_vtl: detach poll waiter on POLLFREE

MSHV_SET_POLL_FILE keeps a poll wait entry after the ioctl returns. A file reference does not always pin the registered waitqueue:
signalfd_poll() uses current->sighand->signalfd_wqh, whose lifetime is tied to the task.

signalfd_cleanup() sends POLLFREE before the sighand is freed. The MSHV callback currently leaves its entry linked, so __wake_up_pollfree() warns and a later replacement calls remove_wait_queue() through a stale wqh pointer. A source-equivalent KASAN run reproduced:

  BUG: KASAN: slab-use-after-free in _raw_spin_lock_irqsave
  _raw_spin_lock_irqsave
  remove_wait_queue
  harness_ioctl [mshv_pollfree_harness]

The object belonged to sighand_cache and had been released through slab_free_after_rcu_debug() after __cleanup_sighand().

On POLLFREE, unlink the entry under the provider waitqueue lock. Preserve the existing CPU cancellation before publishing a NULL wqh, since a NULL load lets a racing replacement reuse the persistent poll state. Use a matching acquire load under RCU when removing a normal registration.

The callback takes no mutex and does not sleep, so the existing mutex-to-waitqueue-lock order is unchanged.

An LLM assisted the source audit, fix development, and validation of this issue.

Fixes 7bfe3b8ea6e3 ("Drivers: hv: Introduce mshv_vtl driver")
---
 drivers/hv/mshv_vtl_main.c | 28 +++++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)
drivers/hv/mshv_vtl_main.c +25 -3
index d8225242c73f8..c6f1e2157d258 100644
@@ -17,6 +17,7 @@
 #include <linux/entry-virt.h>
 #include <linux/eventfd.h>
 #include <linux/poll.h>
+#include <linux/rcupdate.h>
 #include <linux/file.h>
 #include <linux/vmalloc.h>
 #include <asm/debugreg.h>
@@ -448,12 +449,35 @@ static void mshv_vtl_cancel(int cpu)
 static int mshv_vtl_poll_file_wake(wait_queue_entry_t *wait, unsigned int mode, int sync, void *key)
 {
 	struct mshv_vtl_poll_file *poll_file = container_of(wait, struct mshv_vtl_poll_file, wait);
+	__poll_t pollflags = key_to_poll(key);
+
+	if (pollflags & POLLFREE)
+		list_del_init(&wait->entry);
 
 	mshv_vtl_cancel(poll_file->cpu);
 
+	if (pollflags & POLLFREE) {
+		/* Publish teardown last; pair with acquire in the removal path. */
+		smp_store_release(&poll_file->wqh, NULL);
+	}
+
 	return 0;
 }
 
+static void mshv_vtl_poll_file_remove_wait_queue(struct mshv_vtl_poll_file *poll_file)
+{
+	wait_queue_head_t *wqh;
+
+	/* POLLFREE providers RCU-delay the waitqueue free. */
+	rcu_read_lock();
+	/* Pair with the release store in the POLLFREE callback. */
+	wqh = smp_load_acquire(&poll_file->wqh);
+	if (wqh)
+		remove_wait_queue(wqh, &poll_file->wait);
+	rcu_read_unlock();
+	WRITE_ONCE(poll_file->wqh, NULL);
+}
+
 static void mshv_vtl_ptable_queue_proc(struct file *file, wait_queue_head_t *wqh, poll_table *pt)
 {
 	struct mshv_vtl_poll_file *poll_file = container_of(pt, struct mshv_vtl_poll_file, pt);
@@ -490,9 +514,7 @@ static int mshv_vtl_ioctl_set_poll_file(struct mshv_vtl_set_poll_file __user *us
 
 	mutex_lock(&mshv_vtl_poll_file_lock);
 
-	if (poll_file->wqh)
-		remove_wait_queue(poll_file->wqh, &poll_file->wait);
-	poll_file->wqh = NULL;
+	mshv_vtl_poll_file_remove_wait_queue(poll_file);
 
 	old_file = poll_file->file;
 	poll_file->file = file;
Signature · 1 line
2.55.0
#2 lore raw

PATCH v2 2/2 mshv_vtl: reject poll files with multiple waitqueues

In reply to Daehyeon Ko

MSHV_SET_POLL_FILE invokes a file descriptor poll callback with a poll table backed by a single embedded wait entry. A poll callback may, however, register the table with more than one waitqueue. pipe_poll() does this naturally for an O_RDWR FIFO.

The second registration currently warns but then inserts the same list node into another list. The new insertion overwrites the entry links while the first waitqueue still points at it, corrupting the first waitqueue and causing an invalid callback traversal when that queue wakes.

Reject a second distinct waitqueue, remove the first registration through the common removal helper, and return -EINVAL from the ioctl. Repeated registration of the same waitqueue is treated as a no-op. A rejected replacement leaves the CPU slot unregistered, consistent with the existing remove-before-register ordering. Single-waitqueue users such as the OpenHCL io_uring poll file keep their existing behavior.

An LLM assisted the source audit, fix development, and validation of this issue.

Fixes 7bfe3b8ea6e3 ("Drivers: hv: Introduce mshv_vtl driver")
---
 drivers/hv/mshv_vtl_main.c | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)
drivers/hv/mshv_vtl_main.c +21 -2
index c6f1e2157d258..9133646b5b66c 100644
@@ -73,6 +73,7 @@ struct mshv_vtl_poll_file {
 	wait_queue_entry_t wait;
 	wait_queue_head_t *wqh;
 	poll_table pt;
+	int error;
 	int cpu;
 };
 
@@ -482,7 +483,18 @@ static void mshv_vtl_ptable_queue_proc(struct file *file, wait_queue_head_t *wqh
 {
 	struct mshv_vtl_poll_file *poll_file = container_of(pt, struct mshv_vtl_poll_file, pt);
 
-	WARN_ON(poll_file->wqh);
+	if (poll_file->error)
+		return;
+
+	if (poll_file->wqh) {
+		if (poll_file->wqh == wqh)
+			return;
+
+		mshv_vtl_poll_file_remove_wait_queue(poll_file);
+		poll_file->error = -EINVAL;
+		return;
+	}
+
 	poll_file->wqh = wqh;
 	add_wait_queue(wqh, &poll_file->wait);
 }
@@ -492,6 +504,7 @@ static int mshv_vtl_ioctl_set_poll_file(struct mshv_vtl_set_poll_file __user *us
 	struct file *file, *old_file;
 	struct mshv_vtl_poll_file *poll_file;
 	struct mshv_vtl_set_poll_file input;
+	int ret;
 
 	if (copy_from_user(&input, user_input, sizeof(input)))
 		return -EFAULT;
@@ -518,6 +531,7 @@ static int mshv_vtl_ioctl_set_poll_file(struct mshv_vtl_set_poll_file __user *us
 
 	old_file = poll_file->file;
 	poll_file->file = file;
+	poll_file->error = 0;
 	poll_file->cpu = input.cpu;
 
 	if (file) {
@@ -525,13 +539,18 @@ static int mshv_vtl_ioctl_set_poll_file(struct mshv_vtl_set_poll_file __user *us
 		init_poll_funcptr(&poll_file->pt, mshv_vtl_ptable_queue_proc);
 		vfs_poll(file, &poll_file->pt);
 	}
+	ret = poll_file->error;
+	if (ret)
+		poll_file->file = NULL;
 
 	mutex_unlock(&mshv_vtl_poll_file_lock);
 
 	if (old_file)
 		fput(old_file);
+	if (ret)
+		fput(file);
 
-	return 0;
+	return ret;
 }
 
 /* Static table mapping register names to their corresponding actions */
Signature · 1 line
2.55.0
Reviewed-by on the v2 series
#3 lore raw
In reply to Daehyeon Ko
Quoted 38 lines · Daehyeon Ko

MSHV_SET_POLL_FILE retains one embedded wait entry per CPU after vfs_poll() returns. Two independent bugs affect that state.

First, a provider such as signalfd may retire a non-file-owned waitqueue with POLLFREE. The MSHV callback leaves its entry linked and retains the queue pointer, allowing a later replacement to access freed provider memory. Patch 1 detaches on POLLFREE and introduces an RCU-safe removal helper.

Second, a poll callback may register more than one waitqueue. MSHV inserts the same embedded list node into each queue, corrupting the first list. Patch 2 rejects a second distinct queue and uses the common helper to unwind the first registration.

The roots, triggers, effects, and runtime oracles remain independent. Source-equivalent KASAN/LOCKDEP tests reproduced both bugs and validated the fixes. The local system is not a Hyper-V VTL2 environment, so the series has not been exercised through the real MSHV ioctl. OpenVMM does not exercise these paths in its current use of the interface.

An LLM assisted the source audits, fix development, and validation. Each patch notes that assistance in its changelog.

Changes in v2:
- combine the two previously standalone fixes into one series;
- rebase onto current Torvalds master;
- introduce the removal helper in patch 1 and reuse it in patch 2;
- replace the obsolete Assisted-by trailer with changelog prose.

Previous standalone submissions:
- POLLFREE v1: <[email protected]>
- multi-waitqueue v2, now withdrawn:

   <[email protected]>

Daehyeon Ko (2):
mshv_vtl: detach poll waiter on POLLFREE
mshv_vtl: reject poll files with multiple waitqueues

Quoted 5 lines
  drivers/hv/mshv_vtl_main.c | 51 ++++++++++++++++++++++++++++++++++----
  1 file changed, 46 insertions(+), 5 deletions(-)

base-commit: c03c1a14507dfaf8b856c2608f9ac216021fc222

I would suggest you to generally wait for a couple more days to collect other reviewers feedback as well, in case someone is reviewing your changes. You can then address them together in the next version.

For the series,

Regards,
Naman

guidance: https://docs.kernel.org/process/submitting-patches.html#don-t-get-discouraged-or-impatient

maintainer Applied
#4 lore raw
Quoted 45 lines · Naman Jain
Quoted 42 lines · Daehyeon Ko

MSHV_SET_POLL_FILE retains one embedded wait entry per CPU after vfs_poll() returns. Two independent bugs affect that state.

First, a provider such as signalfd may retire a non-file-owned waitqueue with POLLFREE. The MSHV callback leaves its entry linked and retains the queue pointer, allowing a later replacement to access freed provider memory. Patch 1 detaches on POLLFREE and introduces an RCU-safe removal helper.

Second, a poll callback may register more than one waitqueue. MSHV inserts the same embedded list node into each queue, corrupting the first list. Patch 2 rejects a second distinct queue and uses the common helper to unwind the first registration.

The roots, triggers, effects, and runtime oracles remain independent. Source-equivalent KASAN/LOCKDEP tests reproduced both bugs and validated the fixes. The local system is not a Hyper-V VTL2 environment, so the series has not been exercised through the real MSHV ioctl. OpenVMM does not exercise these paths in its current use of the interface.

An LLM assisted the source audits, fix development, and validation. Each patch notes that assistance in its changelog.

Changes in v2:
- combine the two previously standalone fixes into one series;
- rebase onto current Torvalds master;
- introduce the removal helper in patch 1 and reuse it in patch 2;
- replace the obsolete Assisted-by trailer with changelog prose.

Previous standalone submissions:
- POLLFREE v1: <[email protected]>
- multi-waitqueue v2, now withdrawn:

   <[email protected]>

Daehyeon Ko (2):
mshv_vtl: detach poll waiter on POLLFREE
mshv_vtl: reject poll files with multiple waitqueues

  drivers/hv/mshv_vtl_main.c | 51 ++++++++++++++++++++++++++++++++++----
  1 file changed, 46 insertions(+), 5 deletions(-)
Quoted 8 lines
Quoted 1 line

base-commit: c03c1a14507dfaf8b856c2608f9ac216021fc222

I would suggest you to generally wait for a couple more days to collect other reviewers feedback as well, in case someone is reviewing your changes. You can then address them together in the next version.

For the series,

Applied.

Wei