[PATCH] fs/ntfs3: fix double ni_unlock() in ntfs_readdir()

[PATCH] fs/ntfs3: fix double ni_unlock() in ntfs_readdir()

[Christian Aurich Zanettini Martins]

Commit da4391c496fe ("ntfs3: release ni_lock before dir_emit in
ntfs_readdir to fix deadlocks") drops ni_lock before calling
ntfs_read_hdr() in ntfs_readdir(). If either call returns non-zero,
execution jumps to out, which calls ni_unlock() even though the lock
is no longer held.

This also happens when getdents64() fills the supplied buffer,
triggering a lockdep "bad unlock balance" warning.

Jump to out_free instead to avoid unlocking an already released lock.

Reported-by: syzbot+5c0ed4f2974a8b100ed4@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=5c0ed4f2974a8b100ed4
Fixes: da4391c496fe ("ntfs3: release ni_lock before dir_emit in ntfs_readdir to fix deadlocks")
Signed-off-by: Christian Aurich Zanettini Martins <christian.aurichzm@gmail.com>
---
Based on Paragon's ntfs3 master (da4391c496fe), also in linux-next.

Tested in QEMU with lockdep. A 128-byte getdents64() buffer
reproduced the warning before the fix. After the fix, no warning
occurred when reading root-only and 700-entry directories,
including four concurrent readers and a writer.

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

diff --git a/fs/ntfs3/dir.c b/fs/ntfs3/dir.c
index b54828735d55..45d6a41a7222 100644
--- a/fs/ntfs3/dir.c
+++ b/fs/ntfs3/dir.c
@@ -563,7 +563,7 @@ static int ntfs_readdir(struct file *file, struct dir_context *ctx)
 		 */
 		err = ntfs_read_hdr(sbi, ni, &root->ihdr, 0, pos, name, ctx);
 		if (err)
-			goto out;
+			goto out_free;

 		ni_lock(ni);
 		bit = 0;
@@ -607,7 +607,7 @@ static int ntfs_readdir(struct file *file, struct dir_context *ctx)
 				    ((u64)bit << index_bits) + sbi->record_size,
 				    pos, name, ctx);
 		if (err)
-			goto out;
+			goto out_free;

 		ni_lock(ni);
 	}
@@ -615,6 +615,7 @@ static int ntfs_readdir(struct file *file, struct dir_context *ctx)
 out:
 	ni_unlock(ni);

+out_free:
 	kfree(name);
 	put_indx_node(node);

base-commit: da4391c496fedc1ca78a99907712f2256ce8fb80

Re: [PATCH] fs/ntfs3: fix double ni_unlock() in ntfs_readdir()

[Zhou, Yun]

On 10/24/26 06:35, Christian Aurich Zanettini Martins wrote:
> Commit da4391c496fe ("ntfs3: release ni_lock before dir_emit in
> ntfs_readdir to fix deadlocks") drops ni_lock before calling
> ntfs_read_hdr() in ntfs_readdir(). If either call returns non-zero,
> execution jumps to out, which calls ni_unlock() even though the lock
> is no longer held.
> 

The issue does indeed exist in the current linux-next (next-20261027),
but it was not introduced by da4391c496. When da4391c496 was committed
on July 1, fb058ccff57 had not yet been applied; fb058ccff57 was only
applied on October 8. This patch made the following changes (directly
altering the position of the out label):

-out_unlock:
+out:
         ni_unlock(ni);

-out:
         kfree(name);
         put_indx_node(node);

The issue is caused by a conflict between two patches from different
time periods.

> This also happens when getdents64() fills the supplied buffer,
> triggering a lockdep "bad unlock balance" warning.
> 
> Jump to out_free instead to avoid unlocking an already released lock.
> 
> Reported-by: syzbot+5c0ed4f2974a8b100ed4@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=5c0ed4f2974a8b100ed4
> Fixes: da4391c496fe ("ntfs3: release ni_lock before dir_emit in ntfs_readdir to fix deadlocks")
> Signed-off-by: Christian Aurich Zanettini Martins <christian.aurichzm@gmail.com>
> ---
> Based on Paragon's ntfs3 master (da4391c496fe), also in linux-next.
> 
> Tested in QEMU with lockdep. A 128-byte getdents64() buffer
> reproduced the warning before the fix. After the fix, no warning
> occurred when reading root-only and 700-entry directories,
> including four concurrent readers and a writer.
> 
>   fs/ntfs3/dir.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/ntfs3/dir.c b/fs/ntfs3/dir.c
> index b54828735d55..45d6a41a7222 100644
> --- a/fs/ntfs3/dir.c
> +++ b/fs/ntfs3/dir.c
> @@ -563,7 +563,7 @@ static int ntfs_readdir(struct file *file, struct dir_context *ctx)
>                   */
>                  err = ntfs_read_hdr(sbi, ni, &root->ihdr, 0, pos, name, ctx);
>                  if (err)
> -                       goto out;
> +                       goto out_free;
> 
>                  ni_lock(ni);
>                  bit = 0;
> @@ -607,7 +607,7 @@ static int ntfs_readdir(struct file *file, struct dir_context *ctx)
>                                      ((u64)bit << index_bits) + sbi->record_size,
>                                      pos, name, ctx);
>                  if (err)
> -                       goto out;
> +                       goto out_free;
> 
>                  ni_lock(ni);
>          }
> @@ -615,6 +615,7 @@ static int ntfs_readdir(struct file *file, struct dir_context *ctx)
>   out:
>          ni_unlock(ni);
> 
> +out_free:
>          kfree(name);
>          put_indx_node(node);
> 
> base-commit: da4391c496fedc1ca78a99907712f2256ce8fb80

Re: [PATCH] fs/ntfs3: fix double ni_unlock() in ntfs_readdir()

[Christian Aurich Zanettini Martins]

On 10/29/26 11:25, Zhou, Yun wrote:
> The issue is caused by a conflict between two patches from different
> time periods.

Thanks, you're right. I'll send a v2 with the commit message fixed.

I'll keep the Fixes tag on your commit (7a5aa6c45072 after the rebase),
since that's where the bug first shows up in the tree.