Forwarded: [PATCH] ocfs2: fix deadlock between dio write and orphan dir inode lock

[syzbot <syzbot+ce129763ce7d7e914739@syzkaller.appspotmail.com>]

For archival purposes, forwarding an incoming command email to
linux-kernel@vger.kernel.org, syzkaller-bugs@googlegroups.com.

***

Subject: [PATCH] ocfs2: fix deadlock between dio write and orphan dir inode lock
Author: kartikey406@gmail.com

#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master


The following deadlock is detected by PREEMPT_RT's rtmutex detector:

  Thread 1 (pwritev2 -> ocfs2_file_write_iter):
    inode_lock(file_inode)            <- held here
      ocfs2_dio_end_io_write()
        ocfs2_inode_lock()            <- cluster lock
          ocfs2_del_inode_from_orphan()
            inode_lock(orphan_dir)    <- deadlock!

  Thread 2 (recovery path):
    inode_lock(orphan_dir)            <- held first
      inode_lock(file_inode)          <- reverse order!

ocfs2_file_write_iter() acquires inode_lock(file_inode) and holds it
across the entire write including the synchronous DIO completion
callback ocfs2_dio_end_io_write(). When dwc->dw_orphaned is set,
ocfs2_dio_end_io_write() calls ocfs2_del_inode_from_orphan() which
acquires inode_lock(orphan_dir_inode), creating an AB-BA lock
inversion with recovery paths that acquire orphan_dir lock first.

Fix this by:
- Saving the orphaned state before releasing locks
- Moving ocfs2_inode_unlock() and brelse(di_bh) BEFORE the call to
  ocfs2_del_inode_from_orphan() in ocfs2_dio_end_io_write()
- Passing NULL for di_bh to ocfs2_del_inode_from_orphan() to signal
  it should acquire its own cluster lock and di_bh internally
- In ocfs2_del_inode_from_orphan(), changing di initialization from
  a declaration+assignment to just a declaration, then acquiring a
  fresh ocfs2_inode_lock() when di_bh is NULL, and releasing it
  in the bail path

Reported-by: syzbot+ce129763ce7d7e914739@syzkaller.appspotmail.com
Link: https://syzkaller.appspot.com/bug?extid=ce129763ce7d7e914739
Signed-off-by: Deepanshu kartikey <kartikey406@gmail.com>
---
 fs/ocfs2/aops.c  | 14 ++++++++------
 fs/ocfs2/namei.c | 17 ++++++++++++++++-
 2 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/fs/ocfs2/aops.c b/fs/ocfs2/aops.c
index 6ec198bdab12..d1d22019dc61 100644
--- a/fs/ocfs2/aops.c
+++ b/fs/ocfs2/aops.c
@@ -2280,6 +2280,7 @@ static int ocfs2_dio_end_io_write(struct inode *inode,
 	handle_t *handle = NULL;
 	loff_t end = offset + bytes;
 	int ret = 0, credits = 0, batch = 0;
+	bool orphaned = false;
 
 	ocfs2_init_dealloc_ctxt(&dealloc);
 
@@ -2371,17 +2372,18 @@ static int ocfs2_dio_end_io_write(struct inode *inode,
 		ocfs2_commit_trans(osb, handle);
 unlock:
 	up_write(&oi->ip_alloc_sem);
+	orphaned = (!ret && dwc->dw_orphaned);
+	ocfs2_inode_unlock(inode, 1);
+	brelse(di_bh);
+	di_bh = NULL;
 
-	/* everything looks good, let's start the cleanup */
-	if (!ret && dwc->dw_orphaned) {
+	/* everything looks good, let's start the orphan cleanup */
+	if (orphaned) {
 		BUG_ON(dwc->dw_writer_pid != task_pid_nr(current));
-
-		ret = ocfs2_del_inode_from_orphan(osb, inode, di_bh, 0, 0);
+		ret = ocfs2_del_inode_from_orphan(osb, inode, NULL, 0, 0);
 		if (ret < 0)
 			mlog_errno(ret);
 	}
-	ocfs2_inode_unlock(inode, 1);
-	brelse(di_bh);
 out:
 	if (data_ac)
 		ocfs2_free_alloc_context(data_ac);
diff --git a/fs/ocfs2/namei.c b/fs/ocfs2/namei.c
index 1277666c77cd..25bbe2a9776e 100644
--- a/fs/ocfs2/namei.c
+++ b/fs/ocfs2/namei.c
@@ -2712,10 +2712,21 @@ int ocfs2_del_inode_from_orphan(struct ocfs2_super *osb,
 {
 	struct inode *orphan_dir_inode = NULL;
 	struct buffer_head *orphan_dir_bh = NULL;
-	struct ocfs2_dinode *di = (struct ocfs2_dinode *)di_bh->b_data;
+	struct ocfs2_dinode *di;
 	handle_t *handle = NULL;
 	int status = 0;
+	struct buffer_head *local_di_bh = NULL;
 
+	if (!di_bh) {
+		status = ocfs2_inode_lock(inode, &local_di_bh, 1);
+		if (status < 0) {
+			mlog_errno(status);
+			return status;
+		}
+		di_bh = local_di_bh;
+	}
+
+	di = (struct ocfs2_dinode *)di_bh->b_data;
 	orphan_dir_inode = ocfs2_get_system_file_inode(osb,
 			ORPHAN_DIR_SYSTEM_INODE,
 			le16_to_cpu(di->i_dio_orphaned_slot));
@@ -2779,6 +2790,10 @@ int ocfs2_del_inode_from_orphan(struct ocfs2_super *osb,
 	iput(orphan_dir_inode);
 
 bail:
+	if (local_di_bh) {
+		ocfs2_inode_unlock(inode, 1);
+		brelse(local_di_bh);
+	}
 	return status;
 }
 
-- 
2.43.0