[PATCH v2] ocfs2: use inode_lock_nested() for orphan dir locking

[PATCH v2] ocfs2: use inode_lock_nested() for orphan dir locking

[Deepanshu Kartikey]

PREEMPT_RT's rtmutex PI chain walker warns about a lock dependency
cycle when inode_lock(orphan_dir_inode) is called while holding
inode_lock(file_inode):

  ocfs2_file_write_iter()
    inode_lock(file_inode)               [class 0]
      ocfs2_dio_end_io_write()
        ocfs2_del_inode_from_orphan()
          inode_lock(orphan_dir_inode)   [class 0] <- warning!

However this is a false positive. write_iter() is never called on a
directory, and orphan_dir is always a directory, so these two locks
can never actually conflict in practice.

Fix by using inode_lock_nested(orphan_dir_inode, I_MUTEX_NONDIR2) in
all three places where orphan_dir_inode is locked in namei.c, placing
it in a separate lock class so the rtmutex PI chain walker understands
these locks have distinct roles and does not warn about their ordering.

Suggested-by: Matthew Wilcox <willy@infradead.org>
Reported-by: syzbot+ce129763ce7d7e914739@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=ce129763ce7d7e914739
Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>

---
Changes in v2:
- Replaced inode_lock() with inode_lock_nested(orphan_dir_inode,
  I_MUTEX_NONDIR2) in all three call sites in namei.c
- Dropped incorrect v1 changes to aops.c and namei.c that
  restructured lock ordering, as the deadlock was a false positive
- Approach suggested by Matthew Wilcox
---
 fs/ocfs2/namei.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/ocfs2/namei.c b/fs/ocfs2/namei.c
index 1277666c77cd..4cfd7b3d3e1a 100644
--- a/fs/ocfs2/namei.c
+++ b/fs/ocfs2/namei.c
@@ -2126,7 +2126,7 @@ static int ocfs2_lookup_lock_orphan_dir(struct ocfs2_super *osb,
 		return ret;
 	}
 
-	inode_lock(orphan_dir_inode);
+	inode_lock_nested(orphan_dir_inode, I_MUTEX_NONDIR2);
 
 	ret = ocfs2_inode_lock(orphan_dir_inode, &orphan_dir_bh, 1);
 	if (ret < 0) {
@@ -2725,7 +2725,7 @@ int ocfs2_del_inode_from_orphan(struct ocfs2_super *osb,
 		goto bail;
 	}
 
-	inode_lock(orphan_dir_inode);
+	inode_lock_nested(orphan_dir_inode, I_MUTEX_NONDIR2);
 	status = ocfs2_inode_lock(orphan_dir_inode, &orphan_dir_bh, 1);
 	if (status < 0) {
 		inode_unlock(orphan_dir_inode);
@@ -2838,7 +2838,7 @@ int ocfs2_mv_orphaned_inode_to_new(struct inode *dir,
 		goto leave;
 	}
 
-	inode_lock(orphan_dir_inode);
+	inode_lock_nested(orphan_dir_inode, I_MUTEX_NONDIR2);
 
 	status = ocfs2_inode_lock(orphan_dir_inode, &orphan_dir_bh, 1);
 	if (status < 0) {
-- 
2.43.0

Re: [PATCH v2] ocfs2: use inode_lock_nested() for orphan dir locking

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Incorrect assignment of the I_MUTEX_NONDIR2 subclass to the orphan directory violates VFS semantics, introduces a new lockdep cycle during rename, and fragments lockdep tracking.

Pre-existing issues:
- [Critical] Unvalidated on-disk fields i_dio_orphaned_slot and i_orphaned_slot are used as array indices, leading to out-of-bounds memory accesses.
--

commit f1e102271ff71f1f4452e0af815188536b47c93e
Author: Deepanshu Kartikey <kartikey406@gmail.com>

ocfs2: use inode_lock_nested() for orphan dir locking
    
PREEMPT_RT's rtmutex PI chain walker warns about a lock dependency
cycle when inode_lock(orphan_dir_inode) is called while holding
inode_lock(file_inode):

[ ... ]

Closes: https://syzkaller.appspot.com/bug?extid=ce129763ce7d7e914739

> diff --git a/fs/ocfs2/namei.c b/fs/ocfs2/namei.c
> index 1277666c77cd2..4cfd7b3d3e1a4 100644
> --- a/fs/ocfs2/namei.c
> +++ b/fs/ocfs2/namei.c
> @@ -2126,7 +2126,7 @@ static int ocfs2_lookup_lock_orphan_dir(struct ocfs2_super *osb,
>  		return ret;
>  	}
>  
> -	inode_lock(orphan_dir_inode);
> +	inode_lock_nested(orphan_dir_inode, I_MUTEX_NONDIR2);

[Severity: High]
Does this violate vfs locking rules by assigning a non-directory lock class
to a directory? The orphan_dir_inode is a directory, so it expects a parent
lock class like I_MUTEX_PARENT.

Also, are we missing other locations that lock the orphan directory?
For example, in fs/ocfs2/inode.c:ocfs2_wipe_inode() and
fs/ocfs2/journal.c:ocfs2_recover_orphans(), we still call
inode_lock(orphan_dir_inode). Could using different subclasses for the same
inode corrupt lockdep dependency tracking?

[ ... ]

> @@ -2725,7 +2725,7 @@ int ocfs2_del_inode_from_orphan(struct ocfs2_super *osb,
>  		goto bail;
>  	}
>  

[Severity: Critical]
This is a pre-existing issue, but can the unvalidated on-disk value
di->i_dio_orphaned_slot lead to an out-of-bounds access?

A few lines above, we read the slot directly from the on-disk buffer:

    orphan_dir_inode = ocfs2_get_system_file_inode(osb,
            ORPHAN_DIR_SYSTEM_INODE,
            le16_to_cpu(di->i_dio_orphaned_slot));

If the on-disk slot value is maliciously crafted,
fs/ocfs2/sysfile.c:get_local_system_inode() will use it as an array
index for osb->local_system_inodes, leading to an out-of-bounds read.

Similarly, could this cause an out-of-bounds write in
fs/ocfs2/inode.c:ocfs2_check_orphan_recovery_state() when it increments
osb->osb_orphan_wipes[slot]?

> -	inode_lock(orphan_dir_inode);
> +	inode_lock_nested(orphan_dir_inode, I_MUTEX_NONDIR2);

[ ... ]

> @@ -2838,7 +2838,7 @@ int ocfs2_mv_orphaned_inode_to_new(struct inode *dir,
>  		goto leave;
>  	}
>  
> -	inode_lock(orphan_dir_inode);
> +	inode_lock_nested(orphan_dir_inode, I_MUTEX_NONDIR2);

[Severity: High]
Could using I_MUTEX_NONDIR2 here introduce a new lockdep cycle during
file-over-file rename operations?

In the rename path, vfs_rename() uses lock_two_nondirectories() to lock the
target file with I_MUTEX_NONDIR2:

fs/inode.c:lock_two_nondirectories() {
    ...
    inode_lock_nested(inode2, I_MUTEX_NONDIR2);
    ...
}

If ocfs2_rename() then orphans the overwritten target file, it calls
ocfs2_prepare_orphan_dir(), which will now try to acquire the orphan
directory lock with I_MUTEX_NONDIR2. Could acquiring subclass 4 while
already holding subclass 4 trigger a 'possible recursive locking detected'
warning?

>  
>  	status = ocfs2_inode_lock(orphan_dir_inode, &orphan_dir_bh, 1);
>  	if (status < 0) {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260620231223.46588-1-kartikey406@gmail.com?part=1

Re: [PATCH v2] ocfs2: use inode_lock_nested() for orphan dir locking

[Joseph Qi]

On 6/21/26 7:12 AM, Deepanshu Kartikey wrote:
> PREEMPT_RT's rtmutex PI chain walker warns about a lock dependency
> cycle when inode_lock(orphan_dir_inode) is called while holding
> inode_lock(file_inode):
> 
>   ocfs2_file_write_iter()
>     inode_lock(file_inode)               [class 0]
>       ocfs2_dio_end_io_write()
>         ocfs2_del_inode_from_orphan()
>           inode_lock(orphan_dir_inode)   [class 0] <- warning!
> 
> However this is a false positive. write_iter() is never called on a
> directory, and orphan_dir is always a directory, so these two locks
> can never actually conflict in practice.
> 
> Fix by using inode_lock_nested(orphan_dir_inode, I_MUTEX_NONDIR2) in
> all three places where orphan_dir_inode is locked in namei.c, placing
> it in a separate lock class so the rtmutex PI chain walker understands
> these locks have distinct roles and does not warn about their ordering.
> 
> Suggested-by: Matthew Wilcox <willy@infradead.org>
> Reported-by: syzbot+ce129763ce7d7e914739@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=ce129763ce7d7e914739
> Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>

Looks fine.
Reviewed-by: Joseph Qi <joseph.qi@linux.alibaba.com>

> 
> ---
> Changes in v2:
> - Replaced inode_lock() with inode_lock_nested(orphan_dir_inode,
>   I_MUTEX_NONDIR2) in all three call sites in namei.c
> - Dropped incorrect v1 changes to aops.c and namei.c that
>   restructured lock ordering, as the deadlock was a false positive
> - Approach suggested by Matthew Wilcox
> ---
>  fs/ocfs2/namei.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/ocfs2/namei.c b/fs/ocfs2/namei.c
> index 1277666c77cd..4cfd7b3d3e1a 100644
> --- a/fs/ocfs2/namei.c
> +++ b/fs/ocfs2/namei.c
> @@ -2126,7 +2126,7 @@ static int ocfs2_lookup_lock_orphan_dir(struct ocfs2_super *osb,
>  		return ret;
>  	}
>  
> -	inode_lock(orphan_dir_inode);
> +	inode_lock_nested(orphan_dir_inode, I_MUTEX_NONDIR2);
>  
>  	ret = ocfs2_inode_lock(orphan_dir_inode, &orphan_dir_bh, 1);
>  	if (ret < 0) {
> @@ -2725,7 +2725,7 @@ int ocfs2_del_inode_from_orphan(struct ocfs2_super *osb,
>  		goto bail;
>  	}
>  
> -	inode_lock(orphan_dir_inode);
> +	inode_lock_nested(orphan_dir_inode, I_MUTEX_NONDIR2);
>  	status = ocfs2_inode_lock(orphan_dir_inode, &orphan_dir_bh, 1);
>  	if (status < 0) {
>  		inode_unlock(orphan_dir_inode);
> @@ -2838,7 +2838,7 @@ int ocfs2_mv_orphaned_inode_to_new(struct inode *dir,
>  		goto leave;
>  	}
>  
> -	inode_lock(orphan_dir_inode);
> +	inode_lock_nested(orphan_dir_inode, I_MUTEX_NONDIR2);
>  
>  	status = ocfs2_inode_lock(orphan_dir_inode, &orphan_dir_bh, 1);
>  	if (status < 0) {