[PATCH] ntfs: validate the full MST update sequence array

[PATCH] ntfs: validate the full MST update sequence array

[DaeMyung Kang]

The NTFS update sequence array contains the update sequence number
itself followed by one fixup entry for each protected sector.  The
current MST validation subtracts one from usa_count before checking
the array bounds, so it only accounts for the fixup entries and not
the leading update sequence number.

A malformed record can therefore pass validation with the last fixup
entry outside the record buffer.  post_read_mst_fixup() then reads past
the supplied buffer, while pre_write_mst_fixup() can write past it.
The USA also must end before the last word of the first 512-byte sector,
otherwise applying the fixups can overlap the sector trailer that the
array is supposed to protect.
This matches the documented NTFS MULTI_SECTOR_HEADER layout, where the
update sequence array is required to end before the last USHORT in the
first sector, and the sequence number stride is 512 bytes.

Validate the on-disk usa_count before converting it to a fixup count,
check the full USA size, and share this validation between the MST
helpers.  Pass the record size to post_write_mst_fixup() as well so
error paths do not restore data from a malformed update sequence array.

A small userspace ASAN reproducer with size=512, usa_ofs=510 and
usa_count=2 triggers a heap-buffer-overflow with the old validation.
With this change the same malformed record is rejected before the
fixup array is dereferenced.

Fixes: d3ad708fecaa ("ntfs: Initial commit")
Signed-off-by: DaeMyung Kang <charsyam@gmail.com>
---
 fs/ntfs/index.c |  3 ++-
 fs/ntfs/mst.c   | 68 ++++++++++++++++++++++++++++++++++++++---------------------------
 fs/ntfs/ntfs.h  |  2 +-
 3 files changed, 43 insertions(+), 30 deletions(-)

diff --git a/fs/ntfs/index.c b/fs/ntfs/index.c
index 413a1425d640..c2289550bc8f 100644
--- a/fs/ntfs/index.c
+++ b/fs/ntfs/index.c
@@ -178,7 +178,8 @@ int ntfs_icx_ib_sync_write(struct ntfs_index_context *icx)
 		icx->ib = NULL;
 		icx->ib_dirty = false;
 	} else {
-		post_write_mst_fixup((struct ntfs_record *)icx->ib);
+		post_write_mst_fixup((struct ntfs_record *)icx->ib,
+				icx->block_size);
 		icx->sync_write = false;
 	}
 
diff --git a/fs/ntfs/mst.c b/fs/ntfs/mst.c
index 7f9faad924ad..d1b9ed0703bd 100644
--- a/fs/ntfs/mst.c
+++ b/fs/ntfs/mst.c
@@ -9,6 +9,30 @@
 
 #include "ntfs.h"
 
+static bool mst_fixup_valid(const struct ntfs_record *b, const u32 size,
+		u16 *usa_ofs, u16 *fixup_count)
+{
+	u16 usa_count;
+	u32 usa_end;
+
+	if (!b)
+		return false;
+
+	*usa_ofs = le16_to_cpu(b->usa_ofs);
+	usa_count = le16_to_cpu(b->usa_count);
+
+	if (!size || (size & (NTFS_BLOCK_SIZE - 1)) || (*usa_ofs & 1) ||
+	    usa_count != (size >> NTFS_BLOCK_SIZE_BITS) + 1)
+		return false;
+
+	usa_end = *usa_ofs + usa_count * sizeof(__le16);
+	if (usa_end > size || usa_end > NTFS_BLOCK_SIZE - sizeof(__le16))
+		return false;
+
+	*fixup_count = usa_count - 1;
+	return true;
+}
+
 /*
  * post_read_mst_fixup - deprotect multi sector transfer protected data
  * @b:		pointer to the data to deprotect
@@ -28,18 +52,12 @@
  */
 int post_read_mst_fixup(struct ntfs_record *b, const u32 size)
 {
-	u16 usa_ofs, usa_count, usn;
+	u16 usa_ofs, usa_count, fixup_count, usn;
 	u16 *usa_pos, *data_pos;
 
-	/* Setup the variables. */
-	usa_ofs = le16_to_cpu(b->usa_ofs);
-	/* Decrement usa_count to get number of fixups. */
-	usa_count = le16_to_cpu(b->usa_count) - 1;
-	/* Size and alignment checks. */
-	if (size & (NTFS_BLOCK_SIZE - 1) || usa_ofs & 1	||
-	    usa_ofs + (usa_count * 2) > size ||
-	    (size >> NTFS_BLOCK_SIZE_BITS) != usa_count)
+	if (!mst_fixup_valid(b, size, &usa_ofs, &fixup_count))
 		return 0;
+	usa_count = fixup_count;
 	/* Position of usn in update sequence array. */
 	usa_pos = (u16 *)b + usa_ofs/sizeof(u16);
 	/*
@@ -75,8 +93,7 @@ int post_read_mst_fixup(struct ntfs_record *b, const u32 size)
 		}
 		data_pos += NTFS_BLOCK_SIZE / sizeof(u16);
 	}
-	/* Re-setup the variables. */
-	usa_count = le16_to_cpu(b->usa_count) - 1;
+	usa_count = fixup_count;
 	data_pos = (u16 *)b + NTFS_BLOCK_SIZE / sizeof(u16) - 1;
 	/* Fixup all sectors. */
 	while (usa_count--) {
@@ -115,21 +132,14 @@ int post_read_mst_fixup(struct ntfs_record *b, const u32 size)
 int pre_write_mst_fixup(struct ntfs_record *b, const u32 size)
 {
 	__le16 *usa_pos, *data_pos;
-	u16 usa_ofs, usa_count, usn;
+	u16 usa_ofs, fixup_count, usn;
 	__le16 le_usn;
 
 	/* Sanity check + only fixup if it makes sense. */
 	if (!b || ntfs_is_baad_record(b->magic) ||
 	    ntfs_is_hole_record(b->magic))
 		return -EINVAL;
-	/* Setup the variables. */
-	usa_ofs = le16_to_cpu(b->usa_ofs);
-	/* Decrement usa_count to get number of fixups. */
-	usa_count = le16_to_cpu(b->usa_count) - 1;
-	/* Size and alignment checks. */
-	if (size & (NTFS_BLOCK_SIZE - 1) || usa_ofs & 1	||
-	    usa_ofs + (usa_count * 2) > size ||
-	    (size >> NTFS_BLOCK_SIZE_BITS) != usa_count)
+	if (!mst_fixup_valid(b, size, &usa_ofs, &fixup_count))
 		return -EINVAL;
 	/* Position of usn in update sequence array. */
 	usa_pos = (__le16 *)((u8 *)b + usa_ofs);
@@ -145,7 +155,7 @@ int pre_write_mst_fixup(struct ntfs_record *b, const u32 size)
 	/* Position in data of first u16 that needs fixing up. */
 	data_pos = (__le16 *)b + NTFS_BLOCK_SIZE/sizeof(__le16) - 1;
 	/* Fixup all sectors. */
-	while (usa_count--) {
+	while (fixup_count--) {
 		/*
 		 * Increment the position in the usa and save the
 		 * original data from the data buffer into the usa.
@@ -162,17 +172,19 @@ int pre_write_mst_fixup(struct ntfs_record *b, const u32 size)
 /*
  * post_write_mst_fixup - fast deprotect multi sector transfer protected data
  * @b:		pointer to the data to deprotect
+ * @size:	size in bytes of @b
  *
- * Perform the necessary post write multi sector transfer fixup, not checking
- * for any errors, because we assume we have just used pre_write_mst_fixup(),
- * thus the data will be fine or we would never have gotten here.
+ * Perform the necessary post write multi sector transfer fixup. This is only
+ * expected after pre_write_mst_fixup() has applied fixups, but keep the bounds
+ * checks so error paths cannot restore from a malformed update sequence array.
  */
-void post_write_mst_fixup(struct ntfs_record *b)
+void post_write_mst_fixup(struct ntfs_record *b, const u32 size)
 {
 	__le16 *usa_pos, *data_pos;
+	u16 usa_ofs, fixup_count;
 
-	u16 usa_ofs = le16_to_cpu(b->usa_ofs);
-	u16 usa_count = le16_to_cpu(b->usa_count) - 1;
+	if (!mst_fixup_valid(b, size, &usa_ofs, &fixup_count))
+		return;
 
 	/* Position of usn in update sequence array. */
 	usa_pos = (__le16 *)b + usa_ofs/sizeof(__le16);
@@ -181,7 +193,7 @@ void post_write_mst_fixup(struct ntfs_record *b)
 	data_pos = (__le16 *)b + NTFS_BLOCK_SIZE/sizeof(__le16) - 1;
 
 	/* Fixup all sectors. */
-	while (usa_count--) {
+	while (fixup_count--) {
 		/*
 		 * Increment position in usa and restore original data from
 		 * the usa into the data buffer.
diff --git a/fs/ntfs/ntfs.h b/fs/ntfs/ntfs.h
index e301c68c780b..445101c06ef9 100644
--- a/fs/ntfs/ntfs.h
+++ b/fs/ntfs/ntfs.h
@@ -230,7 +230,7 @@ int ntfs_write_volume_label(struct ntfs_volume *vol, char *label);
 /* From fs/ntfs/mst.c */
 int post_read_mst_fixup(struct ntfs_record *b, const u32 size);
 int pre_write_mst_fixup(struct ntfs_record *b, const u32 size);
-void post_write_mst_fixup(struct ntfs_record *b);
+void post_write_mst_fixup(struct ntfs_record *b, const u32 size);
 
 /* From fs/ntfs/unistr.c */
 bool ntfs_are_names_equal(const __le16 *s1, size_t s1_len,