mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] jfs: fix directory tree corruption in dtSplitRoot()
@ 2025-12-07  6:51 Deepanshu Kartikey
  2025-12-07  6:55 ` Deepanshu Kartikey
  0 siblings, 1 reply; 3+ messages in thread
From: Deepanshu Kartikey @ 2025-12-07  6:51 UTC (permalink / raw)
  To: shaggy, brauner
  Cc: mjguzik, jfs-discussion, linux-kernel, Deepanshu Kartikey,
	syzbot+a099d674daa27a9272db

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 2833 bytes --]

When reusing transaction locks for DTREE operations, the index field
may contain stale values from previous operations, causing assertion
failures in dtSplitRoot():

  ASSERT(dtlck->index == 0)

This results in kernel crashes like:

  kernel BUG at fs/jfs/jfs_dtree.c:1942!
  Call Trace:
   dtSplitRoot+0x1694/0x16c0 fs/jfs/jfs_dtree.c:1942
   dtSplitUp fs/jfs/jfs_dtree.c:1244 [inline]
   dtInsert+0x2525/0x5f40 fs/jfs/jfs_dtree.c:871
   jfs_create+0x6c8/0xa80 fs/jfs/namei.c:137

The bug occurs because txLock() has multiple code paths for lock
acquisition:

1. Fresh allocation (allocateLock) - correctly initializes index to 0
2. Lock reuse (same transaction) - skips initialization
3. Anonymous lock acquisition - skips initialization

Paths 2 and 3 jump directly to the grantLock label, bypassing the
index initialization. When dtSplitRoot() is called multiple times
within a batched transaction (which JFS uses for performance), it may
receive a reused lock with index=3 from a previous operation instead
of the expected index=0.

Example sequence:
  Transaction tid=1:
    - First dtSplitRoot: gets fresh lock, index=0 ✓
    - Modifies entries, index becomes 3
    - Lock returned to pool but not freed

  Transaction tid=1 (continues):
    - Second dtSplitRoot: reuses same lock
    - index still = 3 (stale value) ✗
    - ASSERT(index == 0) fails → crash

Fix by resetting dtlck->index to 0 at the grantLock label, but only
for operations with the tlckNEW flag set. This ensures:

- New pages (like dtSplitRoot) start with clean state (index=0)
- Existing pages preserve accumulated changes within a transaction
- No performance impact (only affects new page operations)

The tlckNEW flag is used by dtSplitRoot() when creating a new root
page, making this fix targeted to the exact scenario that requires
index=0.

Reported-by: syzbot+a099d674daa27a9272db@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=a099d674daa27a9272db
Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
---
 fs/jfs/jfs_txnmgr.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/fs/jfs/jfs_txnmgr.c b/fs/jfs/jfs_txnmgr.c
index c16578af3a77..5ce2fc17d967 100644
--- a/fs/jfs/jfs_txnmgr.c
+++ b/fs/jfs/jfs_txnmgr.c
@@ -811,6 +811,17 @@ struct tlock *txLock(tid_t tid, struct inode *ip, struct metapage * mp,
 	 * update tlock vector
 	 */
       grantLock:
+	/*
+	 * Reset index for new DTREE locks to ensure clean state.
+	 * When locks are reused, index may contain stale values from
+	 * previous operations. Operations like dtSplitRoot() expect
+	 * index to be 0 when creating new pages (tlckNEW flag).
+	 */
+	if ((type & tlckDTREE) && (type & tlckNEW)) {
+		struct dt_lock *dtlck = (struct dt_lock *)&tlck->lock;
+
+		dtlck->index = 0;
+	}
 	tlck->type |= type;
 
 	return tlck;
-- 
2.43.0


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

* Re: [PATCH] jfs: fix directory tree corruption in dtSplitRoot()
  2025-12-07  6:51 [PATCH] jfs: fix directory tree corruption in dtSplitRoot() Deepanshu Kartikey
@ 2025-12-07  6:55 ` Deepanshu Kartikey
  0 siblings, 0 replies; 3+ messages in thread
From: Deepanshu Kartikey @ 2025-12-07  6:55 UTC (permalink / raw)
  To: shaggy, brauner
  Cc: mjguzik, jfs-discussion, linux-kernel, syzbot+a099d674daa27a9272db

On Sun, Dec 7, 2025 at 12:21 PM Deepanshu Kartikey
<kartikey406@gmail.com> wrote:
>
> When reusing transaction locks for DTREE operations, the index field
> may contain stale values from previous operations, causing assertion
> failures in dtSplitRoot():
>
>   ASSERT(dtlck->index == 0)
>
> This results in kernel crashes like:
>
>   kernel BUG at fs/jfs/jfs_dtree.c:1942!
>   Call Trace:
>    dtSplitRoot+0x1694/0x16c0 fs/jfs/jfs_dtree.c:1942
>    dtSplitUp fs/jfs/jfs_dtree.c:1244 [inline]
>    dtInsert+0x2525/0x5f40 fs/jfs/jfs_dtree.c:871
>    jfs_create+0x6c8/0xa80 fs/jfs/namei.c:137
>
> The bug occurs because txLock() has multiple code paths for lock
> acquisition:
>
> 1. Fresh allocation (allocateLock) - correctly initializes index to 0
> 2. Lock reuse (same transaction) - skips initialization
> 3. Anonymous lock acquisition - skips initialization
>
> Paths 2 and 3 jump directly to the grantLock label, bypassing the
> index initialization. When dtSplitRoot() is called multiple times
> within a batched transaction (which JFS uses for performance), it may
> receive a reused lock with index=3 from a previous operation instead
> of the expected index=0.
>
> Example sequence:
>   Transaction tid=1:
>     - First dtSplitRoot: gets fresh lock, index=0 ✓
>     - Modifies entries, index becomes 3
>     - Lock returned to pool but not freed
>
>   Transaction tid=1 (continues):
>     - Second dtSplitRoot: reuses same lock
>     - index still = 3 (stale value) ✗
>     - ASSERT(index == 0) fails → crash
>
> Fix by resetting dtlck->index to 0 at the grantLock label, but only
> for operations with the tlckNEW flag set. This ensures:
>
> - New pages (like dtSplitRoot) start with clean state (index=0)
> - Existing pages preserve accumulated changes within a transaction
> - No performance impact (only affects new page operations)
>
> The tlckNEW flag is used by dtSplitRoot() when creating a new root
> page, making this fix targeted to the exact scenario that requires
> index=0.
>
> Reported-by: syzbot+a099d674daa27a9272db@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=a099d674daa27a9272db
> Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
> ---
>  fs/jfs/jfs_txnmgr.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/fs/jfs/jfs_txnmgr.c b/fs/jfs/jfs_txnmgr.c
> index c16578af3a77..5ce2fc17d967 100644
> --- a/fs/jfs/jfs_txnmgr.c
> +++ b/fs/jfs/jfs_txnmgr.c
> @@ -811,6 +811,17 @@ struct tlock *txLock(tid_t tid, struct inode *ip, struct metapage * mp,
>          * update tlock vector
>          */
>        grantLock:
> +       /*
> +        * Reset index for new DTREE locks to ensure clean state.
> +        * When locks are reused, index may contain stale values from
> +        * previous operations. Operations like dtSplitRoot() expect
> +        * index to be 0 when creating new pages (tlckNEW flag).
> +        */
> +       if ((type & tlckDTREE) && (type & tlckNEW)) {
> +               struct dt_lock *dtlck = (struct dt_lock *)&tlck->lock;
> +
> +               dtlck->index = 0;
> +       }
>         tlck->type |= type;
>
>         return tlck;
> --
> 2.43.0
>

Please disregard this patch. As,  it contains special characters. I
will be resending the patch again

Thanks

Deepanshu

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

* [PATCH] jfs: fix directory tree corruption in dtSplitRoot()
@ 2025-12-07  6:58 Deepanshu Kartikey
  0 siblings, 0 replies; 3+ messages in thread
From: Deepanshu Kartikey @ 2025-12-07  6:58 UTC (permalink / raw)
  To: shaggy, brauner
  Cc: mjguzik, jfs-discussion, linux-kernel, Deepanshu Kartikey,
	syzbot+a099d674daa27a9272db

When reusing transaction locks for DTREE operations, the index field
may contain stale values from previous operations, causing assertion
failures in dtSplitRoot():

  ASSERT(dtlck->index == 0)

This results in kernel crashes like:

  kernel BUG at fs/jfs/jfs_dtree.c:1942!
  Call Trace:
   dtSplitRoot+0x1694/0x16c0 fs/jfs/jfs_dtree.c:1942
   dtSplitUp fs/jfs/jfs_dtree.c:1244 [inline]
   dtInsert+0x2525/0x5f40 fs/jfs/jfs_dtree.c:871
   jfs_create+0x6c8/0xa80 fs/jfs/namei.c:137

The bug occurs because txLock() has multiple code paths for lock
acquisition:

1. Fresh allocation (allocateLock) - correctly initializes index to 0
2. Lock reuse (same transaction) - skips initialization
3. Anonymous lock acquisition - skips initialization

Paths 2 and 3 jump directly to the grantLock label, bypassing the
index initialization. When dtSplitRoot() is called multiple times
within a batched transaction (which JFS uses for performance), it may
receive a reused lock with index=3 from a previous operation instead
of the expected index=0.

Fix by resetting dtlck->index to 0 at the grantLock label, but only
for operations with the tlckNEW flag set. This ensures new pages start
with clean state while preserving accumulated changes on existing pages.

Reported-by: syzbot+a099d674daa27a9272db@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=a099d674daa27a9272db
Tested-by: Deepanshu Kartikey <kartikey406@gmail.com>
Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
---
 fs/jfs/jfs_txnmgr.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/fs/jfs/jfs_txnmgr.c b/fs/jfs/jfs_txnmgr.c
index c16578af3a77..5ce2fc17d967 100644
--- a/fs/jfs/jfs_txnmgr.c
+++ b/fs/jfs/jfs_txnmgr.c
@@ -811,6 +811,17 @@ struct tlock *txLock(tid_t tid, struct inode *ip, struct metapage * mp,
 	 * update tlock vector
 	 */
       grantLock:
+	/*
+	 * Reset index for new DTREE locks to ensure clean state.
+	 * When locks are reused, index may contain stale values from
+	 * previous operations. Operations like dtSplitRoot() expect
+	 * index to be 0 when creating new pages (tlckNEW flag).
+	 */
+	if ((type & tlckDTREE) && (type & tlckNEW)) {
+		struct dt_lock *dtlck = (struct dt_lock *)&tlck->lock;
+
+		dtlck->index = 0;
+	}
 	tlck->type |= type;
 
 	return tlck;
-- 
2.43.0


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

end of thread, other threads:[~2025-12-07  6:58 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-12-07  6:51 [PATCH] jfs: fix directory tree corruption in dtSplitRoot() Deepanshu Kartikey
2025-12-07  6:55 ` Deepanshu Kartikey
2025-12-07  6:58 Deepanshu Kartikey

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®