fs/ntfs3: validate lcns_follow in log_replay conversion

[PATCH] fs/ntfs3: validate lcns_follow in log_replay conversion

Posted by Pavitra Jha 1 month, 1 week ago

log_replay() converts DIR_PAGE_ENTRY_32 records into DIR_PAGE_ENTRY
records when replaying version 0 restart tables.

During this conversion, the memmove() length is derived directly from
the on-disk lcns_follow field:

	memmove(&dp->vcn, &dp0->vcn_low,
		2 * sizeof(u64) +
				le32_to_cpu(dp->lcns_follow) * sizeof(u64));

check_rstbl() validates restart table structure, but does not constrain
per-entry lcns_follow values relative to the entry size. A malformed
filesystem image can provide an oversized lcns_follow value, causing
the conversion memmove() to access memory beyond the bounds of the
allocated restart table buffer.

The same field is later used to bound iteration over page_lcns[],
so validating lcns_follow during conversion also prevents downstream
out-of-bounds access from the same malformed metadata.

Compute the maximum valid lcns_follow from the already-validated
restart table entry size and reject entries that exceed this bound.
Reuse the existing t16/t32 scratch variables already declared in
log_replay() to avoid introducing new declarations.

Fixes: b46acd6a6a62 ("fs/ntfs3: Add NTFS journal")
Cc: stable@vger.kernel.org
Signed-off-by: Pavitra Jha <jhapavitra98@gmail.com>
---
 fs/ntfs3/fslog.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/fs/ntfs3/fslog.c b/fs/ntfs3/fslog.c
index c0237f7d0..91dc2d503 100644
--- a/fs/ntfs3/fslog.c
+++ b/fs/ntfs3/fslog.c
@@ -4215,13 +4215,22 @@ int log_replay(struct ntfs_inode *ni, bool *initialized)
 	if (rst->major_ver)
 		goto end_conv_1; /* reduce tab pressure. */
 
+	t16 = le16_to_cpu(dptbl->size);
+	if (t16 < sizeof(struct DIR_PAGE_ENTRY))
+		goto dirty_vol;
+
+	t32 = (t16 - sizeof(struct DIR_PAGE_ENTRY)) / sizeof(u64);
+
 	dp = NULL;
 	while ((dp = enum_rstbl(dptbl, dp))) {
 		struct DIR_PAGE_ENTRY_32 *dp0 = (struct DIR_PAGE_ENTRY_32 *)dp;
-		// NOTE: Danger. Check for of boundary.
+		u32 lcns = le32_to_cpu(dp->lcns_follow);
+
+		if (lcns > t32)
+			goto dirty_vol;
+
 		memmove(&dp->vcn, &dp0->vcn_low,
-			2 * sizeof(u64) +
-				le32_to_cpu(dp->lcns_follow) * sizeof(u64));
+			2 * sizeof(u64) + lcns * sizeof(u64));
 	}
 
 end_conv_1:
-- 
2.53.0

Re: [PATCH] fs/ntfs3: validate lcns_follow in log_replay conversion

Posted by Konstantin Komarov 1 week, 3 days ago

On 5/2/26 17:42, Pavitra Jha wrote:

> log_replay() converts DIR_PAGE_ENTRY_32 records into DIR_PAGE_ENTRY
> records when replaying version 0 restart tables.
>
> During this conversion, the memmove() length is derived directly from
> the on-disk lcns_follow field:
>
> 	memmove(&dp->vcn, &dp0->vcn_low,
> 		2 * sizeof(u64) +
> 				le32_to_cpu(dp->lcns_follow) * sizeof(u64));
>
> check_rstbl() validates restart table structure, but does not constrain
> per-entry lcns_follow values relative to the entry size. A malformed
> filesystem image can provide an oversized lcns_follow value, causing
> the conversion memmove() to access memory beyond the bounds of the
> allocated restart table buffer.
>
> The same field is later used to bound iteration over page_lcns[],
> so validating lcns_follow during conversion also prevents downstream
> out-of-bounds access from the same malformed metadata.
>
> Compute the maximum valid lcns_follow from the already-validated
> restart table entry size and reject entries that exceed this bound.
> Reuse the existing t16/t32 scratch variables already declared in
> log_replay() to avoid introducing new declarations.
>
> Fixes: b46acd6a6a62 ("fs/ntfs3: Add NTFS journal")
> Cc: stable@vger.kernel.org
> Signed-off-by: Pavitra Jha <jhapavitra98@gmail.com>
> ---
>   fs/ntfs3/fslog.c | 15 ++++++++++++---
>   1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/fs/ntfs3/fslog.c b/fs/ntfs3/fslog.c
> index c0237f7d0..91dc2d503 100644
> --- a/fs/ntfs3/fslog.c
> +++ b/fs/ntfs3/fslog.c
> @@ -4215,13 +4215,22 @@ int log_replay(struct ntfs_inode *ni, bool *initialized)
>   	if (rst->major_ver)
>   		goto end_conv_1; /* reduce tab pressure. */
>   
> +	t16 = le16_to_cpu(dptbl->size);
> +	if (t16 < sizeof(struct DIR_PAGE_ENTRY))
> +		goto dirty_vol;
> +
> +	t32 = (t16 - sizeof(struct DIR_PAGE_ENTRY)) / sizeof(u64);
> +
>   	dp = NULL;
>   	while ((dp = enum_rstbl(dptbl, dp))) {
>   		struct DIR_PAGE_ENTRY_32 *dp0 = (struct DIR_PAGE_ENTRY_32 *)dp;
> -		// NOTE: Danger. Check for of boundary.
> +		u32 lcns = le32_to_cpu(dp->lcns_follow);
> +
> +		if (lcns > t32)
> +			goto dirty_vol;
> +
>   		memmove(&dp->vcn, &dp0->vcn_low,
> -			2 * sizeof(u64) +
> -				le32_to_cpu(dp->lcns_follow) * sizeof(u64));
> +			2 * sizeof(u64) + lcns * sizeof(u64));
>   	}
>   
>   end_conv_1:

Hello,

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

Regards,
Konstantin