Re: [PATCH v2] ntfs: validate resident attribute values on lookup

[Namjae Jeon]

On Tue, May 26, 2026 at 1:33 AM DaeMyung Kang <charsyam@xxxxxxxxx> wrote:
>
> ntfs_attr_find() and ntfs_external_attr_find() check that generic
> resident attribute values fit in their attribute records and that
> fixed-size resident values are large enough. For variable-length resident
> formats, however, the fixed part is not enough: embedded length fields
> can still point callers past the resident value.
>
> A crafted image can set a small resident $FILE_NAME value_length while
> leaving file_name_length large. Callers then trust file_name_length and
> read past the resident value when converting or comparing the name. This
> was reproduced with a crafted image under KASAN as a slab-out-of-bounds
> read from the kmalloc-1k MFT record copy. The stack included
> ntfs_lookup(), ntfs_iget(), ntfs_read_locked_inode(), ntfs_attr_name_get(),
> ntfs_ucstonls(), and utf16s_to_utf8s().
>
> Add a shared resident attribute value validator and use it before a
> lookup path can return an attribute, including the AT_UNUSED enumeration
> case where callers inspect returned attributes directly. The helper
> validates resident value bounds, minimum value sizes, and variable-length
> $FILE_NAME and $INDEX_ROOT fields, while preserving the fixed-size
> checks for attributes such as $VOLUME_INFORMATION and $EA_INFORMATION.
> Log corruption at the rejection point with the attribute name where
> known.
>
> Reject non-resident $FILE_NAME records too: the format requires
> $FILE_NAME to be resident and callers treat returned records as
> resident.
>
> Type-specific format validation for $VOLUME_NAME is intentionally not
> added to this helper, even though $VOLUME_NAME is a variable-length
> resident attribute. Existing callers such as ntfs_write_volume_label()
> treat a failed AT_VOLUME_NAME lookup as an absent label and
> unconditionally add a new record afterwards. If the helper rejected
> odd-length labels at lookup time, the corrupt record would stay in
> place and the caller would append a new record next to it, producing
> two $VOLUME_NAME attributes on disk. Caller-side cleanup must precede
> any lookup-time rejection of malformed $VOLUME_NAME payloads, so the
> type-specific check is left for a follow-up patch. The generic resident
> bounds checks (value_offset / value_length / attr_len) still apply to
> $VOLUME_NAME, and its entry in the type-name table is kept so that
> those failures log with the attribute name.
>
> Fixes: 6ceb4cc81ef3 ("ntfs: add bound checking to ntfs_attr_find")
> Signed-off-by: DaeMyung Kang <charsyam@xxxxxxxxx>
> ---
> Patch 1/2 of the previous series ("ntfs: free link name from
> ntfs_name_cache") has been applied to ntfs-next; this v2 contains only
> the remaining patch, with the resident attribute validation reworked per
> review feedback.
>
> Changes since v1:
> - Refactor resident attribute validation into a shared helper
>   ntfs_attr_value_is_valid() used by both ntfs_attr_find() and
>   ntfs_external_attr_find(), and remove the duplicated inline check
>   from ntfs_external_attr_find().
> - Split out resident header/value bounds extraction into
>   ntfs_resident_attr_value_get(), returning a small view struct so the
>   top-level orchestration reads as: get view -> common min check ->
>   per-type format check.
> - Validate before the AT_UNUSED enumeration path in ntfs_attr_find()
>   can return the attribute to callers that inspect it directly.
> - Reject non-resident $FILE_NAME records: the format requires
>   $FILE_NAME to be resident and callers treat returned records as
>   resident.
> - Extend type-specific validation beyond $FILE_NAME to $INDEX_ROOT
>   (entries_offset / index_length / allocated_size bounds and 8-byte
>   alignment); add $INDEX_ROOT to ntfs_resident_attr_min_value_length().
> - Type-specific format validation for $VOLUME_NAME is intentionally
>   not added; see the commit message for the caller-side reason.
>   Generic resident bounds checks still apply, and the $VOLUME_NAME
>   entry in the type-name table is kept so that those failures log
>   with a readable attribute name.
> - Log corruption at the rejection point with the attribute type name
>   via a new ntfs_attr_type_name() helper, matching the existing
>   attribute-name corruption message in ntfs_attr_find().
Please check xfstests generic/001 test
failure.(https://github.com/namjaejeon/linux-ntfs/actions/runs/26424312505/job/77785594232)

FSTYP -- ntfs
570PLATFORM -- Linux/x86_64 runnervmg397c 6.17.0-1013-azure
#13~24.04.1-Ubuntu SMP Wed Apr 15 16:52:17 UTC 2026
571MKFS_OPTIONS -- -q --partition-start=0 --sectors-per-track=0
--heads=0 /dev/loop21
572MOUNT_OPTIONS -- /dev/loop21 /mnt/scratch
573
574generic/001 _check_generic_filesystem: filesystem on /dev/loop20 is
inconsistent
575(see /home/runner/work/linux-ntfs/linux-ntfs/exfat-testsuites/xfstests-exfat/results//generic/001.full
for details)
576Trying to repair broken TEST_DEV file system
577- output mismatch (see
/home/runner/work/linux-ntfs/linux-ntfs/exfat-testsuites/xfstests-exfat/results//generic/001.out.bad)
578 --- tests/generic/001.out 2025-01-14 04:54:54.000000000 +0000
579 +++ /home/runner/work/linux-ntfs/linux-ntfs/exfat-testsuites/xfstests-exfat/results//generic/001.out.bad
2026-05-26 00:01:44.663029196 +0000
580 @@ -7,3 +7,4 @@
581 iter 4 chain ... check ....................................
582 iter 5 chain ... check ....................................
583 cleanup
584 +rm: cannot remove '/mnt/test/83726/sub': Input/output error
585 ...
586 (Run 'diff -u
/home/runner/work/linux-ntfs/linux-ntfs/exfat-testsuites/xfstests-exfat/tests/generic/001.out
/home/runner/work/linux-ntfs/linux-ntfs/exfat-testsuites/xfstests-exfat/results//generic/001.out.bad'
to see the entire diff)
587Ran: generic/001
588Failures: generic/001
589Failed 1 of 1 tests