[PATCH] misc: nsm: keep the device state until the last file is closed

[PATCH] misc: nsm: keep the device state until the last file is closed

[Chad Aldreda]

nsm_device_probe() allocates struct nsm with devm_kzalloc(), so it is
freed when the device is removed, but an open /dev/nsm keeps using it
through file->private_data. When the virtio device is removed while the
file is open, for example by a hot-unplug, the next NSM_IOCTL_RAW runs
on the freed struct nsm and queues buffers on the virtqueue that
remove() deleted. KASAN reports a slab-use-after-free in
virtqueue_add_outbuf(), called from nsm_sendrecv_msg_locked().

Allocate struct nsm with kzalloc() and free it on the last reference:
open() takes a reference and release() drops it, and remove() drops the
device's own reference after misc_deregister(). misc_open() calls
open() with misc_mtx held, and misc_deregister() takes misc_mtx before
remove() drops that reference, so open() always runs while the device's
reference is held.

remove() now marks the device as removed and deletes the virtqueue with
nsm->lock held, so a later NSM_IOCTL_RAW fails with -ENODEV once its
argument struct has been copied in. To take the lock, remove() waits for
an ioctl already in progress, which before this patch could still be
using the virtqueue while it was deleted. That wait has no fixed bound
while the ioctl copies to or from user memory. If the device was
surprise-removed while handling the request, it lasts up to
NSM_DEFAULT_TIMEOUT_MSECS (two minutes), after the existing wait in
hwrng_unregister().

Fixes: b9873755a6c8 ("misc: Add Nitro Secure Module driver")
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 a virtio-nsm-pci device on a PCIe root
port. A guest process opens /dev/nsm, the device is removed with
device_del from the QEMU monitor, and the process then issues
NSM_IOCTL_RAW. Without this patch KASAN reports the use-after-free of
the virtqueue; with it the ioctl returns -ENODEV and KASAN reports
nothing. Not tested on Nitro hardware. (QEMU needs module-id= set on
the device, or it aborts when the device is unplugged.)

Yuho Choi's pending "misc: nsm: reset device before deleting the
virtqueue on removal" (2026-10-16) changes the same function. The
reproducer also hits the WARN_ON_ONCE() in virtio_dev_remove() that
patch fixes, the check commented "Driver should have reset device". The
two patches are independent. If that one is applied first I can rebase
on it; the reset would then go inside the locked section, before
del_vqs().

kzalloc() is used rather than kzalloc_obj() so that the fix applies to
stable kernels before v7.0.

Not done here: waking a request that waits on a broken virtqueue, so a
surprise removal does not wait out the full timeout, and copying user
buffers outside nsm->lock.

 drivers/misc/nsm.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 42 insertions(+), 1 deletion(-)

diff --git a/drivers/misc/nsm.c b/drivers/misc/nsm.c
index b6d3fa79a5e8..08c0997fb308 100644
--- a/drivers/misc/nsm.c
+++ b/drivers/misc/nsm.c
@@ -13,6 +13,7 @@
 #include <linux/fs.h>
 #include <linux/interrupt.h>
 #include <linux/hw_random.h>
+#include <linux/kref.h>
 #include <linux/miscdevice.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
@@ -55,6 +56,8 @@ struct nsm {
 	struct miscdevice     misc;
 	struct hwrng          hwrng;
 	struct work_struct    misc_init;
+	struct kref           kref;
+	bool                  removed;	/* protected by lock */
 	struct nsm_msg        msg;
 };
 
@@ -372,6 +375,12 @@ static long nsm_dev_ioctl(struct file *file, unsigned int cmd,
 
 	mutex_lock(&nsm->lock);
 
+	/* The device may have been removed while the file was open */
+	if (nsm->removed) {
+		r = -ENODEV;
+		goto out;
+	}
+
 	/* Convert kernel argument struct to device request */
 	r = fill_req_raw(nsm, &nsm->msg.req, &raw);
 	if (r)
@@ -413,8 +422,31 @@ static int nsm_device_init_vq(struct virtio_device *vdev)
 	return 0;
 }
 
+static void nsm_free(struct kref *kref)
+{
+	kfree(container_of(kref, struct nsm, kref));
+}
+
+/*
+ * misc_open() calls this under misc_mtx, which misc_deregister() also takes,
+ * so the device's own reference is still held here.
+ */
+static int nsm_dev_open(struct inode *inode, struct file *file)
+{
+	kref_get(&file_to_nsm(file)->kref);
+	return 0;
+}
+
+static int nsm_dev_release(struct inode *inode, struct file *file)
+{
+	kref_put(&file_to_nsm(file)->kref, nsm_free);
+	return 0;
+}
+
 static const struct file_operations nsm_dev_fops = {
 	.owner = THIS_MODULE,
+	.open = nsm_dev_open,
+	.release = nsm_dev_release,
 	.unlocked_ioctl = nsm_dev_ioctl,
 	.compat_ioctl = compat_ptr_ioctl,
 };
@@ -426,10 +458,12 @@ static int nsm_device_probe(struct virtio_device *vdev)
 	struct nsm *nsm;
 	int rc;
 
-	nsm = devm_kzalloc(&vdev->dev, sizeof(*nsm), GFP_KERNEL);
+	nsm = kzalloc(sizeof(*nsm), GFP_KERNEL);
 	if (!nsm)
 		return -ENOMEM;
 
+	kref_init(&nsm->kref);
+
 	vdev->priv = nsm;
 	nsm->vdev = vdev;
 
@@ -478,6 +512,7 @@ static int nsm_device_probe(struct virtio_device *vdev)
 	virtio_reset_device(vdev);
 	vdev->config->del_vqs(vdev);
 err_init_vq:
+	kfree(nsm);
 	return rc;
 }
 
@@ -488,8 +523,14 @@ static void nsm_device_remove(struct virtio_device *vdev)
 
 	hwrng_unregister(&nsm->hwrng);
 
+	/* Wait for an ioctl in progress, and fail later ones */
+	mutex_lock(&nsm->lock);
+	nsm->removed = true;
 	vdev->config->del_vqs(vdev);
+	mutex_unlock(&nsm->lock);
+
 	misc_deregister(&nsm->misc);
+	kref_put(&nsm->kref, nsm_free);
 }
 
 /* NSM device configuration structure */

base-commit: d48723711bae9b8cdc56719c448ba42fef28cbee
-- 
2.43.0