⬢ DragonFlyBSD Kernel Audit
DF-4506 / fix.diff
← back to finding ↓ download raw
fix.diff — DF-4506 — defer kfree of IDR node arrays
=====================================================

Root cause: sys/libkern/linux_idr.c idr_find() is a lockless reader, but
idr_grow()/idr_destroy() kfree() the node array synchronously.  The drm
linux-compat layer calls idr_find() under rcu_read_lock(), which is a no-op
on DragonFly (sys/dev/drm/include/linux/rcupdate.h:46-50 -> cpu_ccfence()),
so there is no grace period protecting the reader; it can dereference the
freed array (e.g. the i915_gem_context.c:831/855/901/981 lookups racing
i915_gem_context.c:355 idr_alloc -> idr_grow).

Fix: mirror the convention the shim itself already uses for kfree_rcu()
(a ~1s delay line, sys/dev/drm/linux_rcu.c) and defer the free of the old
node array by 1 second.  A file-scoped alternative (take dev->struct_mutex
around the i915 context lookups) is noted in the finding; the shim-side fix
protects every lockless idr user at once.

git apply from the repository root (never applied to sys/ by the auditor).

--- a/sys/libkern/linux_idr.c	2026-09-20 22:40:38.494056050 +0000
+++ b/sys/libkern/linux_idr.c	2026-09-20 22:41:30.289385312 +0000
@@ -120,6 +120,7 @@
 #include <sys/idr.h>
 #include <sys/kernel.h>
 #include <sys/libkern.h>
+#include <sys/callout.h>
 #include <sys/malloc.h>
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -133,6 +134,28 @@
 
 MALLOC_DEFINE(M_IDR, "idr", "Integer ID management");
 
+struct idr_dfree {
+	struct callout	callout;
+	void		*nodes;
+};
+
+/*
+ * DF-4506: idr_find() is a lockless reader.  The drm linux-compat layer
+ * calls it under rcu_read_lock(), which is a no-op on DragonFly and so
+ * provides no grace period.  Freeing the old node array synchronously
+ * from idr_grow()/idr_destroy() therefore lets a reader dereference
+ * freed memory.  Mirror the shim's kfree_rcu() convention (a ~1s delay
+ * line, sys/dev/drm/linux_rcu.c) and defer the free.
+ */
+static void
+idr_delayed_free(void *arg)
+{
+	struct idr_dfree *df = arg;
+
+	kfree(df->nodes, M_IDR);
+	kfree(df, M_IDR);
+}
+
 static void	idr_grow(struct idr *idp, int want);
 static void	idr_reserve(struct idr *idp, int id, int incr);
 static int	idr_find_free(struct idr *idp, int want, int lim);
@@ -455,7 +478,16 @@
 	idp->idr_count = nf;
 
 	if (oldnodes) {
-		kfree(oldnodes, M_IDR);
+		struct idr_dfree *df;
+
+		df = kmalloc(sizeof(*df), M_IDR, M_WAITOK | M_ZERO);
+		if (df) {
+			df->nodes = oldnodes;
+			callout_init_mp(&df->callout);
+			callout_reset(&df->callout, hz, idr_delayed_free, df);
+		} else {
+			kfree(oldnodes, M_IDR);
+		}
 	}
 	idp->idr_nexpands++;
 }
@@ -506,7 +538,16 @@
 {
 	lwkt_token_uninit(&idp->idr_token);
 	if (idp->idr_nodes) {
-		kfree(idp->idr_nodes, M_IDR);
+		struct idr_dfree *df;
+
+		df = kmalloc(sizeof(*df), M_IDR, M_WAITOK | M_ZERO);
+		if (df) {
+			df->nodes = idp->idr_nodes;
+			callout_init_mp(&df->callout);
+			callout_reset(&df->callout, hz, idr_delayed_free, df);
+		} else {
+			kfree(idp->idr_nodes, M_IDR);
+		}
 		idp->idr_nodes = NULL;
 	}
 	bzero(idp, sizeof(*idp));