[PATCH] fs/ntfs3: bound UpdateFileNameRoot/Allocation by the entry size

[Shirong Zhao]

do_action() resolves lrh->attr_off to an index entry only through
check_if_root_index()/check_if_alloc_index(), which verify that the
offset is an entry boundary.  That boundary may belong to the 16-byte
NTFS_DE end entry, which carries no file name.

Both UpdateFileNameRoot and UpdateFileNameAllocation then assume a full
ATTR_FILE_NAME follows the entry and memmove() sizeof(fname->dup)
(0x38) bytes starting at e + 0x18.  With the end entry placed at the
tail of an ATTR_ROOT that ends at the record boundary, the write runs
past kmalloc(sbi->record_size); the non-resident path has the same
layout in the index buffer.

Reproduced on v7.3-rc6 under KASAN, mounting a crafted image whose
target MFT record (record_size 0x400) holds ATTR_ROOT.size 0x3c0,
data_off 0x18, data_size 0x3a8, ihdr.de_off 0x388, a single 16-byte end
entry, and log action UpdateFileNameRoot with attr_off 0x3b0:

    BUG: KASAN: slab-out-of-bounds in do_action+0x3407/0x8790
    Write of size 56 at addr ffff88800375e400 by task mount/205
    __asan_memmove+0x3c/0x60
    do_action+0x3407/0x8790
    ntfs_fill_super+0x33e4/0x3f40
    ...
    Allocated by task 205:
    __kmalloc_noprof
    mi_init+0x81/0x110
    The buggy address is located 0 bytes to the right of
    allocated 1024-byte region [ffff88800375e000, ffff88800375e400)

Refuse the update unless the entry is large enough to contain the
region being written.

Fixes: b46acd6a6a62 ("fs/ntfs3: Add NTFS journal")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Shirong Zhao <shxzhaosr@xxxxxxx>
---
 fs/ntfs3/fslog.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/fs/ntfs3/fslog.c b/fs/ntfs3/fslog.c
index bb8d40b..abc066a 100644
--- a/fs/ntfs3/fslog.c
+++ b/fs/ntfs3/fslog.c
@@ -3568,7 +3568,19 @@ static int do_action(struct ntfs_log *log, struct OPEN_ATTR_ENRTY *oe,
 
 		e = Add2Ptr(attr, le16_to_cpu(lrh->attr_off));
 		fname = (struct ATTR_FILE_NAME *)(e + 1);
-		memmove(&fname->dup, data, sizeof(fname->dup)); //
+
+		/*
+		 * attr_off is only known to be an entry boundary; it may
+		 * point at the 16-byte end entry, which carries no file
+		 * name.  Bound the update by the entry size, as the
+		 * neighbouring UpdateRecordDataRoot case does for its
+		 * payload.
+		 */
+		if (PtrOffset(e, &fname->dup) + sizeof(fname->dup) >
+		    le16_to_cpu(e->size))
+			goto dirty_vol;
+
+		memmove(&fname->dup, data, sizeof(fname->dup));
 		mi->dirty = true;
 		break;
 
@@ -3753,6 +3765,12 @@ static int do_action(struct ntfs_log *log, struct OPEN_ATTR_ENRTY *oe,
 		}
 
 		fname = (struct ATTR_FILE_NAME *)(e + 1);
+
+		/* As in UpdateFileNameRoot: e may be the 16-byte end entry. */
+		if (PtrOffset(e, &fname->dup) + sizeof(fname->dup) >
+		    le16_to_cpu(e->size))
+			goto dirty_vol;
+
 		memmove(&fname->dup, data, sizeof(fname->dup));
 
 		a_dirty = true;
-- 
2.49.0.windows.1