diff options
| author | Michael Liang <mliang@purestorage.com> | 2026-08-21 12:15:27 -0600 |
|---|---|---|
| committer | Andrew Morton <akpm@linux-foundation.org> | 2026-08-27 22:50:04 -0700 |
| commit | 4fe66b2cb9b545504e280cc8a73586c59f0bbe5e (patch) | |
| tree | 879435e1d4689fb5ef1c5079118882541e6a91b0 /include | |
| parent | bd5659f71cd8cb79c9b250861344692e003b8d4e (diff) | |
| download | linux-next-4fe66b2cb9b545504e280cc8a73586c59f0bbe5e.tar.gz linux-next-4fe66b2cb9b545504e280cc8a73586c59f0bbe5e.zip | |
fault-inject: fix dentry leak
fault_create_debugfs_attr() has always taken an extra dentry reference on
the created directory (attr->dname = dget(dir)) so that fail_dump() could
print the name via %pd from any context. Nothing anywhere in the tree
ever calls dput() on attr->dname.
For callers with a matching teardown, that unmatched reference causes one
dentry plus its attached inode to leak per fault_create_debugfs_attr /
debugfs_remove_recursive cycle. simple_recursive_removal() drops
debugfs's own +1 ref on the child dentry, but the dget()'d ref keeps its
refcount at 1: the dentry ends up unhashed but pinned, and its inode is
never freed.
Boot-once callers (mm/failslab, block/blk-core, etc.) leak exactly once at
init and never destroy the tree, so the impact there is bounded. But
per-lifecycle callers (drivers/nvme, drivers/infiniband/hw/hfi1,
drivers/mmc, drivers/iommu/iommufd, drivers/media, drivers/misc,
drivers/gpu/drm/msm, drivers/crypto, net/sunrpc) leak on every
create/destroy cycle.
We observed this in production: an NVMe/RDMA host repeatedly reconnecting
to a target that rejected the CRTO Property Get went through ~50 nvme
controller create/destroy cycles per second, and dentry and inode_cache
grew by ~13k pinned objects per 240 s -- unrecoverable through
drop_caches. Byte math matched a per-cycle 1-dentry / 1-inode leak from
the "fault_inject" directory dentry.
Fix this by not holding any external reference in fault_attr. Embed the
directory name as a fixed-size char array (FAULT_ATTR_DNAME_LEN, 64 bytes)
inside struct fault_attr, copied by strscpy() at
fault_create_debugfs_attr() time. fail_dump() prints it via %s.
Advantages of an embedded array over kstrdup() + kfree() paired with a new
destroy API:
- Zero API footprint. No new export and no caller changes required:
callers already own their fault_attr's memory and free it when
they are done, and now that suffices.
- No allocation on the create path.
- fault_create_debugfs_attr() cannot fail from the name-copy step.
- No lifetime coupling between attr->dname and debugfs; the string
is valid for exactly as long as the containing struct.
The 64-byte length accommodates every in-tree caller with generous
headroom (the longest current name is "fail_dma_array_full", 19 chars).
The user-visible fail_dump() format changes from "name %pd" to "name %s",
but the printed content is identical -- %pd on the created directory
renders the same string that was passed in as @name.
drivers/infiniband/hw/hfi1/fault.c drops a now-invalid "attr.dname = NULL"
statement; the surrounding kzalloc() already zero-initialises the array.
Link: https://lore.kernel.org/20260821181527.3271414-1-mliang@purestorage.com
Fixes: 6adc4a22f20b ("fault-inject: add ratelimit option")
Signed-off-by: Michael Liang <mliang@purestorage.com>
Reviewed-by: Andrew Morton <akpm@linux-foundation.org>
Cc: Akinbou Mita <akinobu.mita@gmail.com>
Cc: Dennis Dalessandro <dennis.dalessandro@cornelisnetworks.com>
Cc: Jason Gunthorpe <jgg@ziepe.ca>
Cc: Leon Romanovsky <leon@kernel.org>
Cc: Vlastimil Babka <vbabka@kernel.org>
Cc: <stable@vger.kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Diffstat (limited to 'include')
| -rw-r--r-- | include/linux/fault-inject.h | 10 |
1 files changed, 8 insertions, 2 deletions
diff --git a/include/linux/fault-inject.h b/include/linux/fault-inject.h index 58fd14c82270..5c74748a53f3 100644 --- a/include/linux/fault-inject.h +++ b/include/linux/fault-inject.h @@ -19,6 +19,13 @@ enum fault_flags { #include <linux/ratelimit.h> /* + * Length of the debugfs directory name embedded in struct fault_attr. + * Chosen to accommodate every in-tree caller of fault_create_debugfs_attr() + * (the longest is "fail_dma_array_full", 19 chars) with generous headroom. + */ +#define FAULT_ATTR_DNAME_LEN 64 + +/* * For explanation of the elements of this struct, see * Documentation/fault-injection/fault-injection.rst */ @@ -37,7 +44,7 @@ struct fault_attr { unsigned long count; struct ratelimit_state ratelimit_state; - struct dentry *dname; + char dname[FAULT_ATTR_DNAME_LEN]; }; #define FAULT_ATTR_INITIALIZER { \ @@ -47,7 +54,6 @@ struct fault_attr { .stacktrace_depth = 32, \ .ratelimit_state = RATELIMIT_STATE_INIT_DISABLED, \ .verbose = 2, \ - .dname = NULL, \ } #define DECLARE_FAULT_ATTR(name) struct fault_attr name = FAULT_ATTR_INITIALIZER |
