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@xxxxxxxxxxxxxxxxxxxxxxxxx
> 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@xxxxxxxxx>
> ---
> 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