[PATCH net v2] ptp: vmclock: keep the device state until the last file is closed

[Chad Aldreda <ca@modelmiser.com>]

vmclock_probe() allocates struct vmclock_state with devm_kzalloc() and
maps the shared page with devm_memremap(), so both are released when
the driver is unbound, but an open /dev/vmclockN keeps using them
through its file state. After an unbind, read() runs on the freed
struct and reads through the released mapping, and a poll() in
progress sleeps on a wait queue in the freed struct. KASAN reports a
slab-use-after-free in vmclock_miscdev_read(). In testing, the same
read() faulted on the unmapped page once the device had been bound
again.

Allocate the state with kzalloc_obj() and map the page with memremap(),
and release both on the last reference: open() takes a reference and
release() drops it, and a devm action drops the device's own reference
after vmclock_remove() has called misc_deregister(). misc_open() calls
open() with misc_mtx held, and misc_deregister() takes misc_mtx, so
open() always runs while the device's reference is held.

vmclock_remove() now marks the device as removed and wakes every waiter
on the wait queue. A read() or mmap() on a file that is still open fails
with -ENODEV once it sees the mark. On a device that supports
notifications, poll() returns EPOLLHUP | EPOLLERR, including a poll()
that was already waiting; it tests the mark after poll_wait(), so it
cannot miss the wake-up. The wake-up uses wake_up_interruptible_all(),
because no later notification will reach a waiter that the remove-time
wake-up skips, such as a second epoll instance that registered the file
with EPOLLEXCLUSIVE. Without notifications poll() already returns
POLLHUP.

Separately from the report below, the bug was also found by a
Coccinelle script that reports device nodes in devm-allocated memory,
posted at
https://lore.kernel.org/r/20261106193517.3267013-2-ca@modelmiser.com,
and reproduced in QEMU 10.2.4 with -device vmclock. There is no access
to EC2 hardware with this device, so the fix has only been tested in
QEMU. QEMU's device has no notifications, so the poll() path was tested
with a local change that lets poll() wait without them.

Fixes: 205032724226 ("ptp: Add support for the AMZNC10C 'vmclock' device")
Reported-by: Shuangpeng Bai <shuangpeng.kernel@gmail.com>
Closes: https://lore.kernel.org/all/178144969601.60470.14493569608271069160@gmail.com/
Cc: stable@vger.kernel.org # 7.0+
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Chad Aldreda <ca@modelmiser.com>
---
v2:
- vmclock_remove() wakes the wait queue with wake_up_interruptible_all()
  instead of wake_up_interruptible(), so every epoll waiter gets
  EPOLLHUP | EPOLLERR at remove, not only the first EPOLLEXCLUSIVE one.
  Pointed out by Paolo Abeni, from the Sashiko review:
  https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261107150344.3946529-2-ca@modelmiser.com
  The two notification wake-ups are unchanged.
- The commit message says how the bug was found and tested.
- Cc: stable now carries "# 7.0+" (see below).
- Rebased on net/main 19ec8416a76c; no other change.

v1: https://lore.kernel.org/r/20261107150344.3946529-2-ca@modelmiser.com

Testing, all in QEMU 10.2.4 with -device vmclock, KASAN enabled:
- As in v1: a guest process opens /dev/vmclock0, unbinds the driver
  through /sys/bus/platform/drivers/vmclock/unbind, and reads the open
  file again. Without this patch KASAN reports the slab-use-after-free;
  if the device is bound again before the second read, that read oopses
  on the unmapped page. With the patch the read returns -ENODEV, KASAN
  reports nothing, and the rebound device reads normally.
- New in v2, for the wake-up: three threads each add the same open file
  to their own epoll instance and block in epoll_wait(), then the
  driver is unbound. QEMU's device does not set
  VMCLOCK_FLAG_NOTIFICATION_PRESENT, so for this test only, a local
  change (not part of this patch) made poll() take the waiting path
  without that flag. Before the unbind, the test reads each waiter's
  /proc/self/task/<tid>/wchan and counts the run only if all three are
  asleep in do_epoll_wait. With v1 and EPOLLEXCLUSIVE, one waiter got
  EPOLLHUP | EPOLLERR and the other two slept until their 8 s timeout,
  in each of 4 runs; with v2 all three woke at the unbind, in each of 4
  runs. Without EPOLLEXCLUSIVE all three woke with either version.
  KASAN reported nothing in any of these runs.
- Not tested: EC2 hardware, a real notification, and mmap() after
  unbind.

This applies to v7.0 and later, where the per-file state was added.
Earlier kernels have the same bug through the devm-allocated state and
need a separate backport.

Lorenzo Stoakes' "ptp: vmclock: convert mmap hook to mmap_prepare"
(2026-11-05) changes vmclock_miscdev_mmap(). If it lands first I can
rebase.

Not changed here: in vmclock_probe(), !st->miscdev.minor is never true,
because an unregistered dynamic miscdevice keeps MISC_DYNAMIC_MINOR, so
a device with neither interface still binds. That is a separate fix.

 drivers/ptp/ptp_vmclock.c | 62 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 54 insertions(+), 8 deletions(-)

diff --git a/drivers/ptp/ptp_vmclock.c b/drivers/ptp/ptp_vmclock.c
index bb0e14bac9f2..ea94a73ac52e 100644
--- a/drivers/ptp/ptp_vmclock.c
+++ b/drivers/ptp/ptp_vmclock.c
@@ -17,6 +17,7 @@
 #include <linux/io.h>
 #include <linux/interrupt.h>
 #include <linux/kernel.h>
+#include <linux/kref.h>
 #include <linux/miscdevice.h>
 #include <linux/mm.h>
 #include <linux/module.h>
@@ -51,6 +52,8 @@ struct vmclock_state {
 	enum clocksource_ids cs_id, sys_cs_id;
 	int index;
 	char *name;
+	struct kref kref;
+	bool removed;
 };
 
 #define VMCLOCK_MAX_WAIT ms_to_ktime(100)
@@ -369,6 +372,9 @@ static int vmclock_miscdev_mmap(struct file *fp, struct vm_area_struct *vma)
 	struct vmclock_file_state *fst = fp->private_data;
 	struct vmclock_state *st = fst->st;
 
+	if (READ_ONCE(st->removed))
+		return -ENODEV;
+
 	if ((vma->vm_flags & (VM_READ|VM_WRITE)) != VM_READ)
 		return -EROFS;
 
@@ -398,6 +404,9 @@ static ssize_t vmclock_miscdev_read(struct file *fp, char __user *buf,
 	uint32_t seq, old_seq;
 	size_t max_count;
 
+	if (READ_ONCE(st->removed))
+		return -ENODEV;
+
 	if (*ppos >= PAGE_SIZE)
 		return 0;
 
@@ -450,6 +459,10 @@ static __poll_t vmclock_miscdev_poll(struct file *fp, poll_table *wait)
 
 	poll_wait(fp, &st->disrupt_wait, wait);
 
+	/* After poll_wait(), so the wake-up in vmclock_remove() is not missed */
+	if (READ_ONCE(st->removed))
+		return EPOLLHUP | EPOLLERR;
+
 	seq = le32_to_cpu(st->clk->seq_count);
 	if (atomic_read(&fst->seq) != seq)
 		return POLLIN | POLLRDNORM;
@@ -466,6 +479,12 @@ static int vmclock_miscdev_open(struct inode *inode, struct file *fp)
 	if (!fst)
 		return -ENOMEM;
 
+	/*
+	 * misc_open() calls this with misc_mtx held, and vmclock_remove()
+	 * calls misc_deregister(), which takes misc_mtx, before the device's
+	 * own reference is dropped.
+	 */
+	kref_get(&st->kref);
 	fst->st = st;
 	atomic_set(&fst->seq, 0);
 
@@ -474,9 +493,21 @@ static int vmclock_miscdev_open(struct inode *inode, struct file *fp)
 	return 0;
 }
 
+static void vmclock_free(struct kref *kref)
+{
+	struct vmclock_state *st = container_of(kref, struct vmclock_state, kref);
+
+	if (st->clk)
+		memunmap(st->clk);
+	kfree(st);
+}
+
 static int vmclock_miscdev_release(struct inode *inode, struct file *fp)
 {
-	kfree(fp->private_data);
+	struct vmclock_file_state *fst = fp->private_data;
+
+	kref_put(&fst->st->kref, vmclock_free);
+	kfree(fst);
 	return 0;
 }
 
@@ -649,9 +680,20 @@ static void vmclock_remove(void *data)
 	if (st->miscdev.minor != MISC_DYNAMIC_MINOR)
 		misc_deregister(&st->miscdev);
 
+	/* Files still open now fail, and a poll() in progress returns */
+	WRITE_ONCE(st->removed, true);
+	wake_up_interruptible_all(&st->disrupt_wait);
+
 	dev->driver_data = NULL;
 }
 
+static void vmclock_put(void *data)
+{
+	struct vmclock_state *st = data;
+
+	kref_put(&st->kref, vmclock_free);
+}
+
 static void vmclock_put_idx(void *data)
 {
 	struct vmclock_state *st = data;
@@ -665,9 +707,15 @@ static int vmclock_probe(struct platform_device *pdev)
 	struct vmclock_state *st;
 	int ret;
 
-	st = devm_kzalloc(dev, sizeof(*st), GFP_KERNEL);
+	/* Open files can outlive the device, so st is not devm memory */
+	st = kzalloc_obj(*st);
 	if (!st)
 		return -ENOMEM;
+	kref_init(&st->kref);
+
+	ret = devm_add_action_or_reset(dev, vmclock_put, st);
+	if (ret)
+		return ret;
 
 #if IS_ENABLED(CONFIG_ACPI)
 	if (has_acpi_companion(dev))
@@ -686,13 +734,11 @@ static int vmclock_probe(struct platform_device *pdev)
 			 resource_size(&st->res));
 		return -EINVAL;
 	}
-	st->clk = devm_memremap(dev, st->res.start, resource_size(&st->res),
-				MEMREMAP_WB | MEMREMAP_DEC);
-	if (IS_ERR(st->clk)) {
-		ret = PTR_ERR(st->clk);
+	st->clk = memremap(st->res.start, resource_size(&st->res),
+			   MEMREMAP_WB | MEMREMAP_DEC);
+	if (!st->clk) {
 		dev_info(dev, "failed to map shared memory\n");
-		st->clk = NULL;
-		return ret;
+		return -ENXIO;
 	}
 
 	if (le32_to_cpu(st->clk->magic) != VMCLOCK_MAGIC ||
-- 
2.43.0