From: Chad Aldreda <ca@modelmiser.com>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
"Rafael J. Wysocki" <rafael@kernel.org>,
Danilo Krummrich <dakr@kernel.org>,
Johan Hovold <johan@kernel.org>,
driver-core@lists.linux.dev, linux-media@vger.kernel.org,
Mauro Carvalho Chehab <mchehab@kernel.org>,
Hans Verkuil <hverkuil+cisco@kernel.org>,
Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
Nick Faro <yux50000@hotmail.com>,
Jonathan Corbet <corbet@lwn.net>,
linux-doc@vger.kernel.org
Subject: [RFC PATCH 2/4] devres: add DEBUG_DEVRES_REFS to warn about managed memory still held by open files
Date: Wed, 04 Nov 2026 22:00:47 +0000 [thread overview]
Message-ID: <20261104220041.3994559-1-ca@modelmiser.com> (raw)
In-Reply-To: <20261104215546.3990950-1-ca@modelmiser.com>
Memory from devm_kzalloc() is freed when the driver is unbound, but a
device node that user space holds open outlives the unbind. Driver
state that an open file can reach, such as an embedded struct cdev,
video_device or miscdevice, must therefore not be managed memory; when
it is, the next file operation, typically close(), is a use-after-free.
The pattern is long-standing and is still fixed driver by driver. In
mainline today cx231xx allocates its state, with four video_devices
and its mutex embedded, with devm_kzalloc() against the USB interface,
defers its release to close() when a node is open, and devres frees the
state at unbind regardless; xilinx_hwicap embeds its struct cdev in
devm memory the same way.
Add a runtime debug check rather than a static rule: from
device_unbind_cleanup(), with device_lock held, before
devres_release_all(), snapshot the device's managed allocations, walk
every thread's open files once, and warn when a file's private_data,
its inode's i_private, its inode's cdev for a character device, or an
object a subsystem names through a small hook still points into an
allocation about to be freed. Such a pointer dangles once the memory
is freed; it is a use-after-free if anything dereferences it
afterwards, which for private_data is any operation, for a cdev its
release on close, and for a debugfs file a new open or an unguarded
release. One WARN backtrace is printed per allocation; further holders
of it get one line each.
The cdev route works because an open non-O_PATH chrdev file holds a
reference on its cdev's kobject from chrdev_open(), and inode->i_cdev
is cleared only when that kobject is released, so the pointer is
stable here even after cdev_del(); it is compared, never dereferenced.
It only helps when the struct cdev itself is in managed memory: V4L2's
cdev is a separate cdev_alloc() with no kobject parent and misc's is
one shared cdev, so neither leads to the driver's allocation, which is
what the hook is for. The next patch adds V4L2's.
debugfs files need care, because many drivers remove theirs through a
devres action that runs inside devres_release_all(). A hit on a debugfs
file that remove() already took down is decided at once; a hit on a
live one is recorded as (pid, fd, file address) and looked up again
after release. If debugfs has revoked the file by then
(debugfs_file_revoked()) and its release callback is one that cannot
touch the data (debugfs_file_release_safe()), it is reported at info
level; if the release is the driver's own it is a WARN naming it, and
so is a file still live or removed with unguarded file_operations.
The check runs only from the unbind and failed-probe paths, not on
probe deferral and never from devres_release_all() itself, which is
also reached from device_release() and so from any context in which a
final put_device() is legal. The walks run on a kernel thread the
unbinding task waits for: a file reference the walk holds becomes the
last one if the owner closes the file meanwhile, and dropped from a
user task its release would be queued to that task's task work, after
unbind, where a devres action waiting for the close (snd_card_free())
would deadlock under device_lock; dropped from a kernel thread it goes
to the delayed-fput work instead. What remains: that work drains one
global list last-in-first-out with all its runs serialized, so a
release there that needs a lock unbind holds (a usbfs close takes the
USB parent lock a USB interface unbind holds) stalls every later
delayed close, including one such an action is waiting for; without
this option that close would have run from its owner's task work. No
file reference is kept across release. Fd tables are walked once each,
de-duplicated by (owning task, table pointer) with a reference on the
task only; no reference is ever taken on another task's fd table, since
dropping the last one would run that task's file releases in the walk.
Limitations, also stated in the Kconfig help: only files in task fd
tables are seen; the check finds pointers that are dangling at that
moment, not teardown races, nor whether a debugfs removal action ran
before or after the free; a managed allocation whose release waits for
its files to close first (an ALSA card) is not about to be freed, so a
hit on it is a false positive; and the walk is linear in threads times
fd tables and costs every unbind and failed probe.
Written with Claude (Anthropic) under my direction; see the cover letter
for the disclosure required by Documentation/process/generated-content.rst.
Signed-off-by: Chad Aldreda <ca@modelmiser.com>
---
.../driver-api/driver-model/devres.rst | 18 +
drivers/base/Kconfig | 64 ++
drivers/base/base.h | 38 +
drivers/base/dd.c | 14 +-
drivers/base/devres.c | 667 ++++++++++++++++++
include/linux/device/devres.h | 58 ++
6 files changed, 855 insertions(+), 4 deletions(-)
diff --git a/Documentation/driver-api/driver-model/devres.rst b/Documentation/driver-api/driver-model/devres.rst
index 4f0b90e7e1..62a70cc9ba 100644
--- a/Documentation/driver-api/driver-model/devres.rst
+++ b/Documentation/driver-api/driver-model/devres.rst
@@ -221,6 +221,24 @@ in lib/devres.c.
All devres interface functions can be called without context if the
right gfp mask is given.
+Managed memory is freed on driver unbind regardless of whether anything
+else still refers to it. In particular a device node that user space
+holds open outlives the unbind, so driver state that an open file can
+reach - a struct cdev, video_device or miscdevice embedded in it - must
+not live in devm_kzalloc()'d memory. Allocate such state with kzalloc()
+and free it from the release callback of the object that user space
+holds, or from a refcount dropped on last close. debugfs private data
+may be managed memory only if the file is created through
+debugfs_create_file() or one of debugfs's typed helpers such as
+debugfs_create_u32() or debugfs_create_blob(), not through
+debugfs_create_file_unsafe() with the driver's own file_operations, so
+that debugfs refuses operations on it once removed, and the file is
+removed before the allocation is freed: in the remove callback, or by a
+devres action registered after that allocation.
+CONFIG_DEBUG_DEVRES_REFS reports, at driver unbind or after a failed
+probe, open files that still point into a managed allocation about to
+be freed; its Kconfig help text lists what it can and cannot see.
+
5. Overhead
-----------
diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
index a9acf4575f..4b1adf4a06 100644
--- a/drivers/base/Kconfig
+++ b/drivers/base/Kconfig
@@ -159,6 +159,70 @@ config DEBUG_DEVRES
If you are unsure about this, Say N here.
+config DEBUG_DEVRES_REFS
+ bool "Warn about managed memory still held by open files at unbind"
+ depends on DEBUG_KERNEL
+ help
+ Before a driver's managed (devm) allocations are freed, on unbind
+ or after a failed probe, walk the open files of every thread and
+ WARN if one still points into an allocation about to be freed:
+ through file->private_data, the inode's i_private, the inode's
+ cdev for a character device, or an object a subsystem names
+ through a hook (V4L2 names the video_device). Such a pointer
+ dangles once the memory is freed; it is a use-after-free if
+ anything dereferences it afterwards: for private_data any
+ operation that uses it, for a cdev a new open of the device node
+ and its release on close, for a hook object whatever the
+ subsystem's operations do with it (for V4L2 any operation and
+ close), and for a debugfs file a new open or an unguarded
+ release. One WARN backtrace (which
+ taints the kernel) is printed per allocation per unbind; further
+ holders of the same allocation get one line each.
+
+ A removed debugfs file on which debugfs refuses further operations
+ (one created through its full proxy, or through one of its own
+ guarded helpers such as debugfs_create_ulong() or
+ debugfs_create_blob()) is reported at info level if its release
+ callback is one that cannot touch the data, and warned about
+ naming the callback otherwise. A debugfs file still live during
+ the walk is decided after the managed resources were released,
+ since drivers often remove their debugfs files through a devres
+ action: removed and safe to close by then is info, anything else
+ is a WARN. A file created with a debugfs _unsafe variant and the
+ driver's own file_operations gets no guard; it is deferred like a
+ live debugfs file and then warned about.
+
+ The walks run on a kernel thread, so a file reference the walk
+ turns out to hold last is released from the delayed-fput work and
+ never from the unbinding task, so a devres action that waits for
+ a file to close does not wait on the walk. What remains: that
+ work drains one global list last-in-first-out with all its runs
+ serialized, so a release there that needs a lock unbind holds (a
+ usbfs close takes the USB parent lock a USB interface unbind
+ holds) stalls every later delayed close, including one such an
+ action is waiting for; without this option that close would have
+ run from its owner's task work. No file reference is kept across
+ the release of the managed resources.
+
+ Limitations: only files in task fd tables are seen, so references
+ held by sockets, epoll, io_uring or kernel-internal files are not;
+ the check runs from the unbind and failed-probe paths only, not on
+ probe deferral nor on other devres release paths; a V4L2 driver's
+ state reached only through the video_device's drvdata is not
+ named, since that needs dereferencing the video_device; it finds
+ pointers
+ that are dangling at that moment, not teardown races, nor whether
+ a debugfs removal action ran before or after the free; a managed
+ allocation whose release waits for its files to close first (an
+ ALSA card) is not about to be freed, so a hit on it is a false
+ positive; a driver that keeps freeing devm memory after remove()
+ returned can make it misreport. The walk is linear in threads
+ times distinct fd tables and costs every unbind and failed probe.
+
+ This is a slow debug check intended for test systems.
+
+ If you are unsure about this, say N here.
+
config DEBUG_TEST_DRIVER_REMOVE
bool "Test driver remove calls during probe (UNSTABLE)"
depends on DEBUG_KERNEL
diff --git a/drivers/base/base.h b/drivers/base/base.h
index 866b2beb6b..89802070c3 100644
--- a/drivers/base/base.h
+++ b/drivers/base/base.h
@@ -10,6 +10,7 @@
* shared outside of the drivers/base/ directory.
*
*/
+#include <linux/completion.h>
#include <linux/notifier.h>
/**
@@ -242,6 +243,43 @@ void devres_for_each_res(struct device *dev, dr_release_t release,
void (*fn)(struct device *, void *, void *),
void *data);
int devres_release_all(struct device *dev);
+/* CONFIG_DEBUG_DEVRES_REFS state carried across devres_release_all() */
+struct devres_seen_table {
+ struct task_struct *task;
+ struct files_struct *files;
+};
+
+struct devres_open_file_check {
+#ifdef CONFIG_DEBUG_DEVRES_REFS
+ struct device *dev;
+ struct devres_region *regions;
+ unsigned int nr;
+ struct devres_open_file_hook **hooks;
+ unsigned int nhooks;
+ struct devres_seen_table *seen;
+ unsigned int nseen, maxseen;
+ struct devres_pending_ref *pending;
+ unsigned int npending, maxpending;
+ struct completion done;
+#endif
+};
+
+#ifdef CONFIG_DEBUG_DEVRES_REFS
+void devres_check_open_files(struct device *dev,
+ struct devres_open_file_check *chk);
+void devres_finish_open_file_check(struct device *dev,
+ struct devres_open_file_check *chk);
+#else
+static inline void devres_check_open_files(struct device *dev,
+ struct devres_open_file_check *chk)
+{
+}
+
+static inline void devres_finish_open_file_check(struct device *dev,
+ struct devres_open_file_check *chk)
+{
+}
+#endif
void device_block_probing(void);
void device_unblock_probing(void);
void deferred_probe_extend_timeout(void);
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 5212325196..19a664feb9 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -591,11 +591,16 @@ static ssize_t state_synced_show(struct device *dev,
}
static DEVICE_ATTR_RW(state_synced);
-static void device_unbind_cleanup(struct device *dev)
+static void device_unbind_cleanup(struct device *dev, bool check_open_files)
{
+ struct devres_open_file_check chk = {};
+
if (dev->driver->p_cb.post_unbind_rust)
dev->driver->p_cb.post_unbind_rust(dev);
+ if (check_open_files)
+ devres_check_open_files(dev, &chk);
devres_release_all(dev);
+ devres_finish_open_file_check(dev, &chk);
arch_teardown_dma_ops(dev);
kfree(dev->dma_range_map);
dev->dma_range_map = NULL;
@@ -744,7 +749,7 @@ static int really_probe(struct device *dev, const struct device_driver *drv)
driver_sysfs_remove(dev);
if (dev->bus && dev->bus->dma_cleanup)
dev->bus->dma_cleanup(dev);
- device_unbind_cleanup(dev);
+ device_unbind_cleanup(dev, true);
goto re_probe;
}
@@ -770,7 +775,8 @@ static int really_probe(struct device *dev, const struct device_driver *drv)
dev->bus->dma_cleanup(dev);
pinctrl_bind_failed:
device_links_no_driver(dev);
- device_unbind_cleanup(dev);
+ /* A driver's own error arrives here negated (see above); either sign is a deferral. */
+ device_unbind_cleanup(dev, ret != -EPROBE_DEFER && ret != EPROBE_DEFER);
done:
return ret;
}
@@ -1359,7 +1365,7 @@ static void __device_release_driver(struct device *dev, struct device *parent)
if (dev->bus && dev->bus->dma_cleanup)
dev->bus->dma_cleanup(dev);
- device_unbind_cleanup(dev);
+ device_unbind_cleanup(dev, true);
device_links_driver_cleanup(dev);
klist_remove(&dev->p->knode_driver);
diff --git a/drivers/base/devres.c b/drivers/base/devres.c
index 9d9842fc5a..5e28d6340a 100644
--- a/drivers/base/devres.c
+++ b/drivers/base/devres.c
@@ -10,6 +10,17 @@
#include <linux/module.h>
#include <linux/slab.h>
#include <linux/percpu.h>
+#include <linux/dcache.h>
+#include <linux/completion.h>
+#include <linux/debugfs.h>
+#include <linux/kthread.h>
+#include <linux/file.h>
+#include <linux/fs.h>
+#include <linux/magic.h>
+#include <linux/pid.h>
+#include <linux/pid_namespace.h>
+#include <linux/sched.h>
+#include <linux/sched/task.h>
#include <asm/sections.h>
@@ -537,6 +548,662 @@ static int remove_nodes(struct device *dev,
return cnt;
}
+#ifdef CONFIG_DEBUG_DEVRES_REFS
+/*
+ * CONFIG_DEBUG_DEVRES_REFS: before a driver's managed memory is freed, find
+ * open files that still point into it.
+ *
+ * Two phases around devres_release_all(), both started from
+ * device_unbind_cleanup() only, i.e. on driver unbind and after a failed
+ * probe (not a deferred one), in process context with device_lock held.
+ * The walks themselves run on a kernel thread the caller waits for. That
+ * matters for one reason: the walk takes a reference on each file it looks
+ * at, and if the owner closes the file meanwhile the walk's reference is
+ * the last one. A last reference dropped from a user task is released
+ * from that task's task work, i.e. only after unbind returns, so a devres
+ * action that waits for the file to close (snd_card_free() does) would
+ * wait forever under device_lock. Dropped from a kernel thread it goes to
+ * the delayed-fput work instead, which runs on its own. What remains: that
+ * work drains one global list last-in-first-out and all its runs are
+ * serialized, so a release there that needs a lock unbind holds (usbfs
+ * close takes the USB parent lock, which a USB interface unbind holds)
+ * stalls every later delayed close, including one a devres action of
+ * this unbind is waiting for. Without this option that close would have
+ * run from its owner's task work and never touched the list.
+ *
+ * Phase 1 (devres_check_open_files) snapshots the regions and walks every
+ * fd table. A hit on an ordinary file is a WARN at once. A hit on a debugfs
+ * file is decided by whether debugfs has revoked the file: if already
+ * removed and safe to close, info now; otherwise the verdict waits, because
+ * many drivers remove their debugfs files through a devres action that
+ * runs inside devres_release_all(). No reference is kept across release;
+ * only (pid, fd, file address) is recorded. Phase 2
+ * (devres_finish_open_file_check) looks those files up again after
+ * release: gone, or replaced, is nothing; removed and safe to close is
+ * info; anything else is a WARN.
+ */
+static LIST_HEAD(devres_open_file_hooks);
+static DEFINE_MUTEX(devres_open_file_hooks_lock);
+
+/**
+ * devres_register_open_file_hook - tell the open-file check how to reach a
+ * subsystem's objects from an open file
+ * @hook: static or zero-initialised hook with @fops, @owner and @holder set
+ *
+ * Return: 0, -EINVAL if a field is missing, -EEXIST if @fops already has a
+ * hook.
+ */
+int devres_register_open_file_hook(struct devres_open_file_hook *hook)
+{
+ struct devres_open_file_hook *h;
+ int ret = 0;
+
+ if (!hook->fops || !hook->holder)
+ return -EINVAL;
+
+ mutex_lock(&devres_open_file_hooks_lock);
+ list_for_each_entry(h, &devres_open_file_hooks, list) {
+ if (h->fops == hook->fops) {
+ ret = -EEXIST;
+ goto out;
+ }
+ }
+ list_add_tail(&hook->list, &devres_open_file_hooks);
+out:
+ mutex_unlock(&devres_open_file_hooks_lock);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(devres_register_open_file_hook);
+
+/**
+ * devres_unregister_open_file_hook - remove a hook; a no-op if it was never
+ * registered (static or zero-initialised)
+ * @hook: the hook passed to devres_register_open_file_hook()
+ *
+ * A walk in progress holds a module reference on @hook->owner, so an
+ * unload waits for it; the hook itself is never used after this returns.
+ */
+void devres_unregister_open_file_hook(struct devres_open_file_hook *hook)
+{
+ mutex_lock(&devres_open_file_hooks_lock);
+ /* Never registered (static zero) or already unregistered: nothing to do. */
+ if (hook->list.next && !list_empty(&hook->list))
+ list_del_init(&hook->list);
+ mutex_unlock(&devres_open_file_hooks_lock);
+}
+EXPORT_SYMBOL_GPL(devres_unregister_open_file_hook);
+
+struct devres_region {
+ const void *start;
+ size_t size;
+ const char *name;
+ bool warned;
+};
+
+/* A hit on a live debugfs file, looked up again after devres_release_all(). */
+struct devres_pending_ref {
+ const struct file *file; /* identity only, never dereferenced */
+ const void *ptr; /* the offending pointer value */
+ struct devres_region *region;
+ const char *what;
+ char comm[TASK_COMM_LEN];
+ pid_t pnr, tgid;
+ unsigned int fd;
+ bool via_i_private;
+};
+
+/* Beyond this many distinct fd tables, later sharers are walked again. */
+#define DEVRES_SEEN_TABLES_MAX 1048576U
+#define DEVRES_PENDING_MAX 4096U
+#define DEVRES_HOOK_OBJECTS_MAX 8U
+
+/*
+ * Copy the data regions of @dev's managed allocations. Taken before
+ * devres_release_all() so the walk never holds devres_lock and never looks
+ * at a node that is being freed.
+ */
+static unsigned int devres_snapshot_regions(struct device *dev,
+ struct devres_region **out)
+{
+ struct devres_region *regions;
+ struct devres_node *node;
+ unsigned int n = 0, i = 0;
+ unsigned long flags;
+
+ spin_lock_irqsave(&dev->devres_lock, flags);
+ list_for_each_entry(node, &dev->devres_head, entry)
+ if (node->release == dr_node_release && node->size)
+ n++;
+ spin_unlock_irqrestore(&dev->devres_lock, flags);
+
+ if (!n)
+ return 0;
+
+ regions = kcalloc(n, sizeof(*regions), GFP_KERNEL);
+ if (!regions)
+ return 0;
+
+ /*
+ * The list may have changed between the two passes (a driver still
+ * calling devm_* after remove() returned, which is its own bug).
+ * Growth is bounded by @n (extra nodes are not checked); shrinkage
+ * leaves @i below @n, and only [0, @i) is returned.
+ */
+ spin_lock_irqsave(&dev->devres_lock, flags);
+ list_for_each_entry(node, &dev->devres_head, entry) {
+ struct devres *dr;
+
+ if (node->release != dr_node_release || !node->size)
+ continue;
+ if (i == n)
+ break;
+ dr = container_of(node, struct devres, node);
+ regions[i].start = dr->data;
+ regions[i].size = node->size;
+ regions[i].name = node->name;
+ i++;
+ }
+ spin_unlock_irqrestore(&dev->devres_lock, flags);
+
+ if (!i) {
+ kfree(regions);
+ return 0;
+ }
+ *out = regions;
+ return i;
+}
+
+static struct devres_region *devres_region_containing(struct devres_region *r,
+ unsigned int nr,
+ const void *p)
+{
+ unsigned int i;
+
+ if (!p)
+ return NULL;
+ for (i = 0; i < nr; i++)
+ if (p >= r[i].start && p < r[i].start + r[i].size)
+ return &r[i];
+ return NULL;
+}
+
+/*
+ * Hooks are used from the walk thread without the list mutex; a module
+ * reference on each owner keeps the hook object and its holder alive.
+ */
+static void devres_snapshot_hooks(struct devres_open_file_check *chk)
+{
+ struct devres_open_file_hook *hook;
+ unsigned int n = 0;
+
+ mutex_lock(&devres_open_file_hooks_lock);
+ list_for_each_entry(hook, &devres_open_file_hooks, list)
+ n++;
+ if (n) {
+ chk->hooks = kcalloc(n, sizeof(*chk->hooks), GFP_KERNEL);
+ if (chk->hooks) {
+ list_for_each_entry(hook, &devres_open_file_hooks, list) {
+ if (chk->nhooks == n)
+ break;
+ /* An owner that is unloading unregisters next. */
+ if (!try_module_get(hook->owner))
+ continue;
+ chk->hooks[chk->nhooks++] = hook;
+ }
+ }
+ }
+ mutex_unlock(&devres_open_file_hooks_lock);
+}
+
+static void devres_release_hooks(struct devres_open_file_check *chk)
+{
+ while (chk->nhooks)
+ module_put(chk->hooks[--chk->nhooks]->owner);
+ kfree(chk->hooks);
+ chk->hooks = NULL;
+}
+
+#define DEVRES_REF_FMT "DEVRES %s (%zu bytes) still referenced by %s of fd %u (%pD3) in %s[%d]"
+
+/* One backtrace per allocation; further holders get one line each. */
+static void devres_warn_ref(struct device *dev, struct devres_region *region,
+ const char *what, unsigned int fd,
+ struct file *file, const char *comm, pid_t pnr,
+ const char *why)
+{
+ if (!region->warned) {
+ region->warned = true;
+ dev_WARN(dev, DEVRES_REF_FMT " at unbind%s\n", region->name,
+ region->size, what, fd, file, comm, pnr, why);
+ } else {
+ dev_warn(dev, DEVRES_REF_FMT " at unbind%s\n", region->name,
+ region->size, what, fd, file, comm, pnr, why);
+ }
+}
+
+/*
+ * A removed debugfs file: debugfs refuses further operations on it, but
+ * close() still runs the real release callback. Info if that callback is
+ * one that cannot touch the driver's data, a WARN naming it otherwise.
+ */
+static void devres_report_debugfs(struct device *dev,
+ struct devres_region *region,
+ const char *what, unsigned int fd,
+ struct file *file, const char *comm,
+ pid_t pnr, const struct file_operations *fops,
+ const char *when)
+{
+ if (debugfs_file_release_safe(fops)) {
+ dev_info(dev, DEVRES_REF_FMT " at unbind: debugfs file removed %s, ops refused by debugfs, close is safe\n",
+ region->name, region->size, what, fd, file, comm,
+ pnr, when);
+ return;
+ }
+ if (!region->warned) {
+ region->warned = true;
+ dev_WARN(dev, DEVRES_REF_FMT " at unbind: debugfs file removed %s, but close will call %ps on it\n",
+ region->name, region->size, what, fd, file, comm,
+ pnr, when, fops->release);
+ } else {
+ dev_warn(dev, DEVRES_REF_FMT " at unbind: debugfs file removed %s, but close will call %ps on it\n",
+ region->name, region->size, what, fd, file, comm,
+ pnr, when, fops->release);
+ }
+}
+
+static void devres_report_open_file(struct device *dev,
+ struct devres_open_file_check *chk,
+ struct devres_region *region,
+ struct task_struct *task, pid_t pnr,
+ unsigned int fd, struct file *file,
+ const char *what)
+{
+ const struct file_operations *real_fops;
+ struct devres_pending_ref *pend;
+ char comm[TASK_COMM_LEN];
+
+ get_task_comm(comm, task);
+
+ /* Already removed by remove(): decide now. */
+ if (debugfs_file_revoked(file, &real_fops)) {
+ devres_report_debugfs(dev, region, what, fd, file, comm, pnr,
+ real_fops, "in remove()");
+ return;
+ }
+
+ /* Live debugfs file: a devres action may still remove it. Defer. */
+ if (file_inode(file)->i_sb->s_magic == DEBUGFS_MAGIC) {
+ if (chk->npending == chk->maxpending &&
+ chk->maxpending < DEVRES_PENDING_MAX) {
+ unsigned int max = chk->maxpending ? chk->maxpending * 2 : 16;
+
+ pend = kvrealloc(chk->pending,
+ array_size(max, sizeof(*pend)),
+ GFP_KERNEL | __GFP_NOWARN);
+ if (pend) {
+ chk->pending = pend;
+ chk->maxpending = max;
+ }
+ }
+ if (chk->npending < chk->maxpending) {
+ pend = &chk->pending[chk->npending++];
+ pend->file = file;
+ pend->via_i_private = !strcmp(what, "i_private");
+ pend->ptr = pend->via_i_private ?
+ READ_ONCE(file_inode(file)->i_private) :
+ READ_ONCE(file->private_data);
+ pend->region = region;
+ pend->what = what;
+ strscpy(pend->comm, comm, sizeof(pend->comm));
+ pend->pnr = pnr;
+ pend->tgid = task_tgid_nr(task);
+ pend->fd = fd;
+ return;
+ }
+ /* Cannot defer: undecided, so a line without a backtrace. */
+ dev_warn(dev, DEVRES_REF_FMT " at unbind: live debugfs file, verdict not deferred\n",
+ region->name, region->size, what, fd, file, comm, pnr);
+ return;
+ }
+
+ devres_warn_ref(dev, region, what, fd, file, comm, pnr, "");
+}
+
+static void devres_check_one_file(struct device *dev,
+ struct devres_open_file_check *chk,
+ struct task_struct *task, pid_t pnr,
+ unsigned int fd, struct file *file)
+{
+ struct devres_region *region;
+ const void *pd, *ip;
+ unsigned int i;
+
+ /* O_PATH files have no operations and no release: nothing can use them. */
+ if (file->f_mode & FMODE_PATH)
+ return;
+
+ pd = READ_ONCE(file->private_data);
+ region = devres_region_containing(chk->regions, chk->nr, pd);
+ if (region)
+ devres_report_open_file(dev, chk, region, task, pnr, fd, file,
+ "private_data");
+
+ ip = READ_ONCE(file_inode(file)->i_private);
+ if (ip != pd) {
+ region = devres_region_containing(chk->regions, chk->nr, ip);
+ if (region)
+ devres_report_open_file(dev, chk, region, task, pnr,
+ fd, file, "i_private");
+ }
+
+ /*
+ * A character device file holds a reference on its cdev's kobject
+ * (chrdev_open()), and inode->i_cdev is cleared only when that
+ * kobject is released, so for an open non-O_PATH file the pointer
+ * is stable here even after cdev_del(). A struct cdev embedded in
+ * managed memory therefore shows up directly. Compared, never
+ * dereferenced. i_cdev shares a union; only valid for S_ISCHR.
+ */
+ if (S_ISCHR(file_inode(file)->i_mode)) {
+ const void *cdev = READ_ONCE(file_inode(file)->i_cdev);
+
+ region = devres_region_containing(chk->regions, chk->nr, cdev);
+ if (region)
+ devres_report_open_file(dev, chk, region, task, pnr,
+ fd, file, "i_cdev");
+ }
+
+ for (i = 0; i < chk->nhooks; i++) {
+ struct devres_open_file_hook *hook = chk->hooks[i];
+ unsigned int idx;
+
+ if (file->f_op != hook->fops)
+ continue;
+ /* A holder ends with *what NULL; the cap guards a holder that never does. */
+ for (idx = 0; idx < DEVRES_HOOK_OBJECTS_MAX; idx++) {
+ const char *what = NULL;
+ const void *p = hook->holder(file, idx, &what);
+
+ if (!what)
+ break;
+ region = devres_region_containing(chk->regions,
+ chk->nr, p);
+ if (region)
+ devres_report_open_file(dev, chk, region, task,
+ pnr, fd, file, what);
+ }
+ }
+}
+
+/*
+ * Each fd table is walked once. A table already walked is remembered as
+ * (owning task, table) with a reference on the task only: a table counts as
+ * seen while that task still points at it, which proves it was not freed
+ * and reused by another task (the recorded task could only reach the same
+ * address again through two table switches, and what it opens after the
+ * walk visited it is not promised anyway). Nothing takes a reference on the
+ * table itself, so the walk never ends up closing another task's files. If
+ * the set cannot grow, later sharers are walked again, costing duplicate
+ * lines. The set is scanned linearly: O(threads x tables) per walk.
+ */
+static bool devres_fd_table_seen(struct devres_open_file_check *chk,
+ struct files_struct *files)
+{
+ unsigned int i;
+
+ for (i = 0; i < chk->nseen; i++)
+ if (chk->seen[i].files == files &&
+ READ_ONCE(chk->seen[i].task->files) == files)
+ return true;
+ return false;
+}
+
+static void devres_fd_table_mark(struct devres_open_file_check *chk,
+ struct task_struct *task)
+{
+ struct devres_seen_table *seen;
+ struct files_struct *files;
+
+ if (chk->nseen == chk->maxseen) {
+ unsigned int max = chk->maxseen ? chk->maxseen * 2 : 64;
+
+ if (max > DEVRES_SEEN_TABLES_MAX)
+ return;
+ seen = kvrealloc(chk->seen, array_size(max, sizeof(*seen)),
+ GFP_KERNEL | __GFP_NOWARN);
+ if (!seen)
+ return;
+ chk->seen = seen;
+ chk->maxseen = max;
+ }
+
+ files = READ_ONCE(task->files);
+ if (files && !devres_fd_table_seen(chk, files)) {
+ chk->seen[chk->nseen].task = get_task_struct(task);
+ chk->seen[chk->nseen].files = files;
+ chk->nseen++;
+ }
+}
+
+/* Phase 1 body: every thread's open files, each fd table once. */
+static int devres_walk_open_files(void *data)
+{
+ struct devres_open_file_check *chk = data;
+ struct device *dev = chk->dev;
+ unsigned int tid = 0;
+
+ for (;;) {
+ struct task_struct *task;
+ struct file *file;
+ unsigned int fd = 0;
+ struct pid *pid;
+ pid_t pnr;
+
+ rcu_read_lock();
+ pid = find_ge_pid(tid, &init_pid_ns);
+ if (!pid) {
+ rcu_read_unlock();
+ break;
+ }
+ pnr = pid_nr(pid);
+ tid = pnr + 1;
+ task = pid_task(pid, PIDTYPE_PID);
+ if (task && READ_ONCE(task->files) &&
+ !devres_fd_table_seen(chk, READ_ONCE(task->files)))
+ get_task_struct(task);
+ else
+ task = NULL;
+ rcu_read_unlock();
+ if (!task)
+ continue;
+
+ devres_fd_table_mark(chk, task);
+ while ((file = fget_task_next(task, &fd))) {
+ devres_check_one_file(dev, chk, task, pnr, fd, file);
+ fput(file);
+ fd++;
+ }
+ put_task_struct(task);
+ cond_resched();
+ }
+
+ while (chk->nseen)
+ put_task_struct(chk->seen[--chk->nseen].task);
+ kvfree(chk->seen);
+ chk->seen = NULL;
+ chk->maxseen = 0;
+ complete(&chk->done);
+ return 0;
+}
+
+/* Phase 2 body: look each deferred (pid, fd) up again and decide. */
+static int devres_finish_open_files(void *data)
+{
+ struct devres_open_file_check *chk = data;
+ const struct file_operations *real_fops;
+ struct device *dev = chk->dev;
+ unsigned int i;
+
+ for (i = 0; i < chk->npending; i++) {
+ struct devres_pending_ref *pend = &chk->pending[i];
+ struct task_struct *task;
+ struct file *file;
+ const void *p;
+
+ /* The recorded thread, or failing that its group leader. */
+ rcu_read_lock();
+ task = find_task_by_pid_ns(pend->pnr, &init_pid_ns);
+ if (!task)
+ task = find_task_by_pid_ns(pend->tgid, &init_pid_ns);
+ if (task)
+ get_task_struct(task);
+ rcu_read_unlock();
+ if (!task)
+ continue; /* process gone: nothing left to use the pointer */
+
+ /*
+ * Find the recorded struct file anywhere in the task's table:
+ * the descriptor may have moved (dup2) since the walk. The
+ * address alone could be a reuse, so the same pointer value
+ * in the same field is required too; a reopened file of the
+ * same debugfs node satisfies both and dangles just the same.
+ */
+ file = fget_task(task, pend->fd);
+ if (file && file != pend->file) {
+ fput(file);
+ file = NULL;
+ }
+ if (!file) {
+ unsigned int fd = 0;
+
+ while ((file = fget_task_next(task, &fd))) {
+ if (file == pend->file)
+ break;
+ fput(file);
+ fd++;
+ }
+ }
+ put_task_struct(task);
+ if (!file)
+ continue; /* closed: nothing left to use the pointer */
+
+ p = pend->via_i_private ?
+ READ_ONCE(file_inode(file)->i_private) :
+ READ_ONCE(file->private_data);
+ if (p != pend->ptr) {
+ fput(file);
+ continue; /* replaced */
+ }
+
+ if (debugfs_file_revoked(file, &real_fops))
+ devres_report_debugfs(dev, pend->region, pend->what,
+ pend->fd, file, pend->comm,
+ pend->pnr, real_fops,
+ "by a devres action");
+ else if (d_unlinked(file->f_path.dentry))
+ devres_warn_ref(dev, pend->region, pend->what, pend->fd,
+ file, pend->comm, pend->pnr,
+ ": debugfs file removed, but its file_operations are not guarded by debugfs");
+ else
+ devres_warn_ref(dev, pend->region, pend->what, pend->fd,
+ file, pend->comm, pend->pnr,
+ ": debugfs file still registered after its data was freed");
+ fput(file);
+ }
+ complete(&chk->done);
+ return 0;
+}
+
+/*
+ * Run @fn on a kernel thread and wait for it. A last file reference
+ * dropped there goes to the delayed-fput work, never to this task's task
+ * work, so nothing a devres action may wait for is queued behind unbind.
+ * The thread signals @chk->done and exits by itself; kthread_stop() is
+ * not used, since a stop requested before the thread first runs makes
+ * the kthread wrapper skip @fn altogether.
+ */
+static void devres_run_walk(struct devres_open_file_check *chk,
+ int (*fn)(void *), const char *name)
+{
+ struct task_struct *t;
+
+ init_completion(&chk->done);
+ t = kthread_run(fn, chk, "%s/%s", name, dev_name(chk->dev));
+ if (IS_ERR(t)) {
+ dev_warn(chk->dev, "DEVRES: open-file check skipped, no thread\n");
+ return;
+ }
+ wait_for_completion(&chk->done);
+}
+
+/**
+ * devres_check_open_files - find open files into memory about to be freed
+ * @dev: device being unbound; its managed resources are still allocated
+ * @chk: zero-initialised state, passed on to devres_finish_open_file_check()
+ *
+ * Walk every thread's open files and report any that still point into one
+ * of @dev's managed allocations, which devres_release_all() is about to
+ * free. Must be called in process context with no spinlocks held; it
+ * sleeps while a kernel thread does the walk. No file reference survives
+ * the call; a live debugfs hit is recorded as (pid, fd, file address) for
+ * devres_finish_open_file_check().
+ *
+ * If the owner closes a file while the walk holds its reference, the
+ * final fput() comes from the walk thread and the driver's release runs
+ * from the delayed-fput work, after this unbind has freed the memory if
+ * the file pointed into it: that is the bug being reported, hit one close
+ * later. A devres action that waits for such a close still completes,
+ * since that work does not wait for unbind, unless a release ahead of it
+ * on the serialized delayed-fput worker needs a lock unbind holds. A file
+ * passed to another process after the walk and closed here is not found
+ * again in phase 2. If the region snapshot cannot be allocated the check
+ * is skipped silently.
+ */
+void devres_check_open_files(struct device *dev,
+ struct devres_open_file_check *chk)
+{
+ might_sleep();
+
+ chk->dev = dev;
+ chk->nr = devres_snapshot_regions(dev, &chk->regions);
+ if (!chk->nr)
+ return;
+
+ devres_snapshot_hooks(chk);
+ devres_run_walk(chk, devres_walk_open_files, "devres_refs");
+}
+
+/**
+ * devres_finish_open_file_check - decide the deferred debugfs hits
+ * @dev: the device, after devres_release_all()
+ * @chk: state from devres_check_open_files()
+ *
+ * A debugfs file that was live during the walk may have been removed by a
+ * devres action since; if debugfs refuses operations on it and its release
+ * cannot touch the data, the hit is informational. One still live, or
+ * whose close will run a release callback of the driver's own, points at
+ * freed memory.
+ */
+void devres_finish_open_file_check(struct device *dev,
+ struct devres_open_file_check *chk)
+{
+ if (!chk->nr)
+ return;
+
+ if (chk->npending)
+ devres_run_walk(chk, devres_finish_open_files, "devres_refs2");
+
+ devres_release_hooks(chk);
+ kvfree(chk->pending);
+ chk->pending = NULL;
+ chk->npending = 0;
+ chk->maxpending = 0;
+ kfree(chk->regions);
+ chk->regions = NULL;
+ chk->nr = 0;
+}
+#endif /* CONFIG_DEBUG_DEVRES_REFS */
+
static void release_nodes(struct device *dev, struct list_head *todo)
{
struct devres_node *node, *tmp;
diff --git a/include/linux/device/devres.h b/include/linux/device/devres.h
index 7656c9ecd4..dd3f88fd8a 100644
--- a/include/linux/device/devres.h
+++ b/include/linux/device/devres.h
@@ -11,6 +11,8 @@
#include <asm/bug.h>
#include <asm/percpu.h>
+#include <linux/list.h>
+
struct device;
struct device_node;
struct resource;
@@ -284,4 +286,60 @@ static inline int __devm_add_action_or_reset(struct device *dev, void (*action)(
bool devm_is_action_added(struct device *dev, void (*action)(void *), void *data);
+struct file;
+struct file_operations;
+
+/**
+ * struct devres_open_file_hook - how an open file of one file_operations
+ * reaches the driver objects it keeps alive
+ * @list: internal
+ * @fops: the shared file_operations installed on such files at open
+ * @owner: the module holding @holder; a walk pins it
+ * @holder: returns the @idx-th object an open @fops file keeps alive and
+ * sets *@what to its name, for @idx from 0 up; leaves *@what NULL
+ * when there are no more, which ends the sequence; may return
+ * NULL with *@what set if that object is absent. At most eight
+ * objects are consulted.
+ *
+ * CONFIG_DEBUG_DEVRES_REFS checks, on driver unbind or a failed probe,
+ * whether any open file still points into memory devres is about to free. A
+ * subsystem whose files do not reach their objects through
+ * file->private_data, the inode's i_private or i_cdev registers one of
+ * these. @holder is called on a kernel thread with a reference on @file
+ * held and a module reference on @owner; it must not sleep on anything that
+ * could wait for an unbind and must not dereference the objects it returns,
+ * nor anything else that may sit in managed memory of some device, which
+ * may already be freed; only what the open @file itself pins is safe.
+ * @file is not const only because subsystem accessors such as
+ * video_devdata() take a non-const file; @holder must not modify it. The
+ * struct is defined unconditionally so that a subsystem can keep its
+ * instance outside any #ifdef; the registration calls are empty
+ * stubs when the option is off. The instance must be static or
+ * zero-initialised, so that unregistering a hook whose registration failed
+ * is a no-op. Unregister before the owning module's memory goes away,
+ * including on a failed module init.
+ */
+struct module;
+struct devres_open_file_hook {
+ struct list_head list;
+ const struct file_operations *fops;
+ struct module *owner;
+ const void *(*holder)(struct file *file, unsigned int idx,
+ const char **what);
+};
+
+#ifdef CONFIG_DEBUG_DEVRES_REFS
+int devres_register_open_file_hook(struct devres_open_file_hook *hook);
+void devres_unregister_open_file_hook(struct devres_open_file_hook *hook);
+#else
+static inline int devres_register_open_file_hook(struct devres_open_file_hook *hook)
+{
+ return 0;
+}
+
+static inline void devres_unregister_open_file_hook(struct devres_open_file_hook *hook)
+{
+}
+#endif
+
#endif /* _DEVICE_DEVRES_H_ */
--
2.43.0
next prev parent reply other threads:[~2026-11-04 22:00 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-11-04 21:55 [RFC PATCH 0/4] devres: warn about managed memory still held by open files at unbind Chad Aldreda
2026-11-04 21:55 ` [RFC PATCH 1/4] debugfs: add debugfs_file_revoked() for debugging aids Chad Aldreda
2026-11-04 22:00 ` Chad Aldreda [this message]
2026-11-04 22:00 ` [RFC PATCH 3/4] media: v4l2-dev: name the video_device behind an open file for DEBUG_DEVRES_REFS Chad Aldreda
2026-11-04 22:00 ` [RFC PATCH 4/4] drivers: base: test: add a vehicle " Chad Aldreda
2026-11-04 22:07 ` [RFC PATCH 0/4] devres: warn about managed memory still held by open files at unbind Laurent Pinchart
2026-11-04 22:31 ` Chad Aldreda
2026-11-04 22:41 ` Laurent Pinchart
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20261104220041.3994559-1-ca@modelmiser.com \
--to=ca@modelmiser.com \
--cc=corbet@lwn.net \
--cc=dakr@kernel.org \
--cc=driver-core@lists.linux.dev \
--cc=gregkh@linuxfoundation.org \
--cc=hverkuil+cisco@kernel.org \
--cc=johan@kernel.org \
--cc=laurent.pinchart@ideasonboard.com \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=mchehab@kernel.org \
--cc=rafael@kernel.org \
--cc=yux50000@hotmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®