mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] fs/ntfs3: reserve NUL byte when converting UTF-16 names
@ 2026-06-11 21:33 Kyle Zeng
  2026-06-21  0:46 ` XIAO WU
  2026-07-23 14:27 ` Konstantin Komarov
  0 siblings, 2 replies; 3+ messages in thread
From: Kyle Zeng @ 2026-06-11 21:33 UTC (permalink / raw)
  To: ntfs3; +Cc: linux-kernel, Konstantin Komarov, outbounddisclosures, Kyle Zeng

ntfs_utf16_to_nls() appends a trailing NUL to the converted output,
but it passes the caller-supplied size directly to the conversion loop.
For the UTF-8 path, utf16s_to_utf8s() can legitimately fill all
buf_len bytes and return buf_len, after which ntfs_utf16_to_nls()
writes the terminator one byte past the end of the destination buffer.

The same contract problem exists for the NLS path when a converted
character consumes the last available byte.

Reserve one byte for the terminator before doing either conversion.
The function continues to return the number of converted bytes,
excluding the NUL terminator.

Assisted-by: Codex:gpt-5.5
Signed-off-by: Kyle Zeng <kylebot@openai.com>
---
 fs/ntfs3/dir.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/fs/ntfs3/dir.c b/fs/ntfs3/dir.c
index d99ab086ef6f..e8892cd94e04 100644
--- a/fs/ntfs3/dir.c
+++ b/fs/ntfs3/dir.c
@@ -25,6 +25,11 @@ int ntfs_utf16_to_nls(struct ntfs_sb_info *sbi, const __le16 *name, u32 len,
 
 	static_assert(sizeof(wchar_t) == sizeof(__le16));
 
+	if (buf_len <= 0)
+		return -EINVAL;
+
+	buf_len -= 1;
+
 	if (!nls) {
 		/* UTF-16 -> UTF-8 */
 		ret = utf16s_to_utf8s((wchar_t *)name, len, UTF16_LITTLE_ENDIAN,
-- 
2.43.0


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

* Re: [PATCH] fs/ntfs3: reserve NUL byte when converting UTF-16 names
  2026-06-11 21:33 [PATCH] fs/ntfs3: reserve NUL byte when converting UTF-16 names Kyle Zeng
@ 2026-06-21  0:46 ` XIAO WU
  2026-07-23 14:27 ` Konstantin Komarov
  1 sibling, 0 replies; 3+ messages in thread
From: XIAO WU @ 2026-06-21  0:46 UTC (permalink / raw)
  To: Kyle Zeng, ntfs3; +Cc: linux-kernel, Konstantin Komarov, outbounddisclosures

From: XIAOWU <xiaowu.417@qq.com>
To: Kyle Zeng <kylebot@openai.com>
Cc: linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH] fs/ntfs3: reserve NUL byte when converting UTF-16 names
In-Reply-To: <20260611213331.16763-1-kylebot@openai.com>

Hi Kyle,

I came across a Sashiko AI code review [1] that flagged a potential
use-after-free in `ntfs_utf16_to_nls()` — specifically, the read of
`sbi->options->nls` without holding sb->s_umount, allowing a concurrent
remount to free the options structure while a directory iteration is in
progress.

I was able to reproduce this in QEMU with KASAN enabled.  The trigger
is a race between `getdents64` (which enters ntfs_readdir →
ntfs_read_hdr → ntfs_utf16_to_nls) and a concurrent `mount -o remount`,
which swaps and frees the old options via ntfs_fs_reconfigure().

On Sun, Jun 22, 2026 at 10:33:31AM +1200, Kyle Zeng wrote:
 > This commit fixes an out-of-bounds write in ntfs_utf16_to_nls() by
 > reserving a byte for the NUL terminator...
...
 > --- a/fs/ntfs3/dir.c
 > +++ b/fs/ntfs3/dir.c
 > @@ -25,6 +25,11 @@ int ntfs_utf16_to_nls(struct ntfs_sb_info *sbi,
 >                       const __le16 *name, u32 len,
 >
 >      static_assert(sizeof(wchar_t) == sizeof(__le16));

At this point, `sbi->options->nls` is dereferenced without any lock
protecting it against concurrent modification:

```c
int ntfs_utf16_to_nls(struct ntfs_sb_info *sbi, ...)
{
     const struct nls_table *nls = sbi->options->nls;  // unprotected read
```

Meanwhile, a concurrent remount path does:

     swap(sbi->options, fc->fs_private);  // in ntfs_fs_reconfigure()
     ...
     put_mount_options(opts);             // kfree() the old options

If the free lands between the nls pointer being loaded and being
dereferenced, the directory iteration hits freed memory.

[Reproduction]

I set up an NTFS filesystem image and ran two threads in parallel:
one calling getdents64 in a loop, the other calling mount -o remount.
The race triggered a KASAN report within a few seconds.

[KASAN report — kernel 7.1.0-rc6+, CONFIG_KASAN=y]

   ==================================================================
   BUG: KASAN: slab-use-after-free in ntfs_utf16_to_nls+0x563/0x5f0
   Read of size 8 at addr ffff888026403028 by task poc/9531

   Call Trace:
    <TASK>
    dump_stack_lvl+0x116/0x1f0
    print_report+0xf4/0x600
    kasan_report+0xe0/0x110
    ntfs_utf16_to_nls+0x563/0x5f0
    ntfs_read_hdr+0x6a7/0xb60
    ntfs_readdir+0x6ef/0x10a0
    iterate_dir+0x336/0x560
    __do_sys_getdents64+0x1de/0x380
    do_syscall_64+0xcd/0xf80
    entry_SYSCALL_64_after_hwframe+0x77/0x7f

   Allocated by task 9532:
    ntfs_init_fs_context+0x...      // remount allocates new options
    alloc_mount_options+0x...
    kfree+0x...

   Freed by task 9532:
    put_mount_options+0x...         // old options freed during remount
    ntfs_fs_free+0x...

The crash is a read of the freed `sbi->options->nls` pointer. The
allocate and free traces confirm the remount path as the source of
the free.

[1] 
https://sashiko.dev/#/patchset/20260611213331.16763-1-kylebot%40openai.com
     (Sashiko AI code review — "Use-After-Free", Severity: High)

Thanks,
XIAO



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

* Re: [PATCH] fs/ntfs3: reserve NUL byte when converting UTF-16 names
  2026-06-11 21:33 [PATCH] fs/ntfs3: reserve NUL byte when converting UTF-16 names Kyle Zeng
  2026-06-21  0:46 ` XIAO WU
@ 2026-07-23 14:27 ` Konstantin Komarov
  1 sibling, 0 replies; 3+ messages in thread
From: Konstantin Komarov @ 2026-07-23 14:27 UTC (permalink / raw)
  To: Kyle Zeng, ntfs3; +Cc: linux-kernel, outbounddisclosures

On 6/11/26 23:33, Kyle Zeng wrote:

> ntfs_utf16_to_nls() appends a trailing NUL to the converted output,
> but it passes the caller-supplied size directly to the conversion loop.
> For the UTF-8 path, utf16s_to_utf8s() can legitimately fill all
> buf_len bytes and return buf_len, after which ntfs_utf16_to_nls()
> writes the terminator one byte past the end of the destination buffer.
>
> The same contract problem exists for the NLS path when a converted
> character consumes the last available byte.
>
> Reserve one byte for the terminator before doing either conversion.
> The function continues to return the number of converted bytes,
> excluding the NUL terminator.
>
> Assisted-by: Codex:gpt-5.5
> Signed-off-by: Kyle Zeng <kylebot@openai.com>
> ---
>   fs/ntfs3/dir.c | 5 +++++
>   1 file changed, 5 insertions(+)
>
> diff --git a/fs/ntfs3/dir.c b/fs/ntfs3/dir.c
> index d99ab086ef6f..e8892cd94e04 100644
> --- a/fs/ntfs3/dir.c
> +++ b/fs/ntfs3/dir.c
> @@ -25,6 +25,11 @@ int ntfs_utf16_to_nls(struct ntfs_sb_info *sbi, const __le16 *name, u32 len,
>
>          static_assert(sizeof(wchar_t) == sizeof(__le16));
>
> +       if (buf_len <= 0)
> +               return -EINVAL;
> +
> +       buf_len -= 1;
> +
>          if (!nls) {
>                  /* UTF-16 -> UTF-8 */
>                  ret = utf16s_to_utf8s((wchar_t *)name, len, UTF16_LITTLE_ENDIAN,
> --
> 2.43.0
>
Hello,

Sorry for the delay.
Your patch was applied, thank you.

Regards,
Konstantin


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

end of thread, other threads:[~2026-07-23 14:27 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-06-11 21:33 [PATCH] fs/ntfs3: reserve NUL byte when converting UTF-16 names Kyle Zeng
2026-06-21  0:46 ` XIAO WU
2026-07-23 14:27 ` Konstantin Komarov

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®