From: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
[ Upstream commit 2a8061ee5e41034eb14170ec4517b5583dbeff9f ]
We want a class that nests outside of I_MUTEX_NORMAL (for the sake of
callbacks that might want to lock the victim) and inside I_MUTEX_PARENT
(so that a variant of that could be used with parent of the victim
held locked by the caller).
In reality, simple_recursive_removal()
* never holds two locks at once
* holds the lock on parent of dentry passed to callback
* is used only on the trees with fixed topology, so the depths
are not changing.
So the locking order is actually fine.
AFAICS, the best solution is to assign I_MUTEX_CHILD to the locks
grabbed by that thing.
Reported-by: syzbot+169de184e9defe7fe709@xxxxxxxxxxxxxxxxxxxxxxxxx
Signed-off-by: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>
---
LLM Generated explanations, may be completely bogus:
**Backport Status: YES**
This commit should be backported to stable kernel trees for the
following reasons:
1. **Fixes a lockdep warning reported by syzbot**: The commit message
explicitly mentions "Reported-by:
syzbot+169de184e9defe7fe709@xxxxxxxxxxxxxxxxxxxxxxxxx", indicating
this fixes a real issue detected by kernel testing infrastructure.
2. **Small and contained fix**: The change is minimal - only 2 lines
changed, replacing `inode_lock(inode)` with `inode_lock_nested(inode,
I_MUTEX_CHILD)` in two locations. This is exactly the type of
targeted fix suitable for stable backports.
3. **Fixes incorrect lockdep annotations**: The commit corrects lockdep
annotations without changing actual runtime behavior. According to
the commit message, "the locking order is actually fine" - this is
purely fixing false positive lockdep warnings that could mask real
locking issues.
4. **No functional changes**: The code only changes lockdep annotations
by using `inode_lock_nested()` with `I_MUTEX_CHILD` instead of plain
`inode_lock()`. This doesn't change the actual locking behavior, just
tells lockdep about the correct locking hierarchy.
5. **Prevents false positives in debugging**: False lockdep warnings can
obscure real locking problems and make kernel debugging more
difficult. Fixing these annotations helps maintain the effectiveness
of lockdep as a debugging tool.
6. **Used by multiple filesystems**: Based on my grep results,
`simple_recursive_removal()` is used by several filesystems including
debugfs, tracefs, efivarfs, fuse, and nfsd. A lockdep false positive
here could affect debugging across multiple subsystems.
7. **Low risk**: Since this only changes lockdep annotations and not
actual locking behavior, the risk of regression is minimal. The worst
case would be if the annotation was wrong, which would just result in
lockdep warnings again.
The commit follows stable kernel rules by being a targeted fix for a
specific issue (lockdep false positive) without introducing new features
or architectural changes.
fs/libfs.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/libfs.c b/fs/libfs.c
index aa0fbd720409..c6ed6c58dee6 100644
--- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -272,7 +272,7 @@ void simple_recursive_removal(struct dentry *dentry,
struct dentry *victim = NULL, *child;
struct inode *inode = this->d_inode;
- inode_lock(inode);
+ inode_lock_nested(inode, I_MUTEX_CHILD);
if (d_is_dir(this))
inode->i_flags |= S_DEAD;
while ((child = find_next_child(this, victim)) == NULL) {
@@ -284,7 +284,7 @@ void simple_recursive_removal(struct dentry *dentry,
victim = this;
this = this->d_parent;
inode = this->d_inode;
- inode_lock(inode);
+ inode_lock_nested(inode, I_MUTEX_CHILD);
if (simple_positive(victim)) {
d_invalidate(victim); // avoid lost mounts
if (d_is_dir(victim))
--
2.39.5
