[PATCH 0/2] fs/ntfs3: fix log replay payload overreads

[PATCH 0/2] fs/ntfs3: fix log replay payload overreads

[Giulia Aloia]

This series fixes two journal replay paths that can read past crafted
redo or undo payloads while mounting a dirty NTFS volume.

Patch 1 validates that redo and undo payload ranges stay within the log
record before replay uses them, while preserving the compensation-log
record undo exception used by Windows. It also validates the fixed-size
OpenNonresidentAttribute redo entry and optional undo name before the
analysis pass copies them.

Patch 2 validates the minimum payload length for SetNewAttributeSizes,
which always reads alloc_size, valid_size and data_size but previously
only checked whether the optional total_size field was present.

The two checks are complementary. Patch 1 makes sure the selected
payload range is inside the log record when that range is expected to be
present. Patch 2 makes sure that the selected range is large enough for
the SetNewAttributeSizes operation that interprets it.

Both issues are reachable while mounting crafted images. Each patch
contains the KASAN report observed before the fix. With these patches,
the same images are rejected during mount without sanitizer reports.

Giulia Aloia (2):
  fs/ntfs3: fix out-of-bounds read of redo/undo data during log replay
  fs/ntfs3: validate SetNewAttributeSizes payload length

 fs/ntfs3/fslog.c | 35 ++++++++++++++++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

-- 
2.55.0

[PATCH 1/2] fs/ntfs3: fix out-of-bounds read of redo/undo data during log replay

[Giulia Aloia]

check_log_rec() is the common validator for log records consumed during
journal replay. It checks the header size, the transaction table offset,
the 8-byte alignment of redo_off and undo_off, and that the record holds
at least lrh_length(lr) bytes. It does not check that the redo and undo
data ranges themselves lie inside the record.

log_replay() builds those ranges directly from on-disk fields:

	data = Add2Ptr(lrh, le16_to_cpu(lrh->redo_off));
	dlen = le16_to_cpu(lrh->redo_len);
	...
	err = do_action(log, oe, lrh, t16, data, dlen, rec_len, &rec_lsn);

do_action() bounds the destination in its copy cases, for example:

	case InitializeFileRecordSegment:
		if (roff + dlen > record_size)
			goto dirty_vol;

		memcpy(Add2Ptr(rec, roff), data, dlen);

but it does not bound the source range. The undo pass has the same issue
for normal undo records, and the analysis pass reads DeleteDirtyClusters
LCN ranges and OpenNonresidentAttribute entries and optional names from
unchecked redo/undo ranges.

redo_off, redo_len, undo_off and undo_len are 16-bit fields read from
the on-disk log. A crafted dirty log record can therefore make journal
replay read past the end of the log record while applying redo or undo
data. In the copy cases, the out-of-bounds bytes are copied into the
in-memory file record being replayed.

This is independent of target_attr and restart-table offset validation:
even with a valid open-attribute entry and valid destination bounds,
do_action() can still over-read the log record because redo_off/redo_len
and undo_off/undo_len are not bounded against client_data_len.

Mounting needs CAP_SYS_ADMIN, since ntfs3 is FS_REQUIRES_DEV and not
FS_USERNS_MOUNT, but the image is untrusted wherever removable media are
auto-mounted onto the in-tree driver; replay runs on any rw mount of a
volume whose log is dirty.

Before this fix, mounting the crafted image on an x86-64 KASAN build
produced:

  BUG: KASAN: slab-out-of-bounds in do_action+0x2820/0x89e0
  Read of size 1024 at addr ffff888102404f30 by task mount/67
  Call Trace:
   __asan_memcpy+0x23/0x60
   do_action+0x2820/0x89e0
   log_replay+0xcd38/0xe690
   ntfs_loadlog_and_replay+0x3e0/0x500
   ntfs_fill_super+0x1fd3/0x4510
   get_tree_bdev_flags+0x2ff/0x5d0
   vfs_get_tree+0x80/0x2d0
   fc_mount+0x15/0x1f0
   path_mount+0x77e/0x1d70
   __x64_sys_mount+0x207/0x270
   do_syscall_64+0xde/0x4b0
   entry_SYSCALL_64_after_hwframe+0x77/0x7f

  Allocated by task 67:
   __kmalloc_noprof+0x1cc/0x480
   read_log_page+0x354/0x5a0
   find_log_rec+0x383/0x5f0
   read_log_rec_lcb+0x1c7/0x550
   log_replay+0xc85a/0xe690
   ntfs_loadlog_and_replay+0x3e0/0x500
   ntfs_fill_super+0x1fd3/0x4510
   get_tree_bdev_flags+0x2ff/0x5d0
   vfs_get_tree+0x80/0x2d0
   fc_mount+0x15/0x1f0
   path_mount+0x77e/0x1d70
   __x64_sys_mount+0x207/0x270

  The buggy address is located 3888 bytes inside of
   allocated 4096-byte region [ffff888102404000, ffff888102405000)

Reject redo ranges that extend beyond client_data_len. Also reject undo
ranges that extend beyond client_data_len, except for compensation log
records, which can have undo_len set even when no undo bytes are present.

OpenNonresidentAttribute also reads a fixed-size open-attribute entry
from redo_off; validate that the entry is contained in the log record
before reading it. Because check_log_rec() intentionally leaves the CLR
undo range unchecked, also validate the undo range locally before copying
the attribute name from it.

Every caller already turns a false return into -EINVAL and aborts the
replay.

Fixes: b46acd6a6a62 ("fs/ntfs3: Add NTFS journal")
Cc: stable@vger.kernel.org
Assisted-by: Bynario AI
Signed-off-by: Giulia Aloia <giulia@bynar.io>
---
 fs/ntfs3/fslog.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/fs/ntfs3/fslog.c b/fs/ntfs3/fslog.c
index ed50c1d0c23e..88bd6ef98467 100644
--- a/fs/ntfs3/fslog.c
+++ b/fs/ntfs3/fslog.c
@@ -694,6 +694,7 @@ static bool check_log_rec(const struct LOG_REC_HDR *lr, u32 bytes, u32 tr,
 			  u32 bytes_per_attr_entry)
 {
 	u16 t16;
+	u32 off, len;
 
 	if (bytes < sizeof(struct LOG_REC_HDR))
 		return false;
@@ -731,6 +732,17 @@ static bool check_log_rec(const struct LOG_REC_HDR *lr, u32 bytes, u32 tr,
 	if (bytes < lrh_length(lr))
 		return false;
 
+	off = le16_to_cpu(lr->redo_off);
+	len = le16_to_cpu(lr->redo_len);
+	if (off > bytes || len > bytes - off)
+		return false;
+
+	off = le16_to_cpu(lr->undo_off);
+	len = le16_to_cpu(lr->undo_len);
+	if (lr->undo_op != cpu_to_le16(CompensationLogRecord) &&
+	    (off > bytes || len > bytes - off))
+		return false;
+
 	return true;
 }
 
@@ -4736,6 +4748,25 @@ int log_replay(struct ntfs_inode *ni, bool *initialized)
 	}
 
 	case OpenNonresidentAttribute:
+		t32 = !rst->major_ver ? SIZEOF_OPENATTRIBUTEENTRY0 :
+					 bytes_per_attr_entry;
+		off = le16_to_cpu(lrh->redo_off);
+		if (off > rec_len || t32 > rec_len - off) {
+			err = -EINVAL;
+			goto out;
+		}
+
+		/*
+		 * The attribute name is copied from the undo range, which
+		 * check_log_rec() does not bound for compensation log records.
+		 */
+		off = le16_to_cpu(lrh->undo_off);
+		t32 = le16_to_cpu(lrh->undo_len);
+		if (t32 && (off > rec_len || t32 > rec_len - off)) {
+			err = -EINVAL;
+			goto out;
+		}
+
 		t16 = le16_to_cpu(lrh->target_attr);
 		if (t16 >= bytes_per_rt(oatbl)) {
 			/*
-- 
2.55.0

[PATCH 2/2] fs/ntfs3: validate SetNewAttributeSizes payload length

[Giulia Aloia]

do_action() handles SetNewAttributeSizes by reading alloc_size,
valid_size and data_size from the redo or undo payload. It only treats
total_size as optional, but it does not first check that the payload is
large enough for the three fields it always consumes.

For LOG_RECORD_MULTI_PAGE records, find_log_rec() allocates exactly
client_data_len bytes. A crafted SetNewAttributeSizes undo record can keep
the undo payload within client_data_len while setting undo_len too small
for the fields read by do_action(), causing an out-of-bounds read.

Before this fix, mounting the crafted image on a KASAN build produced:

  BUG: KASAN: slab-out-of-bounds in do_action+0x7640/0x89e0
  Read of size 8 at addr ff11000101d94ed0 by task mount/61
  Call Trace:
   do_action+0x7640/0x89e0
   log_replay+0xcd38/0xe690
   ntfs_loadlog_and_replay+0x3e0/0x500
   ntfs_fill_super+0x1fd3/0x4510

The same report showed the object came from find_log_rec():

  Allocated by task 61:
   find_log_rec+0x17d/0x5f0
  The buggy address is located 0 bytes to the right of
   allocated 80-byte region [ff11000101d94e80, ff11000101d94ed0)

Reject SetNewAttributeSizes payloads that do not contain the three
fields it always reads. Keep the existing total_size check unchanged,
because that field is optional in the current replay code.

Fixes: b46acd6a6a62 ("fs/ntfs3: Add NTFS journal")
Cc: stable@vger.kernel.org
Assisted-by: Bynario AI
Signed-off-by: Giulia Aloia <giulia@bynar.io>
---
 fs/ntfs3/fslog.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/ntfs3/fslog.c b/fs/ntfs3/fslog.c
index 88bd6ef98467..c5de4ebcc7ec 100644
--- a/fs/ntfs3/fslog.c
+++ b/fs/ntfs3/fslog.c
@@ -3463,7 +3463,9 @@ static int do_action(struct ntfs_log *log, struct OPEN_ATTR_ENRTY *oe,
 
 	case SetNewAttributeSizes:
 		new_sz = data;
-		if (!check_if_attr(rec, lrh) || !attr->non_res)
+		if (!check_if_attr(rec, lrh) || !attr->non_res ||
+		    dlen < offsetof(struct NEW_ATTRIBUTE_SIZES, data_size) +
+				   sizeof(new_sz->data_size))
 			goto dirty_vol;
 
 		attr->nres.alloc_size = new_sz->alloc_size;
-- 
2.55.0