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

[CharSyam]

I will check it soon. Thanks.

2026년 5월 26일 (화) 오전 9:09, Namjae Jeon <linkinjeon@xxxxxxxxxx>님이 작성:
>
> 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