From: Al Viro viro@zeniv.linux.org.uk
[ 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@syzkaller.appspotmail.com Signed-off-by: Al Viro viro@zeniv.linux.org.uk Signed-off-by: Sasha Levin sashal@kernel.org ---
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@syzkaller.appspotmail.com", 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 aada4e7c8713..cbd42d76fbd0 100644 --- a/fs/libfs.c +++ b/fs/libfs.c @@ -274,7 +274,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) { @@ -286,7 +286,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))