⬢ DragonFlyBSD Kernel Audit
DF-4454 / fix.diff
← back to finding ↓ download raw
--- a/sys/dev/misc/evdev/evdev.c
+++ b/sys/dev/misc/evdev/evdev.c
@@ -103,11 +103,41 @@
 static void evdev_stop_repeat(struct evdev_dev *);
 static int evdev_check_event(struct evdev_dev *, uint16_t, uint16_t, int32_t);
 
+/*
+ * evdev_master_lock serializes si_drv1 publication/teardown between
+ * evdev_cdev_create()/evdev_unregister() and evdev_open().  It guarantees
+ * that any thread which observes si_drv1 != NULL can safely take a
+ * reference on the evdev before the device is torn down.
+ * Lock order: evdev_master_lock -> evdev state lock.
+ */
+struct lock evdev_master_lock = LOCK_INITIALIZER("evdvmstr", 0, 0);
+
+void
+evdev_ref(struct evdev_dev *evdev)
+{
+
+	atomic_add_int(&evdev->ev_refs, 1);
+}
+
+void
+evdev_unref(struct evdev_dev *evdev)
+{
+
+	KKASSERT(evdev->ev_refs > 0);
+	if (atomic_fetchadd_int(&evdev->ev_refs, -1) == 1)
+		kfree(evdev, M_EVDEV);
+}
+
 struct evdev_dev *
 evdev_alloc(void)
 {
+	struct evdev_dev *evdev;
+
+	evdev = kmalloc(sizeof(struct evdev_dev), M_EVDEV, M_WAITOK | M_ZERO);
+	if (evdev != NULL)
+		evdev->ev_refs = 1;	/* caller's reference */
 
-	return kmalloc(sizeof(struct evdev_dev), M_EVDEV, M_WAITOK | M_ZERO);
+	return (evdev);
 }
 
 void
@@ -116,7 +146,12 @@
 	if (evdev) {
 		if (evdev->ev_cdev != NULL && evdev->ev_cdev->si_drv1 != NULL)
 			evdev_unregister(evdev);
-		kfree(evdev, M_EVDEV);
+		/*
+		 * Drop the allocation reference.  The memory is freed only
+		 * after the last open file descriptor (which holds its own
+		 * reference) has been closed.
+		 */
+		evdev_unref(evdev);
 	}
 }
 
@@ -372,8 +407,16 @@
 
 	sysctl_ctx_free(&evdev->ev_sysctl_ctx);
 
+	/*
+	 * Serialize with evdev_open(): NULL out si_drv1 and drop the cdev
+	 * reference under evdev_master_lock, so an open that observes a
+	 * non-NULL si_drv1 can still safely acquire its own reference.
+	 */
+	lockmgr(&evdev_master_lock, LK_EXCLUSIVE);
 	EVDEV_LOCK(evdev);
 	evdev->ev_cdev->si_drv1 = NULL;
+	evdev_unref(evdev);		/* drop the cdev reference */
+
 	/* Wake up sleepers */
 	LIST_FOREACH_MUTABLE(client, &evdev->ev_clients, ec_link, tmp) {
 		evdev_revoke_client(client);
@@ -386,16 +429,18 @@
 		}
 	}
 	EVDEV_UNLOCK(evdev);
+	lockmgr(&evdev_master_lock, LK_RELEASE);
 
 	ret = evdev_cdev_destroy(evdev);
 	evdev->ev_cdev = NULL;
-	if (ret == 0 && evdev->ev_lock_type == EV_LOCK_INTERNAL) {
-		lockuninit(&evdev->ev_mtx);
-	}
 
 	/*
 	 * For some devices, e.g. keyboards, ev_absinfo and ev_mt
 	 * may be NULL, so check before freeing them.
+	 *
+	 * NOTE: the evdev structure itself (and its state lock) must stay
+	 * valid until the last client fd is closed, so lockuninit() is
+	 * deferred to the final kfree in evdev_unref().
 	 */
 	if (evdev->ev_absinfo != NULL)
 	    evdev_free_absinfo(evdev->ev_absinfo);
--- a/sys/dev/misc/evdev/cdev.c
+++ b/sys/dev/misc/evdev/cdev.c
@@ -99,7 +99,7 @@
 evdev_open(struct dev_open_args *ap)
 {
 	cdev_t dev = ap->a_head.a_dev;
-	struct evdev_dev *evdev = dev->si_drv1;
+	struct evdev_dev *evdev;
 	struct evdev_client *client;
 	size_t buffer_size;
 	int ret;
@@ -110,8 +110,19 @@
 	if (caps_priv_check_self(SYSCAP_RESTRICTEDROOT))
 		return (EPERM);
 
-	if (evdev == NULL)
+	/*
+	 * Pin the evdev under evdev_master_lock so the sleeping allocation
+	 * below cannot race evdev_unregister()/evdev_free() (si_drv1 is
+	 * cleared and the cdev reference dropped under the same lock).
+	 */
+	lockmgr(&evdev_master_lock, LK_EXCLUSIVE);
+	evdev = dev->si_drv1;
+	if (evdev == NULL) {
+		lockmgr(&evdev_master_lock, LK_RELEASE);
 		return (ENODEV);
+	}
+	evdev_ref(evdev);
+	lockmgr(&evdev_master_lock, LK_RELEASE);
 
 	/* Initialize client structure */
 	buffer_size = evdev->ev_report_size * DEF_RING_REPORTS;
@@ -152,6 +163,8 @@
 
 	if (ret != 0) {
 		debugf(client, "cannot register evdev client");
+		/* Drop the reference taken above (client leak is DF-1631) */
+		evdev_unref(client->ec_evdev);
 	}
 
 	return (ret);
@@ -161,14 +174,26 @@
 evdev_dtor(void *data)
 {
 	struct evdev_client *client = (struct evdev_client *)data;
+	struct evdev_dev *evdev = client->ec_evdev;
 
-	EVDEV_LOCK(client->ec_evdev);
+	/*
+	 * Safe: every open file holds a reference on the evdev, so the
+	 * structure (and its state lock) stays allocated until the last
+	 * fd is closed even after the driver called evdev_free().
+	 */
+	EVDEV_LOCK(evdev);
 	if (!client->ec_revoked)
-		evdev_dispose_client(client->ec_evdev, client);
-	EVDEV_UNLOCK(client->ec_evdev);
+		evdev_dispose_client(evdev, client);
+	EVDEV_UNLOCK(evdev);
 
 	funsetown(&client->ec_sigio);
 	lockuninit(&client->ec_buffer_mtx);
+	/*
+	 * Drop the reference while `client' is still allocated (the
+	 * compiler is free to sink a load past kfree(), so the unref
+	 * MUST NOT read client fields after the free).
+	 */
+	evdev_unref(evdev);	/* may free the evdev */
 	kfree(client, M_EVDEV);
 }
 
@@ -255,6 +280,14 @@
 	}
 
 	while (uio->uio_resid > 0 && ret == 0) {
+		/*
+		 * The device may be revoked (driver detached) mid-write;
+		 * stop touching the evdev immediately (DF-4454).
+		 */
+		if (client->ec_revoked) {
+			ret = ENODEV;
+			break;
+		}
 		ret = uiomove((void *)&event, sizeof(struct input_event), uio);
 		if (ret == 0)
 			ret = evdev_inject_event(evdev, event.type, event.code,
@@ -716,6 +749,7 @@
 
 	if (dev != NULL) {
 		dev->si_drv1 = evdev;
+		evdev_ref(evdev);	/* reference owned by si_drv1 */
 		evdev->ev_cdev = dev;
 		evdev->ev_unit = unit;
 		ret = 0;
--- a/sys/dev/misc/evdev/evdev_private.h
+++ b/sys/dev/misc/evdev/evdev_private.h
@@ -133,6 +133,9 @@
 	uint64_t		ev_event_count;			/* (s) */
 	uint64_t		ev_report_count;		/* (s) */
 
+	/* Lifetime: reference count pinning the struct past unregister */
+	volatile u_int		ev_refs;			/* (m) */
+
 	/* Parent driver callbacks: */
 	const struct evdev_methods * ev_methods;
 	void *			ev_softc;
@@ -209,6 +212,9 @@
 bool evdev_event_supported(struct evdev_dev *, uint16_t);
 void evdev_set_abs_bit(struct evdev_dev *, uint16_t);
 void evdev_set_absinfo(struct evdev_dev *, uint16_t, struct input_absinfo *);
+void evdev_ref(struct evdev_dev *);
+void evdev_unref(struct evdev_dev *);
+extern struct lock evdev_master_lock;
 
 /* Client interface: */
 int evdev_register_client(struct evdev_dev *, struct evdev_client *);