The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH] fs/ntfs3: validate dirty page table entry sizes
@ 2026-06-11 21:33 Kyle Zeng
  2026-07-23 14:25 ` Konstantin Komarov
  2026-07-24 11:30 ` Konstantin Komarov
  0 siblings, 2 replies; 6+ messages in thread
From: Kyle Zeng @ 2026-06-11 21:33 UTC (permalink / raw)
  To: ntfs3; +Cc: linux-kernel, Konstantin Komarov, outbounddisclosures, Kyle Zeng

The generic restart table check only verifies the table header and
free-list shape.  Dirty Page Table entries also carry an entry-local
LCN array whose length is controlled by lcns_follow, and replay later
trusts that value when converting version-0 entries and when copying
LCNs from log records.

A malformed $LogFile can provide a Dirty Page Table dump whose generic
restart table is valid but whose entry is too small for the declared
lcns_follow count.  log_replay() can then run the version-0 conversion
memmove() past the kmemdup() allocation, or later copy/read past
dp->page_lcns when a log record spans beyond the matched DPT entry.

Add Dirty Page Table-specific validation before copying the table:
require entries to be large enough for the typed DPT layout, require
the old version-0 source layout to fit before conversion, and require
lcns_follow to fit within the restart table entry.  Also validate each
log record's LCN span against the matched DPT entry before touching
dp->page_lcns.

Assisted-by: Codex:gpt-5.5
Signed-off-by: Kyle Zeng <kylebot@openai.com>
---
 fs/ntfs3/fslog.c | 61 +++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 55 insertions(+), 6 deletions(-)

diff --git a/fs/ntfs3/fslog.c b/fs/ntfs3/fslog.c
index acfa18b84401..1fc9d04806b5 100644
--- a/fs/ntfs3/fslog.c
+++ b/fs/ntfs3/fslog.c
@@ -647,6 +647,23 @@ static inline void *enum_rstbl(struct RESTART_TABLE *t, void *c)
 	return NULL;
 }
 
+static inline bool dptbl_lcns_in_range(const struct DIR_PAGE_ENTRY *dp,
+				       u64 vcn, u16 lcns_follow)
+{
+	u64 dp_vcn = le64_to_cpu(dp->vcn);
+	u32 dp_lcns = le32_to_cpu(dp->lcns_follow);
+	u64 off;
+
+	if (vcn < dp_vcn)
+		return false;
+
+	off = vcn - dp_vcn;
+	if (off > dp_lcns)
+		return false;
+
+	return lcns_follow <= dp_lcns - (u32)off;
+}
+
 /*
  * find_dp - Search for a @vcn in Dirty Page Table.
  */
@@ -657,12 +674,8 @@ static inline struct DIR_PAGE_ENTRY *find_dp(struct RESTART_TABLE *dptbl,
 	struct DIR_PAGE_ENTRY *dp = NULL;
 
 	while ((dp = enum_rstbl(dptbl, dp))) {
-		u64 dp_vcn = le64_to_cpu(dp->vcn);
-
-		if (dp->target_attr == ta && vcn >= dp_vcn &&
-		    vcn < dp_vcn + le32_to_cpu(dp->lcns_follow)) {
+		if (dp->target_attr == ta && dptbl_lcns_in_range(dp, vcn, 1))
 			return dp;
-		}
 	}
 	return NULL;
 }
@@ -778,6 +791,37 @@ static bool check_rstbl(const struct RESTART_TABLE *rt, size_t bytes)
 	return true;
 }
 
+static bool check_dptbl(const struct RESTART_TABLE *rt, bool is_restart_v0)
+{
+	u16 rsize = le16_to_cpu(rt->size);
+	struct DIR_PAGE_ENTRY *dp = NULL;
+
+	if (rsize < sizeof(struct DIR_PAGE_ENTRY))
+		return false;
+
+	if (is_restart_v0 && rsize < sizeof(struct DIR_PAGE_ENTRY_32))
+		return false;
+
+	while ((dp = enum_rstbl((struct RESTART_TABLE *)rt, dp))) {
+		u32 lcns_follow = le32_to_cpu(dp->lcns_follow);
+		size_t bytes;
+
+		bytes = struct_size(dp, page_lcns, lcns_follow);
+		if (bytes > rsize)
+			return false;
+
+		if (!is_restart_v0)
+			continue;
+
+		bytes = size_add(offsetof(struct DIR_PAGE_ENTRY_32, page_lcns_low),
+				 array_size(lcns_follow, sizeof(u64)));
+		if (bytes > rsize)
+			return false;
+	}
+
+	return true;
+}
+
 /*
  * free_rsttbl_idx - Free a previously allocated index a Restart Table.
  */
@@ -4204,7 +4248,7 @@ int log_replay(struct ntfs_inode *ni, bool *initialized)
 	t32 = rec_len - t16;
 
 	/* Now check that this is a valid restart table. */
-	if (!check_rstbl(rt, t32)) {
+	if (!check_rstbl(rt, t32) || !check_dptbl(rt, !rst->major_ver)) {
 		err = -EINVAL;
 		goto out;
 	}
@@ -4547,9 +4591,13 @@ int log_replay(struct ntfs_inode *ni, bool *initialized)
 		 * whole routine a loop, case Lcns do not fit below.
 		 */
 		t16 = le16_to_cpu(lrh->lcns_follow);
+		if (!dptbl_lcns_in_range(dp, t64, t16)) {
+			err = -EINVAL;
+			goto out;
+		}
+
 		for (i = 0; i < t16; i++) {
-			size_t j = (size_t)(le64_to_cpu(lrh->target_vcn) -
-					    le64_to_cpu(dp->vcn));
+			size_t j = (size_t)(t64 - le64_to_cpu(dp->vcn));
 			dp->page_lcns[j + i] = lrh->page_lcns[i];
 		}
 
@@ -4928,6 +4976,12 @@ int log_replay(struct ntfs_inode *ni, bool *initialized)
 	if (!dp)
 		goto read_next_log_do_action;
 
+	t16 = le16_to_cpu(lrh->lcns_follow);
+	if (!dptbl_lcns_in_range(dp, t64, t16)) {
+		err = -EINVAL;
+		goto out;
+	}
+
 	if (rec_lsn < le64_to_cpu(dp->oldest_lsn))
 		goto read_next_log_do_action;
 
-- 
2.43.0

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] fs/ntfs3: validate dirty page table entry sizes
  2026-06-11 21:33 [PATCH] fs/ntfs3: validate dirty page table entry sizes Kyle Zeng
@ 2026-07-23 14:25 ` Konstantin Komarov
  2026-07-24 11:30 ` Konstantin Komarov
  1 sibling, 0 replies; 6+ messages in thread
From: Konstantin Komarov @ 2026-07-23 14:25 UTC (permalink / raw)
  To: Kyle Zeng, ntfs3; +Cc: linux-kernel, outbounddisclosures

On 6/11/26 23:33, Kyle Zeng wrote:

> The generic restart table check only verifies the table header and
> free-list shape.  Dirty Page Table entries also carry an entry-local
> LCN array whose length is controlled by lcns_follow, and replay later
> trusts that value when converting version-0 entries and when copying
> LCNs from log records.
>
> A malformed $LogFile can provide a Dirty Page Table dump whose generic
> restart table is valid but whose entry is too small for the declared
> lcns_follow count.  log_replay() can then run the version-0 conversion
> memmove() past the kmemdup() allocation, or later copy/read past
> dp->page_lcns when a log record spans beyond the matched DPT entry.
>
> Add Dirty Page Table-specific validation before copying the table:
> require entries to be large enough for the typed DPT layout, require
> the old version-0 source layout to fit before conversion, and require
> lcns_follow to fit within the restart table entry.  Also validate each
> log record's LCN span against the matched DPT entry before touching
> dp->page_lcns.
>
> Assisted-by: Codex:gpt-5.5
> Signed-off-by: Kyle Zeng <kylebot@openai.com>
> ---
>   fs/ntfs3/fslog.c | 61 +++++++++++++++++++++++++++++++++++++++++++-----
>   1 file changed, 55 insertions(+), 6 deletions(-)
>
> diff --git a/fs/ntfs3/fslog.c b/fs/ntfs3/fslog.c
> index acfa18b84401..1fc9d04806b5 100644
> --- a/fs/ntfs3/fslog.c
> +++ b/fs/ntfs3/fslog.c
> @@ -647,6 +647,23 @@ static inline void *enum_rstbl(struct RESTART_TABLE *t, void *c)
>          return NULL;
>   }
>
> +static inline bool dptbl_lcns_in_range(const struct DIR_PAGE_ENTRY *dp,
> +                                      u64 vcn, u16 lcns_follow)
> +{
> +       u64 dp_vcn = le64_to_cpu(dp->vcn);
> +       u32 dp_lcns = le32_to_cpu(dp->lcns_follow);
> +       u64 off;
> +
> +       if (vcn < dp_vcn)
> +               return false;
> +
> +       off = vcn - dp_vcn;
> +       if (off > dp_lcns)
> +               return false;
> +
> +       return lcns_follow <= dp_lcns - (u32)off;
> +}
> +
>   /*
>    * find_dp - Search for a @vcn in Dirty Page Table.
>    */
> @@ -657,12 +674,8 @@ static inline struct DIR_PAGE_ENTRY *find_dp(struct RESTART_TABLE *dptbl,
>          struct DIR_PAGE_ENTRY *dp = NULL;
>
>          while ((dp = enum_rstbl(dptbl, dp))) {
> -               u64 dp_vcn = le64_to_cpu(dp->vcn);
> -
> -               if (dp->target_attr == ta && vcn >= dp_vcn &&
> -                   vcn < dp_vcn + le32_to_cpu(dp->lcns_follow)) {
> +               if (dp->target_attr == ta && dptbl_lcns_in_range(dp, vcn, 1))
>                          return dp;
> -               }
>          }
>          return NULL;
>   }
> @@ -778,6 +791,37 @@ static bool check_rstbl(const struct RESTART_TABLE *rt, size_t bytes)
>          return true;
>   }
>
> +static bool check_dptbl(const struct RESTART_TABLE *rt, bool is_restart_v0)
> +{
> +       u16 rsize = le16_to_cpu(rt->size);
> +       struct DIR_PAGE_ENTRY *dp = NULL;
> +
> +       if (rsize < sizeof(struct DIR_PAGE_ENTRY))
> +               return false;
> +
> +       if (is_restart_v0 && rsize < sizeof(struct DIR_PAGE_ENTRY_32))
> +               return false;
> +
> +       while ((dp = enum_rstbl((struct RESTART_TABLE *)rt, dp))) {
> +               u32 lcns_follow = le32_to_cpu(dp->lcns_follow);
> +               size_t bytes;
> +
> +               bytes = struct_size(dp, page_lcns, lcns_follow);
> +               if (bytes > rsize)
> +                       return false;
> +
> +               if (!is_restart_v0)
> +                       continue;
> +
> +               bytes = size_add(offsetof(struct DIR_PAGE_ENTRY_32, page_lcns_low),
> +                                array_size(lcns_follow, sizeof(u64)));
> +               if (bytes > rsize)
> +                       return false;
> +       }
> +
> +       return true;
> +}
> +
>   /*
>    * free_rsttbl_idx - Free a previously allocated index a Restart Table.
>    */
> @@ -4204,7 +4248,7 @@ int log_replay(struct ntfs_inode *ni, bool *initialized)
>          t32 = rec_len - t16;
>
>          /* Now check that this is a valid restart table. */
> -       if (!check_rstbl(rt, t32)) {
> +       if (!check_rstbl(rt, t32) || !check_dptbl(rt, !rst->major_ver)) {
>                  err = -EINVAL;
>                  goto out;
>          }
> @@ -4547,9 +4591,13 @@ int log_replay(struct ntfs_inode *ni, bool *initialized)
>                   * whole routine a loop, case Lcns do not fit below.
>                   */
>                  t16 = le16_to_cpu(lrh->lcns_follow);
> +               if (!dptbl_lcns_in_range(dp, t64, t16)) {
> +                       err = -EINVAL;
> +                       goto out;
> +               }
> +
>                  for (i = 0; i < t16; i++) {
> -                       size_t j = (size_t)(le64_to_cpu(lrh->target_vcn) -
> -                                           le64_to_cpu(dp->vcn));
> +                       size_t j = (size_t)(t64 - le64_to_cpu(dp->vcn));
>                          dp->page_lcns[j + i] = lrh->page_lcns[i];
>                  }
>
> @@ -4928,6 +4976,12 @@ int log_replay(struct ntfs_inode *ni, bool *initialized)
>          if (!dp)
>                  goto read_next_log_do_action;
>
> +       t16 = le16_to_cpu(lrh->lcns_follow);
> +       if (!dptbl_lcns_in_range(dp, t64, t16)) {
> +               err = -EINVAL;
> +               goto out;
> +       }
> +
>          if (rec_lsn < le64_to_cpu(dp->oldest_lsn))
>                  goto read_next_log_do_action;
>
> --
> 2.43.0

Hello,

Sorry for the delay.
Your patch was applied, thank you.

Regards,
Konstantin


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] fs/ntfs3: validate dirty page table entry sizes
  2026-06-11 21:33 [PATCH] fs/ntfs3: validate dirty page table entry sizes Kyle Zeng
  2026-07-23 14:25 ` Konstantin Komarov
@ 2026-07-24 11:30 ` Konstantin Komarov
  2026-07-25 18:25   ` Kyle Zeng
  1 sibling, 1 reply; 6+ messages in thread
From: Konstantin Komarov @ 2026-07-24 11:30 UTC (permalink / raw)
  To: Kyle Zeng, ntfs3; +Cc: linux-kernel, outbounddisclosures

On 6/11/26 23:33, Kyle Zeng wrote:

> The generic restart table check only verifies the table header and
> free-list shape.  Dirty Page Table entries also carry an entry-local
> LCN array whose length is controlled by lcns_follow, and replay later
> trusts that value when converting version-0 entries and when copying
> LCNs from log records.
>
> A malformed $LogFile can provide a Dirty Page Table dump whose generic
> restart table is valid but whose entry is too small for the declared
> lcns_follow count.  log_replay() can then run the version-0 conversion
> memmove() past the kmemdup() allocation, or later copy/read past
> dp->page_lcns when a log record spans beyond the matched DPT entry.
>
> Add Dirty Page Table-specific validation before copying the table:
> require entries to be large enough for the typed DPT layout, require
> the old version-0 source layout to fit before conversion, and require
> lcns_follow to fit within the restart table entry.  Also validate each
> log record's LCN span against the matched DPT entry before touching
> dp->page_lcns.
>
> Assisted-by: Codex:gpt-5.5
> Signed-off-by: Kyle Zeng <kylebot@openai.com>
> ---
>   fs/ntfs3/fslog.c | 61 +++++++++++++++++++++++++++++++++++++++++++-----
>   1 file changed, 55 insertions(+), 6 deletions(-)
>
> diff --git a/fs/ntfs3/fslog.c b/fs/ntfs3/fslog.c
> index acfa18b84401..1fc9d04806b5 100644
> --- a/fs/ntfs3/fslog.c
> +++ b/fs/ntfs3/fslog.c
> @@ -647,6 +647,23 @@ static inline void *enum_rstbl(struct RESTART_TABLE *t, void *c)
>          return NULL;
>   }
>
> +static inline bool dptbl_lcns_in_range(const struct DIR_PAGE_ENTRY *dp,
> +                                      u64 vcn, u16 lcns_follow)
> +{
> +       u64 dp_vcn = le64_to_cpu(dp->vcn);
> +       u32 dp_lcns = le32_to_cpu(dp->lcns_follow);
> +       u64 off;
> +
> +       if (vcn < dp_vcn)
> +               return false;
> +
> +       off = vcn - dp_vcn;
> +       if (off > dp_lcns)
> +               return false;
> +
> +       return lcns_follow <= dp_lcns - (u32)off;
> +}
> +
>   /*
>    * find_dp - Search for a @vcn in Dirty Page Table.
>    */
> @@ -657,12 +674,8 @@ static inline struct DIR_PAGE_ENTRY *find_dp(struct RESTART_TABLE *dptbl,
>          struct DIR_PAGE_ENTRY *dp = NULL;
>
>          while ((dp = enum_rstbl(dptbl, dp))) {
> -               u64 dp_vcn = le64_to_cpu(dp->vcn);
> -
> -               if (dp->target_attr == ta && vcn >= dp_vcn &&
> -                   vcn < dp_vcn + le32_to_cpu(dp->lcns_follow)) {
> +               if (dp->target_attr == ta && dptbl_lcns_in_range(dp, vcn, 1))
>                          return dp;
> -               }
>          }
>          return NULL;
>   }
> @@ -778,6 +791,37 @@ static bool check_rstbl(const struct RESTART_TABLE *rt, size_t bytes)
>          return true;
>   }
>
> +static bool check_dptbl(const struct RESTART_TABLE *rt, bool is_restart_v0)
> +{
> +       u16 rsize = le16_to_cpu(rt->size);
> +       struct DIR_PAGE_ENTRY *dp = NULL;
> +
> +       if (rsize < sizeof(struct DIR_PAGE_ENTRY))
> +               return false;
> +
> +       if (is_restart_v0 && rsize < sizeof(struct DIR_PAGE_ENTRY_32))
> +               return false;
> +
> +       while ((dp = enum_rstbl((struct RESTART_TABLE *)rt, dp))) {
> +               u32 lcns_follow = le32_to_cpu(dp->lcns_follow);
> +               size_t bytes;
> +
> +               bytes = struct_size(dp, page_lcns, lcns_follow);
> +               if (bytes > rsize)
> +                       return false;
> +
> +               if (!is_restart_v0)
> +                       continue;
> +
> +               bytes = size_add(offsetof(struct DIR_PAGE_ENTRY_32, page_lcns_low),
> +                                array_size(lcns_follow, sizeof(u64)));
> +               if (bytes > rsize)
> +                       return false;
> +       }
> +
> +       return true;
> +}
> +
>   /*
>    * free_rsttbl_idx - Free a previously allocated index a Restart Table.
>    */
> @@ -4204,7 +4248,7 @@ int log_replay(struct ntfs_inode *ni, bool *initialized)
>          t32 = rec_len - t16;
>
>          /* Now check that this is a valid restart table. */
> -       if (!check_rstbl(rt, t32)) {
> +       if (!check_rstbl(rt, t32) || !check_dptbl(rt, !rst->major_ver)) {
>                  err = -EINVAL;
>                  goto out;
>          }
> @@ -4547,9 +4591,13 @@ int log_replay(struct ntfs_inode *ni, bool *initialized)
>                   * whole routine a loop, case Lcns do not fit below.
>                   */
>                  t16 = le16_to_cpu(lrh->lcns_follow);
> +               if (!dptbl_lcns_in_range(dp, t64, t16)) {
> +                       err = -EINVAL;
> +                       goto out;
> +               }
> +
>                  for (i = 0; i < t16; i++) {
> -                       size_t j = (size_t)(le64_to_cpu(lrh->target_vcn) -
> -                                           le64_to_cpu(dp->vcn));
> +                       size_t j = (size_t)(t64 - le64_to_cpu(dp->vcn));
>                          dp->page_lcns[j + i] = lrh->page_lcns[i];
>                  }
>
> @@ -4928,6 +4976,12 @@ int log_replay(struct ntfs_inode *ni, bool *initialized)
>          if (!dp)
>                  goto read_next_log_do_action;
>
> +       t16 = le16_to_cpu(lrh->lcns_follow);
> +       if (!dptbl_lcns_in_range(dp, t64, t16)) {
> +               err = -EINVAL;
> +               goto out;
> +       }
> +
>          if (rec_lsn < le64_to_cpu(dp->oldest_lsn))
>                  goto read_next_log_do_action;
>
> --
> 2.43.0

Hello,

I didn't hit anything at first, but during internal testing journal
replay now fails on one of our images.

I can provide an image where the problem reproduces if that helps.

Regards,
Konstantin


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] fs/ntfs3: validate dirty page table entry sizes
  2026-07-24 11:30 ` Konstantin Komarov
@ 2026-07-25 18:25   ` Kyle Zeng
  2026-07-29 10:56     ` Konstantin Komarov
  0 siblings, 1 reply; 6+ messages in thread
From: Kyle Zeng @ 2026-07-25 18:25 UTC (permalink / raw)
  To: Konstantin Komarov; +Cc: ntfs3, linux-kernel, outbounddisclosures

> Hello,
> 
> I didn't hit anything at first, but during internal testing journal
> replay now fails on one of our images.
> 
> I can provide an image where the problem reproduces if that helps.
> 
> Regards,
> Konstantin
> 

Hi,

Can you provide the image and the test that triggers the issue?
Preferrably if the test can be made minimal.

Thanks,
Kyle

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] fs/ntfs3: validate dirty page table entry sizes
  2026-07-25 18:25   ` Kyle Zeng
@ 2026-07-29 10:56     ` Konstantin Komarov
  2026-08-04  1:16       ` Kyle Zeng
  0 siblings, 1 reply; 6+ messages in thread
From: Konstantin Komarov @ 2026-07-29 10:56 UTC (permalink / raw)
  To: Kyle Zeng; +Cc: ntfs3, linux-kernel, outbounddisclosures

[-- Attachment #1: Type: text/plain, Size: 604 bytes --]

On 7/25/26 20:25, Kyle Zeng wrote:

>> Hello,
>>
>> I didn't hit anything at first, but during internal testing journal
>> replay now fails on one of our images.
>>
>> I can provide an image where the problem reproduces if that helps.
>>
>> Regards,
>> Konstantin
>>
> Hi,
>
> Can you provide the image and the test that triggers the issue?
> Preferrably if the test can be made minimal.
>
> Thanks,
> Kyle

Hello,

I've attached an image that reproduces the problem: mounting fails to
replay the log file.

I can provide the commands I used to create it (image) if that would help.

Regards,
Konstantin

[-- Attachment #2: image.zst --]
[-- Type: application/zstd, Size: 850194 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] fs/ntfs3: validate dirty page table entry sizes
  2026-07-29 10:56     ` Konstantin Komarov
@ 2026-08-04  1:16       ` Kyle Zeng
  0 siblings, 0 replies; 6+ messages in thread
From: Kyle Zeng @ 2026-08-04  1:16 UTC (permalink / raw)
  To: Konstantin Komarov; +Cc: ntfs3, linux-kernel, outbounddisclosures

Hi Konstantin,

I tried to mount this image after the patch using the following command
and it works (I adapted the patch for the master branch).
~~~
mount -t ntfs3 -o loop,ro image /tmp/test
~~~
Can you share your setup (tree, commit, command) so I can reproduce and
fix the issue?

Best,
Kyle

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-08-04  1:16 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-11 21:33 [PATCH] fs/ntfs3: validate dirty page table entry sizes Kyle Zeng
2026-07-23 14:25 ` Konstantin Komarov
2026-07-24 11:30 ` Konstantin Komarov
2026-07-25 18:25   ` Kyle Zeng
2026-07-29 10:56     ` Konstantin Komarov
2026-08-04  1:16       ` Kyle Zeng

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox