[PATCH 0/4] fs/ntfs3: tighten restart-table offset validation

[PATCH 0/4] fs/ntfs3: tighten restart-table offset validation

[Giulia Aloia]

This series makes NTFS3 journal replay safer when the on-disk restart
tables contain inconsistent offsets.

Patch 1 checks open attribute offsets in the dirty-page walk. Patch 2
checks transaction and attribute offsets in check_log_rec(), including
nonzero attribute offsets in records without LCNs, and validates
allocator offsets against the on-disk open-attribute table entry size.
Patch 3 rejects restart-table dumps without a complete header, rejects
short open-attribute entries, and requires the transaction-table entry
size to match TRANSACTION_ENTRY.
Patch 4 bounds the free-list walk used when allocating a specific
restart table entry.

Cen Zhang's earlier patch [1] checks target_attr at the redo and undo
lookups. This series adds checks in the dirty-page walk, in log record
validation, and in the restart table allocator. It can be applied
independently of [1], but [1] is still needed for the additional
validation at the redo and undo lookups.

Greg KH asked on that thread why this should be fixed in the kernel
rather than in userspace fsck. Checking and repairing a filesystem
before mounting it remains the administrator's responsibility. These
patches serve a narrower purpose: if journal replay encounters an
invalid offset, it should return an error instead of accessing invalid
memory. They do not repair the filesystem or replace fsck. I am
proposing them as robustness improvements, consistent with the kernel
threat model [2].

Each patch includes a KASAN trace captured using a crafted image before
the fix. With the series applied, those images were re-mounted on the
same x86-64 KASAN build with no reports. The series builds clean with
W=1 after each patch and passes checkpatch.pl --strict with
GIT_COMMIT_ID ignored for the KASAN addresses.

[1] https://lore.kernel.org/all/20260901174934.6275-1-cenzhang@linux.microsoft.com/
[2] https://docs.kernel.org/process/threat-model.html

Giulia Aloia (4):
  fs/ntfs3: validate dirty page open attribute offsets
  fs/ntfs3: validate restart table offsets in log records
  fs/ntfs3: validate on-disk restart tables before use
  fs/ntfs3: fix out-of-bounds access in alloc_rsttbl_from_idx()

 fs/ntfs3/fslog.c | 110 +++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 70 insertions(+), 40 deletions(-)


base-commit: 238650ef6c7c7cca08e032527329424c9fbd70e5
-- 
2.55.0

[PATCH 1/4] fs/ntfs3: validate dirty page open attribute offsets

[Giulia Aloia]

The dirty-page walk in log_replay() uses dp->target_attr as a byte
offset into the open attribute table without validating it first.
Checking oe->next already dereferences the unchecked pointer.

A malformed dirty-page entry can select the table header, the middle
of an entry, or data beyond the table. The lookup can then read an
invalid entry and follow a bogus attribute pointer.

A crafted table dump containing a fake open attribute entry at
bytes_per_rt(oatbl), within the larger dump buffer, produced the
following on x86-64 before the fix:

 KASAN: maybe wild-memory-access in range
        [0x4141414141414148-0x414141414141414f]
 RIP: 0010:log_replay+0xa698/0xe690
 Call Trace:
  ntfs_loadlog_and_replay+0x3e0/0x500
  ntfs_fill_super+0x1fd3/0x4510
  ...

Require the offset to be at or beyond the end of the table header,
below the logical table size, and aligned to the table's entry size.
Also require that each entry can hold an OPEN_ATTR_ENRTY; together
these checks keep the whole selected entry within the table. Reject
invalid offsets with -EINVAL, as the redo lookup does. Keep the existing
handling of unallocated entries and NULL attribute pointers.

This lookup uses dirty-page table entries and is separate from the
redo and undo log-record lookups discussed in the linked report.

Fixes: b46acd6a6a62 ("fs/ntfs3: Add NTFS journal")
Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/all/20260901174934.6275-1-cenzhang@linux.microsoft.com/
Assisted-by: Bynario AI
Signed-off-by: Giulia Aloia <giulia@bynar.io>
---
 fs/ntfs3/fslog.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/fs/ntfs3/fslog.c b/fs/ntfs3/fslog.c
index ed50c1d0c23e..8ac0dbd2f07f 100644
--- a/fs/ntfs3/fslog.c
+++ b/fs/ntfs3/fslog.c
@@ -4987,6 +4987,15 @@ int log_replay(struct ntfs_inode *ni, bool *initialized)
 	if (!dp)
 		goto do_redo_1;
 
+	t32 = le32_to_cpu(dp->target_attr);
+	t16 = le16_to_cpu(oatbl->size);
+	if (t16 < sizeof(*oe) || t32 < sizeof(*oatbl) ||
+	    t32 >= bytes_per_rt(oatbl) ||
+	    (t32 - sizeof(*oatbl)) % t16) {
+		err = -EINVAL;
+		goto out;
+	}
+
 	oe = Add2Ptr(oatbl, le32_to_cpu(dp->target_attr));
 
 	if (oe->next != RESTART_ENTRY_ALLOCATED_LE)
 		goto next_dirty_page;
-- 
2.55.0

[PATCH 2/4] fs/ntfs3: validate restart table offsets in log records

[Giulia Aloia]

check_log_rec() validates transact_id and target_attr by subtracting the
24-byte restart-table header size, sizeof(struct RESTART_TABLE), and then
checking entry alignment. This is unsafe for offsets that point inside
the header. For example, offset 8 is below the header size, so the
unsigned subtraction wraps and the wrapped value can still pass the
alignment check.

The driver uses transact_id as an offset into the transaction table
when it looks up or allocates entries during journal analysis. This
happens even on read-only mounts, before replay stops for read-only
mode, so the offset must be checked at this stage too. If a forged
transact_id points into the restart-table header, analysis first reads
header bytes as tr->next. If those bytes do not look allocated, it can
then ask alloc_rsttbl_from_idx() to allocate an offset inside the
header. With crafted table metadata, that can make replay overwrite
restart-table header bytes and later treat those bytes as a
TRANSACTION_ENTRY.

The attribute-offset check is also skipped when lcns_follow is zero.
However, lcns_follow only describes page_lcns[] payload. It does not
mean target_attr is unused. OpenNonresidentAttribute can have no LCN
payload but still uses target_attr to choose or create an open-attribute
entry. Header and misaligned offsets can therefore reach the
open-attribute allocator unchecked.

For offset 8, the subtraction wraps on both 32-bit and 64-bit systems.
The wrapped value is divisible by 40, sizeof(struct TRANSACTION_ENTRY),
on both, so the transaction-ID check can accept it. The same wrapped
value is also divisible by the 40-byte v1 open-attribute entry size and,
on 64-bit systems, by 44, SIZEOF_OPENATTRIBUTEENTRY0, so the
attribute-offset check can accept it too.

For target_attr, replay can then interpret the restart table header as an
open-attribute entry. With crafted on-disk values, the interpreted entry
can contain a NULL open_attr pointer, which log_replay() later
dereferences.

This is reachable by mounting the crafted image on an x86-64 KASAN
kernel before this fix:

 KASAN: null-ptr-deref in range
        [0x0000000000000008-0x000000000000000f]
 RIP: 0010:log_replay+0xca58/0xe690
 Call Trace:
  ntfs_loadlog_and_replay+0x3e0/0x500
  ntfs_fill_super+0x1fd3/0x4510
  ...

Reject offsets that point inside the restart-table header before
subtracting the header size, so the subtraction cannot wrap. Validate
nonzero target_attr values even when lcns_follow is zero. Preserve zero
target_attr for records that require neither an attribute nor LCN work.
Do not impose a table upper bound in check_log_rec(): valid records can
require the analysis pass to grow the table.

Before OpenNonresidentAttribute grows the open attribute table or selects
an entry, validate target_attr against the actual oatbl->size as well.
Alignment to the version-specific entry size used by check_log_rec() does
not guarantee alignment to the slots used by the current table when the
on-disk table size differs. Allow aligned offsets beyond the current
table so valid records can still grow it.

Cen Zhang described the target_attr underflow in the linked patch and
proposed checks at the redo and undo lookups. Validate the offsets in
check_log_rec() itself, including transact_id and records without LCNs.

Fixes: b46acd6a6a62 ("fs/ntfs3: Add NTFS journal")
Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/all/20260901174934.6275-1-cenzhang@linux.microsoft.com/
Assisted-by: Bynario AI
Signed-off-by: Giulia Aloia <giulia@bynar.io>
---
 fs/ntfs3/fslog.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/fs/ntfs3/fslog.c b/fs/ntfs3/fslog.c
index 8ac0dbd2f07f..8dd233ec7d2f 100644
--- a/fs/ntfs3/fslog.c
+++ b/fs/ntfs3/fslog.c
@@ -697,7 +697,7 @@ static bool check_log_rec(const struct LOG_REC_HDR *lr, u32 bytes, u32 tr,
 
 	if (bytes < sizeof(struct LOG_REC_HDR))
 		return false;
-	if (!tr)
+	if (tr < sizeof(struct RESTART_TABLE))
 		return false;
 
 	if ((tr - sizeof(struct RESTART_TABLE)) %
@@ -711,7 +711,7 @@ static bool check_log_rec(const struct LOG_REC_HDR *lr, u32 bytes, u32 tr,
 		return false;
 
 	if (lr->target_attr)
-		goto check_lcns;
+		goto check_target;
 
 	if (is_target_required(le16_to_cpu(lr->redo_op)))
 		return false;
@@ -719,12 +719,13 @@ static bool check_log_rec(const struct LOG_REC_HDR *lr, u32 bytes, u32 tr,
 	if (is_target_required(le16_to_cpu(lr->undo_op)))
 		return false;
 
-check_lcns:
-	if (!lr->lcns_follow)
+check_target:
+	if (!lr->lcns_follow && !lr->target_attr)
 		goto check_length;
 
 	t16 = le16_to_cpu(lr->target_attr);
-	if ((t16 - sizeof(struct RESTART_TABLE)) % bytes_per_attr_entry)
+	if (t16 < sizeof(struct RESTART_TABLE) ||
+	    (t16 - sizeof(struct RESTART_TABLE)) % bytes_per_attr_entry)
 		return false;
 
 check_length:
@@ -4737,6 +4738,12 @@ int log_replay(struct ntfs_inode *ni, bool *initialized)
 
 	case OpenNonresidentAttribute:
 		t16 = le16_to_cpu(lrh->target_attr);
+		if (t16 < sizeof(*oatbl) ||
+		    (t16 - sizeof(*oatbl)) % le16_to_cpu(oatbl->size)) {
+			err = -EINVAL;
+			goto out;
+		}
+
 		if (t16 >= bytes_per_rt(oatbl)) {
 			/*
 			 * Compute how big the table needs to be.
-- 
2.55.0

[PATCH 3/4] fs/ntfs3: validate on-disk restart tables before use

[Giulia Aloia]

log_replay() locates restart-table dumps using redo_off and passes the
remaining record length to check_rstbl(). However, check_log_rec() does
not ensure that redo_off leaves room for a complete RESTART_TABLE
header. check_rstbl() reads the header fields before validating the
table size, so a truncated dump or an offset beyond the record can
cause an out-of-bounds read.

Before passing an on-disk restart table to check_rstbl(), require
redo_off to leave enough bytes in the log record for a complete
RESTART_TABLE header. Apply this to the transaction, dirty-page and
open-attribute table dumps.

For the open-attribute table, also require the table entry size to be
at least as large as the expected entry size for the restart-area
version. This ensures that each slot is large enough for the entry
format used when converting and initializing the table.

check_rstbl() also accepts transaction tables whose entry size differs
from sizeof(struct TRANSACTION_ENTRY). check_log_rec() aligns transact_id
to that structure size, not the on-disk entry size. Accessing smaller
entries can read or write past their end. Larger entries can make an
accepted offset point into the middle of a slot and leave too little
space for the transaction fields. Require the entry size to equal
sizeof(struct TRANSACTION_ENTRY) when loading the table. This also
protects accesses to existing transaction entries, which bypass
alloc_rsttbl_from_idx().

This is reachable by mounting the crafted image on an x86-64 KASAN
kernel before this fix:

KASAN: slab-out-of-bounds in log_replay+0x8094/0xe690
    Write of size 8 at addr ffff888101107680 by task mount/67
Call Trace:
 log_replay+0x8094/0xe690
 ntfs_loadlog_and_replay+0x3e0/0x500
 ntfs_fill_super+0x1fd3/0x4510
 ...

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 | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/fs/ntfs3/fslog.c b/fs/ntfs3/fslog.c
index 8dd233ec7d2f..e3b5a19f0e30 100644
--- a/fs/ntfs3/fslog.c
+++ b/fs/ntfs3/fslog.c
@@ -4274,12 +4274,17 @@ int log_replay(struct ntfs_inode *ni, bool *initialized)
 	}
 
 	t16 = le16_to_cpu(lrh->redo_off);
+	if (t16 > rec_len || rec_len - t16 < sizeof(*rt)) {
+		err = -EINVAL;
+		goto out;
+	}
 
 	rt = Add2Ptr(lrh, t16);
 	t32 = rec_len - t16;
 
 	/* Now check that this is a valid restart table. */
-	if (!check_rstbl(rt, t32)) {
+	if (le16_to_cpu(rt->size) != sizeof(struct TRANSACTION_ENTRY) ||
+	    !check_rstbl(rt, t32)) {
 		err = -EINVAL;
 		goto out;
 	}
@@ -4314,6 +4319,10 @@ int log_replay(struct ntfs_inode *ni, bool *initialized)
 	}
 
 	t16 = le16_to_cpu(lrh->redo_off);
+	if (t16 > rec_len || rec_len - t16 < sizeof(*rt)) {
+		err = -EINVAL;
+		goto out;
+	}
 
 	rt = Add2Ptr(lrh, t16);
 	t32 = rec_len - t16;
@@ -4441,11 +4450,16 @@ int log_replay(struct ntfs_inode *ni, bool *initialized)
 	}
 
 	t16 = le16_to_cpu(lrh->redo_off);
+	if (t16 > rec_len || rec_len - t16 < sizeof(*rt)) {
+		err = -EINVAL;
+		goto out;
+	}
 
 	rt = Add2Ptr(lrh, t16);
 	oatbl_bytes = rec_len - t16;
 
-	if (!check_rstbl(rt, oatbl_bytes)) {
+	if (le16_to_cpu(rt->size) < bytes_per_attr_entry ||
+	    !check_rstbl(rt, oatbl_bytes)) {
 		err = -EINVAL;
 		goto out;
 	}
-- 
2.55.0

[PATCH 4/4] fs/ntfs3: fix out-of-bounds access in alloc_rsttbl_from_idx()

[Giulia Aloia]

alloc_rsttbl_from_idx() walks the restart table free list until it finds
the requested offset. If the requested entry is not already allocated,
the old code expects to find it in the free list and keeps walking until
it does.

A crafted on-disk restart table can use individually valid free-list
offsets but still omit the requested entry from the list.
When log replay asks alloc_rsttbl_from_idx() to allocate that entry,
the old code keeps following the list without bound checks.

This is reachable by mounting the crafted image on an x86-64 KASAN
kernel before this fix:

KASAN: use-after-free in log_replay+0x8986/0xe690
    Read of size 4 at addr ffff888102477828 by task mount/67
Call Trace:
 log_replay+0x8986/0xe690
 ntfs_loadlog_and_replay+0x3e0/0x500
 ntfs_fill_super+0x1fd3/0x4510
 ...

Validate the requested offset against the table entry size before using
it. Then bound the free-list search by rt->used and reject invalid,
allocated, out-of-range, or misaligned links while walking. If the
requested entry is not found in the bounded walk, return failure instead
of continuing indefinitely.

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 | 66 ++++++++++++++++++++++++------------------------
 1 file changed, 33 insertions(+), 33 deletions(-)

diff --git a/fs/ntfs3/fslog.c b/fs/ntfs3/fslog.c
index e3b5a19f0e30..1793dd9ccfba 100644
--- a/fs/ntfs3/fslog.c
+++ b/fs/ntfs3/fslog.c
@@ -947,12 +947,20 @@ static inline void *alloc_rsttbl_idx(struct RESTART_TABLE **tbl)
  */
 static inline void *alloc_rsttbl_from_idx(struct RESTART_TABLE **tbl, u32 vbo)
 {
+	u32 i;
 	u32 off;
+	u32 prev_off = 0;
 	__le32 *e;
+	__le32 *prev_e = NULL;
 	struct RESTART_TABLE *rt = *tbl;
 	u32 bytes = bytes_per_rt(rt);
+	u16 used;
 	u16 esize = le16_to_cpu(rt->size);
 
+	if (esize < sizeof(__le32) || vbo < sizeof(struct RESTART_TABLE) ||
+	    (vbo - sizeof(struct RESTART_TABLE)) % esize)
+		return NULL;
+
 	/* If the entry is not the table, we will have to extend the table. */
 	if (vbo >= bytes) {
 		/*
@@ -968,57 +976,49 @@ static inline void *alloc_rsttbl_from_idx(struct RESTART_TABLE **tbl, u32 vbo)
 		*tbl = rt = extend_rsttbl(rt, bytes2idx / esize + 1, bytes);
 		if (!rt)
 			return NULL;
+		bytes = bytes_per_rt(rt);
 	}
 
+	used = le16_to_cpu(rt->used);
+
 	/* See if the entry is already allocated, and just return if it is. */
 	e = Add2Ptr(rt, vbo);
 
 	if (*e == RESTART_ENTRY_ALLOCATED_LE)
 		return e;
 
-	/*
-	 * Walk through the table, looking for the entry we're
-	 * interested and the previous entry.
-	 */
 	off = le32_to_cpu(rt->first_free);
-	e = Add2Ptr(rt, off);
-
-	if (off == vbo) {
-		/* this is a match */
-		rt->first_free = *e;
-		goto skip_looking;
-	}
-
-	/*
-	 * Need to walk through the list looking for the predecessor
-	 * of our entry.
-	 */
-	for (;;) {
-		/* Remember the entry just found */
-		u32 last_off = off;
-		__le32 *last_e = e;
 
-		/* Should never run of entries. */
+	for (i = 0; off; i++) {
+		if (i >= used || off == RESTART_ENTRY_ALLOCATED ||
+		    off < sizeof(struct RESTART_TABLE) ||
+		    off > bytes - sizeof(__le32) ||
+		    (off - sizeof(struct RESTART_TABLE)) % esize) {
+			return NULL;
+		}
 
-		/* Lookup up the next entry the list. */
-		off = le32_to_cpu(*last_e);
 		e = Add2Ptr(rt, off);
 
-		/* If this is our match we are done. */
 		if (off == vbo) {
-			*last_e = *e;
+			if (prev_e) {
+				*prev_e = *e;
 
-			/*
-			 * If this was the last entry, we update that
-			 * table as well.
-			 */
-			if (le32_to_cpu(rt->last_free) == off)
-				rt->last_free = cpu_to_le32(last_off);
-			break;
+				if (le32_to_cpu(rt->last_free) == off)
+					rt->last_free = cpu_to_le32(prev_off);
+			} else {
+				rt->first_free = *e;
+			}
+			goto found;
 		}
+
+		prev_e = e;
+		prev_off = off;
+		off = le32_to_cpu(*e);
 	}
 
-skip_looking:
+	return NULL;
+
+found:
 	/* If the list is now empty, we fix the last_free as well. */
 	if (!rt->first_free)
 		rt->last_free = 0;
-- 
2.55.0