All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH RFC v4] ocfs2: fix circular locking dependency in ocfs2_init_acl()
@ 2026-07-20 17:34 syzbot
  2026-07-21  9:33 ` [syzbot ci] " syzbot ci
  0 siblings, 1 reply; 3+ messages in thread
From: syzbot @ 2026-07-20 17:34 UTC (permalink / raw)
  To: syzkaller-upstream-moderation; +Cc: krystianmkaniewski, syzbot

A lockdep warning indicates a circular locking dependency between
`&oi->ip_xattr_sem` and `&journal->j_trans_barrier`.

The deadlock involves two code paths: Path 1 (setxattr) where
`ocfs2_xattr_set()` acquires `ip_xattr_sem` (write) and then starts a
transaction, which acquires `j_trans_barrier` (read); and Path 2
(mkdir/mknod) where `ocfs2_mknod()` starts a transaction (`j_trans_barrier`
read) and then calls `ocfs2_init_acl()`, which attempts to acquire
`ip_xattr_sem` (read) on the parent directory to retrieve the default ACL.

Because rw_semaphores are subject to writer priority, a pending writer on
`j_trans_barrier` (e.g., the journal commit thread) can cause Path 1 to
block, while Path 2 is blocked waiting for Path 1 to release
`ip_xattr_sem`.

The patch fixes the lock ordering by precomputing the ACL state before
starting the OCFS2 transaction, while preserving POSIX ACL storage
semantics and the existing inode/security initialization order. By reading
the parent directory's default ACL and preparing the new inode's ACLs
outside the transaction, `ip_xattr_sem` is always acquired before
`j_trans_barrier`.

We introduce `struct ocfs2_acl_state` to encapsulate the prepared ACL
state, along with `ocfs2_acl_init_prepare()` and `ocfs2_acl_init_release()`
helpers to avoid code duplication between `ocfs2_mknod()` and
`ocfs2_init_security_and_acl()`. `ocfs2_calc_xattr_init()` and
`ocfs2_init_acl()` are updated to use this precomputed state, removing
internal `ip_xattr_sem` acquisition and redundant disk reads.

Fixes: 16c8d569f570 ("ocfs2/acl: use 'ip_xattr_sem' to protect getting extended attribute")
Assisted-by: Gemini:gemini-3.5-flash Gemini:gemini-3.1-pro-preview syzbot
Reported-by: syzbot+4007ab5229e732466d9f@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=4007ab5229e732466d9f
Link: https://syzkaller.appspot.com/ai_job?id=1241036a-ad93-4dc6-8823-45a6f15e1dd8
To: "Joel Becker" <jlbec@evilplan.org>
To: "Joseph Qi" <joseph.qi@linux.alibaba.com>
To: "Mark Fasheh" <mark@fasheh.com>
To: <ocfs2-devel@lists.linux.dev>
Cc: <linux-kernel@vger.kernel.org>

---
v4:
- Initialize `ocfs2_acl_state` structures to `{ 0 }` to ensure safe cleanup.
- Fix memory leaks in `ocfs2_acl_init_prepare()` error paths.
- Ensure `ocfs2_acl_init_release()` is always called in `ocfs2_init_security_and_acl()`.
- Reset pointers to `NULL` in `ocfs2_acl_init_release()` after releasing them.
- Add parameter names to `ocfs2_calc_xattr_init()` prototype in `xattr.h`.

v3:
- Introduced `struct ocfs2_acl_state` to encapsulate the prepared ACL state.
- Added `ocfs2_acl_init_prepare()` and `ocfs2_acl_init_release()` helpers to avoid code duplication between `ocfs2_mknod()` and `ocfs2_init_security_and_acl()`.
- Updated `ocfs2_calc_xattr_init()` and `ocfs2_init_acl()` to accept the new `ocfs2_acl_state` structure.
https://lore.kernel.org/all/857bb6cb-547c-48fb-ae38-b9ffff129432@mail.kernel.org/T/

v2:
- Updated ocfs2_calc_xattr_init() to accept precomputed ACLs for more accurate credit and size calculation.
- Modified ocfs2_mknod() and ocfs2_init_security_and_acl() to fetch and create ACLs before starting a transaction, resolving the lock ordering issue.
- Ensured inode->i_mode is updated correctly during the pre-transaction ACL creation phase.
- Added logic to release the access ACL object if it is not required for storage (i.e., matches the file mode).
https://lore.kernel.org/all/50178043-8171-44ae-ac9e-72449ba95928@mail.kernel.org/T/

v1:
https://lore.kernel.org/all/bbc14e84-b364-4f93-90e0-bfe42220b2b1@mail.kernel.org/T/
---
diff --git a/fs/ocfs2/acl.c b/fs/ocfs2/acl.c
index af1e2cedb..090ec60fb 100644
--- a/fs/ocfs2/acl.c
+++ b/fs/ocfs2/acl.c
@@ -110,8 +110,7 @@ static void *ocfs2_acl_to_xattr(const struct posix_acl *acl, size_t *size)
 	return ocfs2_acl;
 }
 
-static struct posix_acl *ocfs2_get_acl_nolock(struct inode *inode,
-					      int type,
+static struct posix_acl *ocfs2_get_acl_nolock(struct inode *inode, int type,
 					      struct buffer_head *di_bh)
 {
 	int name_index;
@@ -349,63 +348,105 @@ int ocfs2_acl_chmod(struct inode *inode, struct buffer_head *bh)
  * Initialize the ACLs of a new inode. If parent directory has default ACL,
  * then clone to new inode. Called from ocfs2_mknod.
  */
-int ocfs2_init_acl(handle_t *handle,
-		   struct inode *inode,
-		   struct inode *dir,
-		   struct buffer_head *di_bh,
-		   struct buffer_head *dir_bh,
-		   struct ocfs2_alloc_context *meta_ac,
-		   struct ocfs2_alloc_context *data_ac)
+void ocfs2_acl_init_release(struct ocfs2_acl_state *state)
+{
+	posix_acl_release(state->default_acl);
+	posix_acl_release(state->acl);
+	state->default_acl = NULL;
+	state->acl = NULL;
+}
+
+int ocfs2_acl_init_prepare(struct inode *inode, struct inode *dir,
+			   struct buffer_head *dir_bh,
+			   struct ocfs2_acl_state *state)
 {
 	struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
-	struct posix_acl *acl = NULL;
-	int ret = 0, ret2;
-	umode_t mode;
-
-	if (!S_ISLNK(inode->i_mode)) {
-		if (osb->s_mount_opt & OCFS2_MOUNT_POSIX_ACL) {
-			down_read(&OCFS2_I(dir)->ip_xattr_sem);
-			acl = ocfs2_get_acl_nolock(dir, ACL_TYPE_DEFAULT,
-						   dir_bh);
-			up_read(&OCFS2_I(dir)->ip_xattr_sem);
-			if (IS_ERR(acl))
-				return PTR_ERR(acl);
+	int ret = 0;
+
+	state->default_acl = NULL;
+	state->acl = NULL;
+	state->mode = inode->i_mode;
+
+	if (S_ISLNK(inode->i_mode))
+		return 0;
+
+	if (osb->s_mount_opt & OCFS2_MOUNT_POSIX_ACL) {
+		down_read(&OCFS2_I(dir)->ip_xattr_sem);
+		state->default_acl =
+			ocfs2_get_acl_nolock(dir, ACL_TYPE_DEFAULT, dir_bh);
+		up_read(&OCFS2_I(dir)->ip_xattr_sem);
+		if (IS_ERR(state->default_acl)) {
+			ret = PTR_ERR(state->default_acl);
+			state->default_acl = NULL;
+			return ret;
 		}
-		if (!acl) {
-			mode = inode->i_mode & ~current_umask();
-			ret = ocfs2_acl_set_mode(inode, di_bh, handle, mode);
-			if (ret) {
-				mlog_errno(ret);
+		if (state->default_acl) {
+			state->acl = posix_acl_dup(state->default_acl);
+			if (!state->acl) {
+				ret = -ENOMEM;
 				goto cleanup;
 			}
+			ret = __posix_acl_create(&state->acl, GFP_NOFS,
+						 &state->mode);
+			if (ret < 0)
+				goto cleanup;
+			if (ret == 0) {
+				posix_acl_release(state->acl);
+				state->acl = NULL;
+			}
+			if (!S_ISDIR(inode->i_mode)) {
+				posix_acl_release(state->default_acl);
+				state->default_acl = NULL;
+			}
+		} else {
+			state->mode &= ~current_umask();
 		}
+	} else {
+		state->mode &= ~current_umask();
 	}
-	if ((osb->s_mount_opt & OCFS2_MOUNT_POSIX_ACL) && acl) {
-		if (S_ISDIR(inode->i_mode)) {
+
+	return 0;
+cleanup:
+	ocfs2_acl_init_release(state);
+	return ret;
+}
+
+int ocfs2_init_acl(handle_t *handle, struct inode *inode,
+		   struct buffer_head *di_bh,
+		   struct ocfs2_alloc_context *meta_ac,
+		   struct ocfs2_alloc_context *data_ac,
+		   struct ocfs2_acl_state *state)
+{
+	struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
+	int ret = 0;
+
+	if (S_ISLNK(inode->i_mode))
+		return 0;
+
+	if (osb->s_mount_opt & OCFS2_MOUNT_POSIX_ACL) {
+		if (S_ISDIR(inode->i_mode) && state->default_acl) {
 			ret = ocfs2_set_acl(handle, inode, di_bh,
-					    ACL_TYPE_DEFAULT, acl,
-					    meta_ac, data_ac);
+					    ACL_TYPE_DEFAULT,
+					    state->default_acl, meta_ac,
+					    data_ac);
 			if (ret)
-				goto cleanup;
+				return ret;
 		}
-		mode = inode->i_mode;
-		ret = __posix_acl_create(&acl, GFP_NOFS, &mode);
-		if (ret < 0)
-			return ret;
+	}
 
-		ret2 = ocfs2_acl_set_mode(inode, di_bh, handle, mode);
-		if (ret2) {
-			mlog_errno(ret2);
-			ret = ret2;
-			goto cleanup;
-		}
-		if (ret > 0) {
-			ret = ocfs2_set_acl(handle, inode,
-					    di_bh, ACL_TYPE_ACCESS,
-					    acl, meta_ac, data_ac);
+	ret = ocfs2_acl_set_mode(inode, di_bh, handle, state->mode);
+	if (ret) {
+		mlog_errno(ret);
+		return ret;
+	}
+
+	if (osb->s_mount_opt & OCFS2_MOUNT_POSIX_ACL) {
+		if (state->acl) {
+			ret = ocfs2_set_acl(handle, inode, di_bh,
+					    ACL_TYPE_ACCESS, state->acl,
+					    meta_ac, data_ac);
 		}
 	}
-cleanup:
-	posix_acl_release(acl);
+
 	return ret;
 }
diff --git a/fs/ocfs2/acl.h b/fs/ocfs2/acl.h
index 667c6f03f..a91f9ce27 100644
--- a/fs/ocfs2/acl.h
+++ b/fs/ocfs2/acl.h
@@ -20,9 +20,20 @@ struct posix_acl *ocfs2_iop_get_acl(struct inode *inode, int type, bool rcu);
 int ocfs2_iop_set_acl(struct mnt_idmap *idmap, struct dentry *dentry,
 		      struct posix_acl *acl, int type);
 extern int ocfs2_acl_chmod(struct inode *, struct buffer_head *);
-extern int ocfs2_init_acl(handle_t *, struct inode *, struct inode *,
-			  struct buffer_head *, struct buffer_head *,
-			  struct ocfs2_alloc_context *,
-			  struct ocfs2_alloc_context *);
+struct ocfs2_acl_state {
+	struct posix_acl *default_acl;
+	struct posix_acl *acl;
+	umode_t mode;
+};
+
+int ocfs2_acl_init_prepare(struct inode *inode, struct inode *dir,
+			   struct buffer_head *dir_bh,
+			   struct ocfs2_acl_state *state);
+void ocfs2_acl_init_release(struct ocfs2_acl_state *state);
+int ocfs2_init_acl(handle_t *handle, struct inode *inode,
+		   struct buffer_head *di_bh,
+		   struct ocfs2_alloc_context *meta_ac,
+		   struct ocfs2_alloc_context *data_ac,
+		   struct ocfs2_acl_state *state);
 
 #endif /* OCFS2_ACL_H */
diff --git a/fs/ocfs2/namei.c b/fs/ocfs2/namei.c
index 1277666c7..d9aba16a8 100644
--- a/fs/ocfs2/namei.c
+++ b/fs/ocfs2/namei.c
@@ -256,6 +256,7 @@ static int ocfs2_mknod(struct mnt_idmap *idmap,
 	sigset_t oldset;
 	int did_block_signals = 0;
 	struct ocfs2_dentry_lock *dl = NULL;
+	struct ocfs2_acl_state acl_state = { 0 };
 
 	trace_ocfs2_mknod(dir, dentry, dentry->d_name.len, dentry->d_name.name,
 			  (unsigned long long)OCFS2_I(dir)->ip_blkno,
@@ -330,10 +331,13 @@ static int ocfs2_mknod(struct mnt_idmap *idmap,
 		}
 	}
 
+	status = ocfs2_acl_init_prepare(inode, dir, parent_fe_bh, &acl_state);
+	if (status < 0)
+		goto leave;
+
 	/* calculate meta data/clusters for setting security and acl xattr */
-	status = ocfs2_calc_xattr_init(dir, parent_fe_bh, mode,
-				       &si, &want_clusters,
-				       &xattr_credits, &want_meta);
+	status = ocfs2_calc_xattr_init(dir, mode, &si, &want_clusters,
+				       &xattr_credits, &want_meta, &acl_state);
 	if (status < 0) {
 		mlog_errno(status);
 		goto leave;
@@ -411,8 +415,8 @@ static int ocfs2_mknod(struct mnt_idmap *idmap,
 		inc_nlink(dir);
 	}
 
-	status = ocfs2_init_acl(handle, inode, dir, new_fe_bh, parent_fe_bh,
-			 meta_ac, data_ac);
+	status = ocfs2_init_acl(handle, inode, new_fe_bh, meta_ac, data_ac,
+				&acl_state);
 
 	if (status < 0) {
 		mlog_errno(status);
@@ -477,6 +481,8 @@ static int ocfs2_mknod(struct mnt_idmap *idmap,
 	brelse(parent_fe_bh);
 	kfree(si.value);
 
+	ocfs2_acl_init_release(&acl_state);
+
 	ocfs2_free_dir_lookup_result(&lookup);
 
 	if (inode_ac)
diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
index fcddd3c13..a03c24df8 100644
--- a/fs/ocfs2/xattr.c
+++ b/fs/ocfs2/xattr.c
@@ -611,13 +611,10 @@ int ocfs2_calc_security_init(struct inode *dir,
 	return ret;
 }
 
-int ocfs2_calc_xattr_init(struct inode *dir,
-			  struct buffer_head *dir_bh,
-			  umode_t mode,
+int ocfs2_calc_xattr_init(struct inode *dir, umode_t mode,
 			  struct ocfs2_security_xattr_info *si,
-			  int *want_clusters,
-			  int *xattr_credits,
-			  int *want_meta)
+			  int *want_clusters, int *xattr_credits,
+			  int *want_meta, struct ocfs2_acl_state *acl_state)
 {
 	int ret = 0;
 	struct ocfs2_super *osb = OCFS2_SB(dir->i_sb);
@@ -628,19 +625,15 @@ int ocfs2_calc_xattr_init(struct inode *dir,
 						     si->value_len);
 
 	if (osb->s_mount_opt & OCFS2_MOUNT_POSIX_ACL) {
-		down_read(&OCFS2_I(dir)->ip_xattr_sem);
-		acl_len = ocfs2_xattr_get_nolock(dir, dir_bh,
-					OCFS2_XATTR_INDEX_POSIX_ACL_DEFAULT,
-					"", NULL, 0);
-		up_read(&OCFS2_I(dir)->ip_xattr_sem);
-		if (acl_len > 0) {
-			a_size = ocfs2_xattr_entry_real_size(0, acl_len);
-			if (S_ISDIR(mode))
-				a_size <<= 1;
-		} else if (acl_len != 0 && acl_len != -ENODATA) {
-			ret = acl_len;
-			mlog_errno(ret);
-			return ret;
+		if (acl_state->default_acl && S_ISDIR(mode)) {
+			acl_len = acl_state->default_acl->a_count *
+				  sizeof(struct ocfs2_acl_entry);
+			a_size += ocfs2_xattr_entry_real_size(0, acl_len);
+		}
+		if (acl_state->acl) {
+			acl_len = acl_state->acl->a_count *
+				  sizeof(struct ocfs2_acl_entry);
+			a_size += ocfs2_xattr_entry_real_size(0, acl_len);
 		}
 	}
 
@@ -683,14 +676,29 @@ int ocfs2_calc_xattr_init(struct inode *dir,
 							   new_clusters);
 		*want_clusters += new_clusters;
 	}
-	if (osb->s_mount_opt & OCFS2_MOUNT_POSIX_ACL &&
-	    acl_len > OCFS2_XATTR_INLINE_SIZE) {
-		/* for directory, it has DEFAULT and ACCESS two types of acls */
-		new_clusters = (S_ISDIR(mode) ? 2 : 1) *
-				ocfs2_clusters_for_bytes(dir->i_sb, acl_len);
-		*xattr_credits += ocfs2_clusters_to_blocks(dir->i_sb,
-							   new_clusters);
-		*want_clusters += new_clusters;
+	if (osb->s_mount_opt & OCFS2_MOUNT_POSIX_ACL) {
+		if (acl_state->default_acl && S_ISDIR(mode)) {
+			acl_len = acl_state->default_acl->a_count *
+				  sizeof(struct ocfs2_acl_entry);
+			if (acl_len > OCFS2_XATTR_INLINE_SIZE) {
+				new_clusters = ocfs2_clusters_for_bytes(
+					dir->i_sb, acl_len);
+				*xattr_credits += ocfs2_clusters_to_blocks(
+					dir->i_sb, new_clusters);
+				*want_clusters += new_clusters;
+			}
+		}
+		if (acl_state->acl) {
+			acl_len = acl_state->acl->a_count *
+				  sizeof(struct ocfs2_acl_entry);
+			if (acl_len > OCFS2_XATTR_INLINE_SIZE) {
+				new_clusters = ocfs2_clusters_for_bytes(
+					dir->i_sb, acl_len);
+				*xattr_credits += ocfs2_clusters_to_blocks(
+					dir->i_sb, new_clusters);
+				*want_clusters += new_clusters;
+			}
+		}
 	}
 
 	return ret;
@@ -7257,6 +7265,7 @@ int ocfs2_init_security_and_acl(struct inode *dir,
 {
 	int ret = 0;
 	struct buffer_head *dir_bh = NULL;
+	struct ocfs2_acl_state acl_state = { 0 };
 
 	ret = ocfs2_init_security_get(inode, dir, qstr, NULL);
 	if (ret) {
@@ -7269,10 +7278,17 @@ int ocfs2_init_security_and_acl(struct inode *dir,
 		mlog_errno(ret);
 		goto leave;
 	}
-	ret = ocfs2_init_acl(NULL, inode, dir, NULL, dir_bh, NULL, NULL);
+
+	ret = ocfs2_acl_init_prepare(inode, dir, dir_bh, &acl_state);
+	if (ret)
+		goto unlock;
+
+	ret = ocfs2_init_acl(NULL, inode, NULL, NULL, NULL, &acl_state);
 	if (ret)
 		mlog_errno(ret);
 
+unlock:
+	ocfs2_acl_init_release(&acl_state);
 	ocfs2_inode_unlock(dir, 0);
 	brelse(dir_bh);
 leave:
diff --git a/fs/ocfs2/xattr.h b/fs/ocfs2/xattr.h
index 65e9aa743..5cdd6c6b4 100644
--- a/fs/ocfs2/xattr.h
+++ b/fs/ocfs2/xattr.h
@@ -55,9 +55,12 @@ int ocfs2_init_security_set(handle_t *, struct inode *,
 int ocfs2_calc_security_init(struct inode *,
 			     struct ocfs2_security_xattr_info *,
 			     int *, int *, struct ocfs2_alloc_context **);
-int ocfs2_calc_xattr_init(struct inode *, struct buffer_head *,
-			  umode_t, struct ocfs2_security_xattr_info *,
-			  int *, int *, int *);
+
+struct ocfs2_acl_state;
+int ocfs2_calc_xattr_init(struct inode *dir, umode_t mode,
+			  struct ocfs2_security_xattr_info *si,
+			  int *want_clusters, int *xattr_credits,
+			  int *want_meta, struct ocfs2_acl_state *acl_state);
 
 /*
  * xattrs can live inside an inode, as part of an external xattr block,


base-commit: dc59e4fea9d83f03bad6bddf3fa2e52491777482
-- 
This is an AI-generated patch subject to moderation.
Reply with '#syz upstream' to Sign-off the patch as a human author
and send it to the upstream kernel mailing lists.
Reply with '#syz reject' to reject it ('#syz unreject' to undo).

See https://goo.gle/syzbot-ai-patches for information about AI-generated patches.
You can comment on the patch as usual, syzbot will try to address
the comments and send a new version of the patch if necessary.
syzbot engineers can be reached at syzkaller@googlegroups.com.

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [syzbot ci] Re: ocfs2: fix circular locking dependency in ocfs2_init_acl()
  2026-07-20 17:34 [PATCH RFC v4] ocfs2: fix circular locking dependency in ocfs2_init_acl() syzbot
@ 2026-07-21  9:33 ` syzbot ci
  2026-07-21 20:47   ` Krystian Kaniewski
  0 siblings, 1 reply; 3+ messages in thread
From: syzbot ci @ 2026-07-21  9:33 UTC (permalink / raw)
  To: krystianmkaniewski, syzbot, syzbot, syzkaller-upstream-moderation
  Cc: syzbot, syzkaller-bugs

syzbot ci has tested the following series

[v4] ocfs2: fix circular locking dependency in ocfs2_init_acl()
https://lore.kernel.org/all/b2e72251-ea1a-458d-961e-7b7e99950e26@mail.kernel.org
* [PATCH RFC v4] ocfs2: fix circular locking dependency in ocfs2_init_acl()

and found no issues.

Full report is available here:
https://ci.syzbot.org/series/78a8c280-1c08-431d-aab1-f2054f103460

***

If these findings have caused you to resend the series or submit a
separate fix, please add the following tag to your commit message:
  Tested-by: syzbot@syzkaller.appspotmail.com

---
This report is generated by a bot. It may contain errors.
syzbot ci engineers can be reached at syzkaller@googlegroups.com.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [syzbot ci] Re: ocfs2: fix circular locking dependency in ocfs2_init_acl()
  2026-07-21  9:33 ` [syzbot ci] " syzbot ci
@ 2026-07-21 20:47   ` Krystian Kaniewski
  0 siblings, 0 replies; 3+ messages in thread
From: Krystian Kaniewski @ 2026-07-21 20:47 UTC (permalink / raw)
  To: syzbot ci, syzbot, syzbot, syzkaller-upstream-moderation; +Cc: syzkaller-bugs

V4 correctly moves the parent default ACL lookup and ACL derivation 
before allocator reservation and ocfs2_start_trans(). It also fixes the 
v3 ownership problems by zero-initializing the ACL state, clearing 
released pointers, cleaning the state on every preparation error, and 
releasing it on the reflink failure path. Keep those changes, together 
with the current POSIX ACL result semantics, mode and security ordering, 
and exact reservation calculations.

One lock-order problem remains. ocfs2_mknod() starts its transaction and 
then calls ocfs2_init_acl(). When a directory inherits a default ACL, or 
when the derived access ACL must be stored, ocfs2_set_acl() reaches 
ocfs2_xattr_set_handle(). Initial security labels reach the same helper 
through ocfs2_init_security_set(). ocfs2_xattr_set_handle() still takes 
inode->ip_xattr_sem while the create path already holds allocation 
resources, sb_internal, and j_trans_barrier.

The ordinary ocfs2_xattr_set() path establishes the reverse dependency 
by taking ip_xattr_sem before reserving metadata and starting its 
transaction. All ip_xattr_sem instances are initialized at the same call 
site and therefore share one lockdep class. It does not matter to 
lockdep that the create path locks the new inode rather than the parent. 
A create under a parent with a default ACL can still establish the same 
circular class dependency.

The existing reproducer does not prove this path is fixed. It sets 
system.posix_acl_access rather than an inheritable 
system.posix_acl_default, so mkdir can avoid both default and access ACL 
xattr writes after the transaction starts. It also does not necessarily 
force an initial security xattr.

Required corrections:

1. Audit every caller of ocfs2_xattr_set_handle(). Confirm and document 
that this helper is used only while initializing a new inode that has 
not yet been inserted into the inode hash or attached to a dentry.
2. Make creation-time ACL and security xattr writes avoid acquiring the 
normal ip_xattr_sem class after allocator reservation and transaction 
start. A suitable approach is to remove the unnecessary semaphore 
acquisition from the private new-inode helper or split out an explicit 
no-lock creation helper. If serialization is still required, acquire it 
before the allocator resources and transaction, with balanced cleanup on 
every exit.
3. Keep the locking in the ordinary ocfs2_xattr_set() path unchanged. Do 
not suppress a real dependency with a lockdep annotation unless the 
private-inode lifetime argument is explicit and valid for every caller.
4. Preserve the v4 ACL state ownership fixes, __posix_acl_create() 
zero-versus-positive handling, inode owner and security initialization 
order, symlink and no-POSIX-ACL behavior, reflink behavior, and separate 
default/access ACL reservation calculations.
5. Clean up the four expression line breaks in ocfs2_calc_xattr_init() 
that end immediately after an opening parenthesis.

On 7/21/2026 11:33 AM, syzbot ci wrote:
> syzbot ci has tested the following series
>
> [v4] ocfs2: fix circular locking dependency in ocfs2_init_acl()
> https://lore.kernel.org/all/b2e72251-ea1a-458d-961e-7b7e99950e26@mail.kernel.org
> * [PATCH RFC v4] ocfs2: fix circular locking dependency in ocfs2_init_acl()
>
> and found no issues.
>
> Full report is available here:
> https://ci.syzbot.org/series/78a8c280-1c08-431d-aab1-f2054f103460
>
> ***
>
> If these findings have caused you to resend the series or submit a
> separate fix, please add the following tag to your commit message:
>    Tested-by: syzbot@syzkaller.appspotmail.com
>
> ---
> This report is generated by a bot. It may contain errors.
> syzbot ci engineers can be reached at syzkaller@googlegroups.com.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-07-21 20:47 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-20 17:34 [PATCH RFC v4] ocfs2: fix circular locking dependency in ocfs2_init_acl() syzbot
2026-07-21  9:33 ` [syzbot ci] " syzbot ci
2026-07-21 20:47   ` Krystian Kaniewski

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.