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

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

[Chad Aldreda]

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 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. Without notifications poll() already returns POLLHUP.

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
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Chad Aldreda <ca@modelmiser.com>
---
Reproduced in QEMU 10.2.4 with -device vmclock. 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. Not tested on EC2.
QEMU's device does not set VMCLOCK_FLAG_NOTIFICATION_PRESENT, so the
poll() change was not exercised, and mmap() after unbind was not tested.

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..3584a20f681d 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(&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