Re: [PATCH] ntfs: validate $FILE_NAME length before converting the name

[Hyunchul Lee]

Hi Bryam,

2026년 6월 4일 (목) 오후 2:39, Bryam Vargas <hexlabsecurity@xxxxxxxxx>님이 작성:
>
> ntfs_is_extended_system_file() walks a base inode's $FILE_NAME attributes
> and, for a name whose parent is $Extend, converts the on-disk name to the
> local charset via ntfs_attr_name_get() -> ntfs_ucstonls(). It checks that
> the resident value [file_name_attr, +value_length) lies within the
> attribute, but never checks that file_name_length (an on-disk u8) fits
> inside that value.
>
> ntfs_ucstonls() then reads file_name_length __le16 characters starting at
> file_name_attr->file_name (offset 66 in the __packed struct), so a crafted
> $FILE_NAME whose value_length covers just the fixed header but whose
> file_name_length is 255 drives an out-of-bounds read of up to ~508 bytes
> past the resident value -- and, when the MFT record buffer is a tight
> kmalloc (mft_record_size equal to a power-of-two slab bucket), past the
> allocation itself.
>
> Reject the attribute when file_name_length does not fit within
> value_length before the name is read.
>
> Signed-off-by: Bryam Vargas <hexlabsecurity@xxxxxxxxx>
> ---
> Reproducer omitted on the public list -- available to the maintainers on
> request. This is a crafted-image filesystem bug; posting publicly per the
> security@xxxxxxxxxx guidance on the sibling fs/ntfs attribute-list fixes that
> such bugs fall outside the kernel security process's threat model.
>
> The over-read was observed under KASAN by mounting a crafted NTFS image
> (fs/ntfs built as a module against v7.1-rc5): a $FILE_NAME on a system file in
> $Extend with value_length 68 and file_name_length 255 trips, during inode
> read, a slab-out-of-bounds read of size 2 in utf16s_to_utf8s() along the chain
>
>   ntfs_ucstonls <- ntfs_attr_name_get <- ntfs_is_extended_system_file <-
>   ntfs_read_locked_inode <- ntfs_iget <- ntfs_fill_super
>
> off the kmalloc-1k MFT record buffer (mft_record_size 1024, allocated by
> map_mft_record_folio). A benign mkntfs image is clean on the same kernel
> (control). With this bound in place the crafted $FILE_NAME is rejected with
> -EIO ("Corrupt file name attribute") before the conversion, while a benign
> name (value_length == 66 + file_name_length * 2) still passes. The bound is arch-independent: struct
> file_name_attr is __packed, so offsetof(.., file_name) == 66 and the check
> decides every case identically built -m32/-m64.
>
> The only other reader of file_name_length for name conversion is under
> #ifdef DEBUG in fs/ntfs/namei.c (not built in production); it can be guarded
> the same way if desired.
>
> The flaw is original to the fs/ntfs driver: ntfs_is_extended_system_file()
> landed with the driver itself (linkinjeon/ntfs, v7.1 merge window). A Fixes:
> tag against that introducing commit is appropriate -- I'd appreciate you
> pinning the exact SHA from your tree, as it is not unambiguously citable from
> a mainline clone. checkpatch clean.
>
>  fs/ntfs/inode.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/fs/ntfs/inode.c b/fs/ntfs/inode.c
> index 360bebd1ee3f..d1a79c276502 100644
> --- a/fs/ntfs/inode.c
> +++ b/fs/ntfs/inode.c
> @@ -581,6 +581,17 @@ static int ntfs_is_extended_system_file(struct ntfs_attr_search_ctx *ctx)
>                 p2 = (u8 *)file_name_attr + le32_to_cpu(attr->data.resident.value_length);
>                 if (p2 < (u8 *)attr || p2 > p)
>                         goto err_corrupt_attr;
> +               /*
> +                * The name is converted below; file_name_length is an on-disk
> +                * u8 that is not otherwise cross-checked, so make sure the name
> +                * fits within the resident value before it is read.
> +                */
> +               if (le32_to_cpu(attr->data.resident.value_length) <
> +                               offsetof(struct file_name_attr, file_name) ||
> +                   offsetof(struct file_name_attr, file_name) +
> +                               file_name_attr->file_name_length * sizeof(__le16) >
> +                               le32_to_cpu(attr->data.resident.value_length))
> +                       goto err_corrupt_attr;

There is already a patch that validates this in ntfs_attr_lookup().
Please refer to the link below:
https://lore.kernel.org/all/20260530143514.3083601-2-charsyam@xxxxxxxxx



>                 /* This attribute is ok, but is it in the $Extend directory? */
>                 if (MREF_LE(file_name_attr->parent_directory) == FILE_Extend) {
>                         unsigned char *s;
> --
> 2.43.0
>


--
Thanks,
Hyunchul