[PATCH] ntfs3: check split point bounds before copying the split entry

[PATCH] ntfs3: check split point bounds before copying the split entry

[Andrey Misiurov]

indx_insert_into_buffer() copies the split entry before checking that
its on-disk size fits in the index buffer. A crafted image makes the
copy read past the end of the buffer:

  BUG: KASAN: slab-out-of-bounds in indx_insert_into_buffer+0x49c/0x1310 [ntfs3]
  Read of size 4352 at addr ffff888172fcc760 by task touch/970
  CPU: 0 UID: 0 PID: 970 Comm: touch Not tainted 6.12.88+deb13-amd64 #1
  Call Trace:
   <TASK>
   dump_stack_lvl+0x110/0x1a0
   print_report+0x237/0x77f
   kasan_report+0xdc/0x140
   kasan_check_range+0x30f/0x350
   __asan_memcpy+0x33/0xb0
   indx_insert_into_buffer+0x49c/0x1310 [ntfs3]
   indx_insert_entry+0x14b/0x480 [ntfs3]
   ntfs_create_inode+0x2016/0x4b20 [ntfs3]
   </TASK>

  Allocated by task 970:
   __kmalloc_noprof+0x349/0x6c0
   indx_read+0x75c/0xc20 [ntfs3]

  The buggy address belongs to the cache kmalloc-4k of size 4096
  The buggy address is located 1888 bytes inside of that object

Do the check before the allocation and the copy.

Fixes: 82cae269cfa9 ("fs/ntfs3: Add initialization of super block")
Cc: stable@vger.kernel.org
Signed-off-by: Andrey Misiurov <andrey.misyrov@gmail.com>
---
 fs/ntfs3/index.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/fs/ntfs3/index.c b/fs/ntfs3/index.c
--- a/fs/ntfs3/index.c
+++ b/fs/ntfs3/index.c
@@ -1899,12 +1899,6 @@
 		return -EINVAL;
 
 	sp_size = le16_to_cpu(sp->size);
-	up_e = kmalloc(sp_size + sizeof(u64), GFP_NOFS);
-	if (!up_e)
-		return -ENOMEM;
-	memcpy(up_e, sp, sp_size);
-
-	/* Make a copy for undo. */
 	used1 = le32_to_cpu(hdr1->used);
 
 	/*
@@ -1913,13 +1907,19 @@
 	 * that (PtrOffset(hdr1, sp) + sp_size) exceeds used1. Without this
 	 * guard the u32 'used = used1 - to_copy - sp_size' underflows and
 	 * the subsequent memmove count becomes a near-4-GiB value,
-	 * triggering an out-of-bounds kernel write.
+	 * triggering an out-of-bounds kernel write. The memcpy below also
+	 * reads sp_size bytes starting at sp, so the check has to happen
+	 * before it.
 	 */
-	if (PtrOffset(hdr1, sp) + sp_size > used1) {
-		err = -EINVAL;
-		goto out;
-	}
+	if (PtrOffset(hdr1, sp) + sp_size > used1)
+		return -EINVAL;
 
+	up_e = kmalloc(sp_size + sizeof(u64), GFP_NOFS);
+	if (!up_e)
+		return -ENOMEM;
+	memcpy(up_e, sp, sp_size);
+
+	/* Make a copy for undo. */
 	hdr1_saved = kmemdup(hdr1, used1, GFP_NOFS);
 	if (!hdr1_saved) {
 		err = -ENOMEM;