[PATCH] ocfs2: don't reuse SINGLE_DEPTH_NESTING for unpublished inode ip_alloc_sem

[PATCH] ocfs2: don't reuse SINGLE_DEPTH_NESTING for unpublished inode ip_alloc_sem

[Nguyen Ngoc Thang]

Commit c204bf5ac3ee ("ocfs2: fix deadlock in inline-data truncate
transactions") made ocfs2_xattr_set_handle() take ip_alloc_sem of a
not-yet-published inode inside the create transaction, under subclass
OCFS2_IP_ALLOC_SEM_UNPUBLISHED = 1, so lockdep records:

  sb_internal -> j_trans_barrier -> ip_alloc_sem/1

But subclass 1 is SINGLE_DEPTH_NESTING, which ocfs2_remap_file_range()
already uses for the destination inode, and the remap then starts a
transaction in ocfs2_duplicate_inline_data():

  ip_alloc_sem/1 -> sb_internal

Lockdep merges the two and reports a circular dependency.  It cannot
deadlock, since the unpublished inode is unreachable by anyone else,
but the warning hides real reports.

Give the unpublished subclass its own value past SINGLE_DEPTH_NESTING.

Fixes: c204bf5ac3ee ("ocfs2: fix deadlock in inline-data truncate transactions")
Reported-by: syzbot+3f66416993279938e957@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=3f66416993279938e957
Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>
---
Tested in QEMU (x86_64, syzbot config with lockdep) using the syzbot C
reproducer on top of 23c62304dde2:
  - before: "possible circular locking dependency" after ~8s
  - after:  no lockdep report in 3 x 180s runs, debug_locks stays 1;
            /proc/lockdep shows both ip_alloc_sem/2 (create) and
            ip_alloc_sem/1 (remap) exercised.

 fs/ocfs2/xattr.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
index 71e5986bafaa..9f2d5797e4d6 100644
--- a/fs/ocfs2/xattr.c
+++ b/fs/ocfs2/xattr.c
@@ -3867,10 +3867,11 @@ static int __ocfs2_xattr_set_handle(struct inode *inode,
  * ip_alloc_sem there adds a transaction -> ip_alloc_sem order that would
  * form a lockdep cycle with the ip_alloc_sem -> transaction order used
  * elsewhere, if not for this separate subclass.  The inode is unpublished
- * so the acquisition can never contend.
+ * so the acquisition can never contend.  It must not collide with the
+ * SINGLE_DEPTH_NESTING subclass ocfs2_remap_file_range() uses.
  */
 enum {
-	OCFS2_IP_ALLOC_SEM_UNPUBLISHED = 1,
+	OCFS2_IP_ALLOC_SEM_UNPUBLISHED = SINGLE_DEPTH_NESTING + 1,
 };
 
 /*
-- 
2.43.0

Re: [PATCH] ocfs2: don't reuse SINGLE_DEPTH_NESTING for unpublished inode ip_alloc_sem

[Joseph Qi]

On 11/15/26 6:46 PM, Nguyen Ngoc Thang wrote:
> Commit c204bf5ac3ee ("ocfs2: fix deadlock in inline-data truncate
> transactions") made ocfs2_xattr_set_handle() take ip_alloc_sem of a
> not-yet-published inode inside the create transaction, under subclass
> OCFS2_IP_ALLOC_SEM_UNPUBLISHED = 1, so lockdep records:
> 
>   sb_internal -> j_trans_barrier -> ip_alloc_sem/1
> 
> But subclass 1 is SINGLE_DEPTH_NESTING, which ocfs2_remap_file_range()
> already uses for the destination inode, and the remap then starts a
> transaction in ocfs2_duplicate_inline_data():
> 
>   ip_alloc_sem/1 -> sb_internal
> 
> Lockdep merges the two and reports a circular dependency.  It cannot
> deadlock, since the unpublished inode is unreachable by anyone else,
> but the warning hides real reports.
> 
> Give the unpublished subclass its own value past SINGLE_DEPTH_NESTING.
> 
> Fixes: c204bf5ac3ee ("ocfs2: fix deadlock in inline-data truncate transactions")
> Reported-by: syzbot+3f66416993279938e957@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=3f66416993279938e957
> Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>

It takes no runtime effect. But the collision is real.
Reviewed-by: Joseph Qi <joseph.qi@linux.alibaba.com>
 
> ---
> Tested in QEMU (x86_64, syzbot config with lockdep) using the syzbot C
> reproducer on top of 23c62304dde2:
>   - before: "possible circular locking dependency" after ~8s
>   - after:  no lockdep report in 3 x 180s runs, debug_locks stays 1;
>             /proc/lockdep shows both ip_alloc_sem/2 (create) and
>             ip_alloc_sem/1 (remap) exercised.
> 
>  fs/ocfs2/xattr.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
> index 71e5986bafaa..9f2d5797e4d6 100644
> --- a/fs/ocfs2/xattr.c
> +++ b/fs/ocfs2/xattr.c
> @@ -3867,10 +3867,11 @@ static int __ocfs2_xattr_set_handle(struct inode *inode,
>   * ip_alloc_sem there adds a transaction -> ip_alloc_sem order that would
>   * form a lockdep cycle with the ip_alloc_sem -> transaction order used
>   * elsewhere, if not for this separate subclass.  The inode is unpublished
> - * so the acquisition can never contend.
> + * so the acquisition can never contend.  It must not collide with the
> + * SINGLE_DEPTH_NESTING subclass ocfs2_remap_file_range() uses.
>   */
>  enum {
> -	OCFS2_IP_ALLOC_SEM_UNPUBLISHED = 1,
> +	OCFS2_IP_ALLOC_SEM_UNPUBLISHED = SINGLE_DEPTH_NESTING + 1,
>  };
>  
>  /*