ntfs_mft_record_check() verifies that attrs_offset is aligned and that the resulting pointer stays within the allocated MFT record buffer, but it does not check that the first attribute header starts within the bytes_in_use area. A malformed record with attrs_offset greater than bytes_in_use can pass this check as long as attrs_offset is still within bytes_allocated. The attribute parser then computes the remaining record space by subtracting the attribute pointer from bytes_in_use. Because that value is unsigned, the subtraction can underflow and allow bytes after bytes_in_use to be interpreted as an attribute. Reject records where attrs_offset is outside bytes_in_use or where the used area does not even contain the four-byte attribute type/AT_END terminator at attrs_offset. A small userspace model with attrs_offset=128 and bytes_in_use=64 shows the current check accepts the record and the parser space calculation underflows to 0xffffffc0. With this change the same malformed record is rejected before the attribute walker is entered. Fixes: d3ad708fecaa ("ntfs: Initial commit") Signed-off-by: DaeMyung Kang --- fs/ntfs/mft.c | 14 ++++++++++++-- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/fs/ntfs/mft.c b/fs/ntfs/mft.c index c04462fe049e..70c1aa76181b 100644 --- a/fs/ntfs/mft.c +++ b/fs/ntfs/mft.c @@ -30,6 +30,8 @@ int ntfs_mft_record_check(const struct ntfs_volume *vol, struct mft_record *m, { struct attr_record *a; struct super_block *sb = vol->sb; + u16 attrs_offset; + u32 bytes_in_use; if (!ntfs_is_file_record(m->magic)) { ntfs_error(sb, "Record %llu has no FILE magic (0x%x)\n", @@ -65,7 +67,17 @@ int ntfs_mft_record_check(const struct ntfs_volume *vol, struct mft_record *m, goto err_out; } - a = (struct attr_record *)((char *)m + le16_to_cpu(m->attrs_offset)); + attrs_offset = le16_to_cpu(m->attrs_offset); + bytes_in_use = le32_to_cpu(m->bytes_in_use); + + if (attrs_offset > bytes_in_use || + bytes_in_use - attrs_offset < sizeof_field(struct attr_record, type)) { + ntfs_error(sb, "Record %llu has corrupt attribute offset\n", + mft_no); + goto err_out; + } + + a = (struct attr_record *)((char *)m + attrs_offset); if ((char *)a < (char *)m || (char *)a > (char *)m + vol->mft_record_size) { ntfs_error(sb, "Record %llu is corrupt\n", mft_no); goto err_out; -- 2.34.1 NTFS MFT record numbers are limited to the 32-bit range, and ntfs_mft_record_layout() rejects mft_no >= 2^32. The free-MFT-record bitmap scan in ntfs_mft_bitmap_find_and_alloc_free_rec_nolock() also guards against this overflow but uses a strict greater than comparison, allowing record number 2^32 itself through this earlier check. Every other 2^32 boundary check in fs/ntfs/mft.c uses '>=', so the strict greater than here is both a real off-by-one and an internal inconsistency. A model with ll == 2^32 confirms the current check accepts the value while the corrected check rejects it. Use '>=' so the boundary matches the layout-time rejection and the surrounding bitmap-scan checks. Fixes: d3ad708fecaa ("ntfs: Initial commit") Signed-off-by: DaeMyung Kang --- fs/ntfs/mft.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/ntfs/mft.c b/fs/ntfs/mft.c index 70c1aa76181b..f8f2e481c5dc 100644 --- a/fs/ntfs/mft.c +++ b/fs/ntfs/mft.c @@ -1279,7 +1279,7 @@ static s64 ntfs_mft_bitmap_find_and_alloc_free_rec_nolock(struct ntfs_volume *vo b = ffz((unsigned long)*byte); if (b < 8 && b >= (bit & 7)) { ll = data_pos + (bit & ~7ull) + b; - if (unlikely(ll > (1ll << 32))) { + if (unlikely(ll >= (1ll << 32))) { #if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 6, 0) folio_unlock(folio); kunmap_local(buf); -- 2.34.1 ntfs_attr_find() validates a named attribute before comparing it with the requested name, but that check is currently after the AT_UNUSED handling. When callers enumerate attributes with AT_UNUSED, ntfs_attr_find() can return a malformed named attribute before checking whether name_offset and name_length stay within the attribute record. Some enumeration callers use the returned attribute name pointer directly. For example, one path passes (attr + name_offset, name_length) to ntfs_attr_iget(), where the name can later be copied according to name_length. A malformed on-disk name_offset/name_length pair should not be exposed to those callers. Move the existing name bounds validation before returning attributes during AT_UNUSED enumeration, and write it as an offset/remaining-size check so the subtraction cannot underflow. Extract the converted values into local variables (name_offset, attr_len, name_size) to make the intent explicit and avoid repeating the endian conversions inside the bounds check. This keeps matching attributes on the same checked path while also covering attribute enumeration. A small userspace ASAN model with attr length=32, name_offset=124 and name_length=8 reproduces a heap-buffer-overflow read in the old enumeration path. With this change the same malformed attribute is rejected before the name pointer is returned to the caller. Fixes: d3ad708fecaa ("ntfs: Initial commit") Signed-off-by: DaeMyung Kang --- fs/ntfs/attrib.c | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/fs/ntfs/attrib.c b/fs/ntfs/attrib.c index 330127975b26..f51917b4a494 100644 --- a/fs/ntfs/attrib.c +++ b/fs/ntfs/attrib.c @@ -675,6 +675,9 @@ static int ntfs_attr_find(const __le32 type, const __le16 *name, __le16 *upcase = vol->upcase; u32 upcase_len = vol->upcase_len; unsigned int space; + u16 name_offset; + u32 attr_len; + u32 name_size; /* * Iterate over attributes in mft record starting at @ctx->attr, or the @@ -702,6 +705,20 @@ static int ntfs_attr_find(const __le32 type, const __le16 *name, return -ENOENT; if (unlikely(!a->length)) break; + if (a->name_length) { + name_offset = le16_to_cpu(a->name_offset); + attr_len = le32_to_cpu(a->length); + name_size = a->name_length * sizeof(__le16); + + if (name_offset > attr_len || + attr_len - name_offset < name_size) { + ntfs_error(vol->sb, + "Corrupt attribute name in MFT record %llu\n", + ctx->ntfs_ino->mft_no); + break; + } + } + if (type == AT_UNUSED) return 0; if (a->type != type) @@ -715,14 +732,6 @@ static int ntfs_attr_find(const __le32 type, const __le16 *name, if (a->name_length) return -ENOENT; } else { - if (a->name_length && ((le16_to_cpu(a->name_offset) + - a->name_length * sizeof(__le16)) > - le32_to_cpu(a->length))) { - ntfs_error(vol->sb, "Corrupt attribute name in MFT record %llu\n", - ctx->ntfs_ino->mft_no); - break; - } - if (!ntfs_are_names_equal(name, name_len, (__le16 *)((u8 *)a + le16_to_cpu(a->name_offset)), a->name_length, ic, upcase, upcase_len)) { -- 2.34.1