[PATCH v2 1/3] ntfs: validate resident attribute lists and harden the validator

[Bryam Vargas <hexlabsecurity@proton.me>]

A base inode's $ATTRIBUTE_LIST is sanity-checked by load_attribute_list()
only on the non-resident path; ntfs_read_locked_inode() copies a *resident*
attribute list into ni->attr_list with a plain memcpy() and no validation
at all. Every subsequent walk of ni->attr_list -- ntfs_external_attr_find(),
ntfs_inode_attach_all_extents(), ntfs_attrlist_need() -- then trusts the
entries are well-formed and reads attr_list_entry fixed-header fields
(lowest_vcn at offset 8, mft_reference at offset 16, and the name) with
bounds that assume validation already happened. A crafted resident
attribute list therefore reaches those walks unvalidated and can drive
out-of-bounds reads of the attribute-list buffer.

load_attribute_list() itself reads ale->name_offset (offset 7),
ale->mft_reference (offset 16) and the name length under only an
"al < al_start + size" bound, so its own validation loop can over-read the
fixed header of a truncated trailing entry by a few bytes.

Factor the per-entry validation into ntfs_attr_list_is_valid(), which
requires each entry's fixed header (offsetof(struct attr_list_entry, name))
to be in range before any field is dereferenced and that the entries tile
the buffer exactly. Use it both in load_attribute_list() (replacing the
open-coded loop, closing its own over-read) and on the resident path in
ntfs_read_locked_inode() (which previously skipped validation entirely).

Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
---
v2: dropped the redundant Reported-by (it duplicated Signed-off-by); reproducer
    omitted on the public list -- available to the maintainers on request.
    Posting publicly per security@kernel.org guidance that crafted-image
    filesystem bugs fall outside the kernel security process's threat model.

Root cause behind the two out-of-bounds reads in patches 2/3 and 3/3: those
harden the individual read sites; this validates the list centrally so the
other walks (ntfs_inode_attach_all_extents, ntfs_attrlist_need) are covered
too. Verified under KASAN: the crafted attribute list that previously oopsed
ntfs_external_attr_find() is now rejected at load with
"ntfs_read_locked_inode(): Corrupt attribute list." and no out-of-bounds
access; a benign mkntfs image still mounts. checkpatch clean.

 fs/ntfs/attrib.c |   56 ++++++++++++++++++++++++++++++++++++++++--------------
 fs/ntfs/attrib.h |    2 ++
 fs/ntfs/inode.c  |    6 ++++++
 3 files changed, 49 insertions(+), 15 deletions(-)

diff --git a/fs/ntfs/attrib.c b/fs/ntfs/attrib.c
index 421c6cdcbb53..36c1df25a622 100644
--- a/fs/ntfs/attrib.c
+++ b/fs/ntfs/attrib.c
@@ -843,11 +843,49 @@ char *ntfs_attr_name_get(const struct ntfs_volume *vol, const __le16 *uname,
 	return NULL;
 }
 
+/*
+ * ntfs_attr_list_is_valid - sanity check an in-memory $ATTRIBUTE_LIST
+ * @al_start:	start of the attribute list buffer
+ * @size:	length of the attribute list in bytes
+ *
+ * Verify that [@al_start, @al_start + @size) is a well-formed sequence of
+ * attr_list_entry records.  Each record's fixed header must lie within the
+ * buffer before any of its fields are dereferenced, its length must cover
+ * the fixed header plus the name, and the records must tile the buffer
+ * exactly.  Return true if valid, false otherwise.
+ */
+bool ntfs_attr_list_is_valid(const u8 *al_start, s64 size)
+{
+	const u8 *al = al_start;
+	const u8 *al_end = al_start + size;
+
+	while (al < al_end) {
+		const struct attr_list_entry *ale =
+				(const struct attr_list_entry *)al;
+		u16 ale_len;
+
+		/* The fixed header must be in bounds before it is parsed. */
+		if (al + offsetof(struct attr_list_entry, name) > al_end)
+			return false;
+		ale_len = le16_to_cpu(ale->length);
+		if (ale->name_offset != sizeof(struct attr_list_entry))
+			return false;
+		if ((u32)ale->name_offset +
+		    (u32)ale->name_length * sizeof(__le16) > ale_len ||
+		    al + ale_len > al_end)
+			return false;
+		if (ale->type == AT_UNUSED)
+			return false;
+		if (MSEQNO_LE(ale->mft_reference) == 0)
+			return false;
+		al += ale_len;
+	}
+	return al == al_end;
+}
+
 int load_attribute_list(struct ntfs_inode *base_ni, u8 *al_start, const s64 size)
 {
 	struct inode *attr_vi = NULL;
-	u8 *al;
-	struct attr_list_entry *ale;
 
 	if (!al_start || size <= 0)
 		return -EINVAL;
@@ -869,19 +907,7 @@ int load_attribute_list(struct ntfs_inode *base_ni, u8 *al_start, const s64 size
 	}
 	iput(attr_vi);
 
-	for (al = al_start; al < al_start + size; al += le16_to_cpu(ale->length)) {
-		ale = (struct attr_list_entry *)al;
-		if (ale->name_offset != sizeof(struct attr_list_entry))
-			break;
-		if (le16_to_cpu(ale->length) <= ale->name_offset + ale->name_length ||
-		    al + le16_to_cpu(ale->length) > al_start + size)
-			break;
-		if (ale->type == AT_UNUSED)
-			break;
-		if (MSEQNO_LE(ale->mft_reference) == 0)
-			break;
-	}
-	if (al != al_start + size) {
+	if (!ntfs_attr_list_is_valid(al_start, size)) {
 		ntfs_error(base_ni->vol->sb, "Corrupt attribute list, mft = %llu",
 			   base_ni->mft_no);
 		return -EIO;
diff --git a/fs/ntfs/attrib.h b/fs/ntfs/attrib.h
index f7acc7986b09..4b11d4b0be14 100644
--- a/fs/ntfs/attrib.h
+++ b/fs/ntfs/attrib.h
@@ -71,6 +71,8 @@ int ntfs_attr_lookup(const __le32 type, const __le16 *name,
 		const u32 name_len, const u32 ic,
 		const s64 lowest_vcn, const u8 *val, const u32 val_len,
 		struct ntfs_attr_search_ctx *ctx);
+bool ntfs_attr_list_is_valid(const u8 *al_start, s64 size);
+
 int load_attribute_list(struct ntfs_inode *base_ni,
 			       u8 *al_start, const s64 size);
 
diff --git a/fs/ntfs/inode.c b/fs/ntfs/inode.c
index 360bebd1ee3f..a5f7400fd19d 100644
--- a/fs/ntfs/inode.c
+++ b/fs/ntfs/inode.c
@@ -848,6 +848,12 @@ static int ntfs_read_locked_inode(struct inode *vi)
 					a->data.resident.value_offset),
 					le32_to_cpu(
 					a->data.resident.value_length));
+			/* A resident list is not validated on load; check it now. */
+			if (!ntfs_attr_list_is_valid(ni->attr_list,
+						     ni->attr_list_size)) {
+				ntfs_error(vi->i_sb, "Corrupt attribute list.");
+				goto unm_err_out;
+			}
 		}
 	}
 skip_attr_list_load:
-- 
2.43.0