Re: [PATCH v3 2/3] ntfs: bound the look-ahead attribute-list entry in ntfs_external_attr_find()

[Hyunchul Lee]

2026년 6월 4일 (목) 오후 1:29, Bryam Vargas <hexlabsecurity@xxxxxxxxx>님이 작성:
>
> When resolving an attribute lookup with a non-zero @lowest_vcn,
> ntfs_external_attr_find() peeks at the next $ATTRIBUTE_LIST entry to
> decide whether to keep searching, but bounds that not-yet-validated
> entry only with "(u8 *)next_al_entry + 6 < al_end" (which proves just
> bytes 0..6 are in range) and "(u8 *)next_al_entry + length <= al_end"
> with an attacker-controlled, non-8-aligned length. It then reads
> next_al_entry->lowest_vcn (an __le64 at offset 8) and the name at
> next_al_entry->name_offset, both of which can lie past al_end -- the
> exact end of the kvmalloc'd attribute-list buffer (allocated at the
> on-disk attr_list_size, no rounding). A crafted on-disk $ATTRIBUTE_LIST
> whose last entry sits a few bytes before al_end therefore yields a slab
> out-of-bounds read when the inode is read.
>
> Validate the look-ahead entry with ntfs_attr_list_entry_is_valid() (added
> in patch 1/3) before dereferencing lowest_vcn and the name, so the same
> fixed-header, length and name bounds the main attribute-list walk uses now
> guard this read too.
>
> Signed-off-by: Bryam Vargas <hexlabsecurity@xxxxxxxxx>

Looks good to me.

Reviewed-by: Hyunchul Lee <hyc.lee@xxxxxxxxx>

> ---
> v3 (Hyunchul Lee review): use the extracted ntfs_attr_list_entry_is_valid()
>    helper (added in 1/3) instead of an open-coded bound, so the look-ahead
>    gets the same fixed-header, 8-byte-aligned-length and name checks as the
>    main walk.
> v2: dropped the redundant Reported-by; reproducer/KASAN splat omitted on the
>    public list (available to the maintainers on request).
>
> Confirmed under KASAN on the earlier revision: a slab out-of-bounds read of
> next_al_entry->lowest_vcn during inode read of a crafted image; a benign
> mkntfs image is KASAN-clean (control). Arch-independent (lowest_vcn at
> offset 8, fixed header offsetof(.., name) == 26; identical geometry built
> -m32/-m64). Please add the appropriate Fixes: tag for the commit that
> introduced ntfs_external_attr_find() in the new fs/ntfs driver (v7.1 merge
> window).
>
>  fs/ntfs/attrib.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/fs/ntfs/attrib.c b/fs/ntfs/attrib.c
> index abc0add6f0c4..e425c8d074c5 100644
> --- a/fs/ntfs/attrib.c
> +++ b/fs/ntfs/attrib.c
> @@ -1185,9 +1185,8 @@ static int ntfs_external_attr_find(const __le32 type,
>                  * we have reached the right one or the search has failed.
>                  */
>                 if (lowest_vcn && (u8 *)next_al_entry >= al_start &&
> -                               (u8 *)next_al_entry + 6 < al_end &&
> -                               (u8 *)next_al_entry + le16_to_cpu(
> -                                       next_al_entry->length) <= al_end &&
> +                               ntfs_attr_list_entry_is_valid(next_al_entry,
> +                                                             al_end) &&
>                                 le64_to_cpu(next_al_entry->lowest_vcn) <=
>                                         lowest_vcn &&
>                                 next_al_entry->type == al_entry->type &&
> --
> 2.43.0
>
>


-- 
Thanks,
Hyunchul