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

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: [email protected]
> Signed-off-by: Pavitra Jha <[email protected]>
> ---
>   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