linux-kernel - [PATCH v2] ocfs2: fix circular locking dependency in ocfs2_acquire

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20260108141931.67110-1-swilczek.lx@gmail.com>
Date: Thu,  8 Jan 2026 15:19:31 +0100
From: Szymon Wilczek <swilczek.lx@...il.com>
To: ocfs2-devel@...ts.linux.dev
Cc: joseph.qi@...ux.alibaba.com,
	mark@...heh.com,
	jlbec@...lplan.org,
	linux-kernel@...r.kernel.org,
	syzkaller-bugs@...glegroups.com,
	akpm@...ux-foundation.org,
	Szymon Wilczek <swilczek.lx@...il.com>,
	syzbot+51244a05705883616c95@...kaller.appspotmail.com
Subject: [PATCH v2] ocfs2: fix circular locking dependency in ocfs2_acquire_dquot

Move ocfs2_extend_no_holes() and ocfs2_start_trans() to execute before
ocfs2_lock_global_qf() to fix a circular locking dependency reported
by syzbot.

The issue occurs because ocfs2_start_trans() acquires sb_internal (via
sb_start_intwrite) while already holding quota file locks (ip_alloc_sem
and sysfile_lock_key). This conflicts with freeze/dismount paths that
acquire sb_internal first, creating the following circular dependency:

  sb_internal -> sysfile_lock_key -> ip_alloc_sem (freeze/dismount)
  ip_alloc_sem -> sysfile_lock_key -> sb_internal (quota acquire)

Fix this by reordering the operations:
1. Speculatively extend quota file (uses need_alloc estimate)
2. Start transaction
3. Acquire global quota file lock
4. Perform quota operations
5. Release lock
6. Commit transaction

This ensures sb_internal is acquired before quota file locks, matching
the expected kernel lock order.

Reported-by: syzbot+51244a05705883616c95@...kaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug\?extid\=51244a05705883616c95
Signed-off-by: Szymon Wilczek <swilczek.lx@...il.com>
---
 fs/ocfs2/quota_global.c | 49 +++++++++++++++++++++++------------------
 1 file changed, 27 insertions(+), 22 deletions(-)

diff --git a/fs/ocfs2/quota_global.c b/fs/ocfs2/quota_global.c
index e85b1ccf81be..95bb901820cf 100644
--- a/fs/ocfs2/quota_global.c
+++ b/fs/ocfs2/quota_global.c
@@ -821,13 +821,35 @@ static int ocfs2_acquire_dquot(struct dquot *dquot)
 	trace_ocfs2_acquire_dquot(from_kqid(&init_user_ns, dquot->dq_id),
 				  type);
 	mutex_lock(&dquot->dq_lock);
+
+	/*
+	 * Speculatively extend quota file before acquiring any locks or
+	 * starting transaction to avoid lock inversion. sb_start_intwrite
+	 * (via ocfs2_start_trans) ranks above quota file locks.
+	 */
+	if (need_alloc) {
+		WARN_ON(journal_current_handle());
+		status = ocfs2_extend_no_holes(gqinode, NULL,
+			i_size_read(gqinode) + (need_alloc << sb->s_blocksize_bits),
+			i_size_read(gqinode));
+		if (status < 0)
+			goto out;
+	}
+
+	handle = ocfs2_start_trans(osb,
+				   ocfs2_calc_global_qinit_credits(sb, type));
+	if (IS_ERR(handle)) {
+		status = PTR_ERR(handle);
+		goto out;
+	}
+
 	/*
 	 * We need an exclusive lock, because we're going to update use count
 	 * and instantiate possibly new dquot structure
 	 */
 	status = ocfs2_lock_global_qf(info, 1);
 	if (status < 0)
-		goto out;
+		goto out_trans;
 	status = ocfs2_qinfo_lock(info, 0);
 	if (status < 0)
 		goto out_dq;
@@ -843,29 +865,12 @@ static int ocfs2_acquire_dquot(struct dquot *dquot)
 	OCFS2_DQUOT(dquot)->dq_use_count++;
 	OCFS2_DQUOT(dquot)->dq_origspace = dquot->dq_dqb.dqb_curspace;
 	OCFS2_DQUOT(dquot)->dq_originodes = dquot->dq_dqb.dqb_curinodes;
-	if (!dquot->dq_off) {	/* No real quota entry? */
+	if (!dquot->dq_off)	/* No real quota entry? */
 		ex = 1;
-		/*
-		 * Add blocks to quota file before we start a transaction since
-		 * locking allocators ranks above a transaction start
-		 */
-		WARN_ON(journal_current_handle());
-		status = ocfs2_extend_no_holes(gqinode, NULL,
-			i_size_read(gqinode) + (need_alloc << sb->s_blocksize_bits),
-			i_size_read(gqinode));
-		if (status < 0)
-			goto out_dq;
-	}
 
-	handle = ocfs2_start_trans(osb,
-				   ocfs2_calc_global_qinit_credits(sb, type));
-	if (IS_ERR(handle)) {
-		status = PTR_ERR(handle);
-		goto out_dq;
-	}
 	status = ocfs2_qinfo_lock(info, ex);
 	if (status < 0)
-		goto out_trans;
+		goto out_dq;
 	status = qtree_write_dquot(&info->dqi_gi, dquot);
 	if (ex && info_dirty(sb_dqinfo(sb, type))) {
 		err = __ocfs2_global_write_info(sb, type);
@@ -873,10 +878,10 @@ static int ocfs2_acquire_dquot(struct dquot *dquot)
 			status = err;
 	}
 	ocfs2_qinfo_unlock(info, ex);
-out_trans:
-	ocfs2_commit_trans(osb, handle);
 out_dq:
 	ocfs2_unlock_global_qf(info, 1);
+out_trans:
+	ocfs2_commit_trans(osb, handle);
 	if (status < 0)
 		goto out;
 
-- 
2.52.0