mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] ntfs: widen the sub-block offsets in ntfs_decompress()
@ 2026-10-09  7:55 Zhan Xusheng
  2026-10-10  1:15 ` liubaolin
  2026-10-12  0:49 ` Hyunchul Lee
  0 siblings, 2 replies; 3+ messages in thread
From: Zhan Xusheng @ 2026-10-09  7:55 UTC (permalink / raw)
  To: Namjae Jeon, Hyunchul Lee; +Cc: ntfs, linux-kernel, Zhan Xusheng

ntfs_decompress() tracks where the current sub-block starts and ends
inside the destination page with two u16s:

	u16 do_sb_start;
	u16 do_sb_end;
	...
	do_sb_start = *dest_ofs;
	do_sb_end = do_sb_start + NTFS_SB_SIZE;

*dest_ofs is an int page offset masked with ~PAGE_MASK, and
NTFS_SB_SIZE is 0x1000, so do_sb_end needs PAGE_SIZE + 0xfff of range.
That exceeds u16 once PAGE_SIZE reaches 0x10000: the last sub-block of a
page starts at 0xf000 and do_sb_end wraps from 0x10000 to 0.

The zero-fill that pads a short sub-block then computes a negative
length:

	if (dp_addr < dp_sb_end) {
		int nr_bytes = do_sb_end - *dest_ofs;

		memset(dp_addr, 0, nr_bytes);

A sub-block made only of symbol tokens reaches this without consulting
do_sb_end on the way, because that path just does

	*dp_addr++ = *cb++;
	++*dest_ofs;

and is bounded by dp_addr against dp_sb_end.  With do_sb_start at 0xf000
and eight literal bytes emitted, nr_bytes is -61448 and the memset()
runs for 18446744073709490168 bytes.

Getting there is cheap: fifteen sub-blocks of eleven bytes each advance
*dest_ofs to 0xf000, since the same zero-fill pads each of them out to
0x1000 while do_sb_end is still exact, and the sixteenth one triggers
the wrap.  A compression block is 0x10000 bytes with 4 KiB clusters and
the fixed compression unit of 4, so all sixteen fit with room to spare.

Phrase tokens are bounded against do_sb_end and so return -EOVERFLOW
instead, and the

	if (*dest_index == dest_max_index && do_sb_end > dest_max_ofs)

check at the top of the sub-block stops firing for the same reason.

Use int for both, matching *dest_ofs and dest_max_ofs.  The narrow type
came in with the original import and survived the removal and revert of
the driver.

Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
Fixes: 1e9ea7e04472 ("Revert "fs: Remove NTFS classic"")
---
 fs/ntfs/compress.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/ntfs/compress.c b/fs/ntfs/compress.c
index 075b57fc1de61..82c7c7d290737 100644
--- a/fs/ntfs/compress.c
+++ b/fs/ntfs/compress.c
@@ -182,8 +182,8 @@ static int ntfs_decompress(struct page *dest_pages[], int completed_pages[],
 	u8 *dp_addr;		/* Current pointer into dp. */
 	u8 *dp_sb_start;	/* Start of current sub-block in dp. */
 	u8 *dp_sb_end;		/* End of current sb in dp (dp_sb_start + NTFS_SB_SIZE). */
-	u16 do_sb_start;	/* @dest_ofs when starting this sub-block. */
-	u16 do_sb_end;		/* @dest_ofs of end of this sb (do_sb_start + NTFS_SB_SIZE). */
+	int do_sb_start;	/* @dest_ofs when starting this sub-block. */
+	int do_sb_end;		/* @dest_ofs of end of this sb (do_sb_start + NTFS_SB_SIZE). */
 
 	/* Variables for tag and token parsing. */
 	u8 tag;			/* Current tag. */

base-commit: 6c377d19d4a5116d9bec5203aa3c6c11523e7898
-- 
2.43.0


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

* Re: [PATCH] ntfs: widen the sub-block offsets in ntfs_decompress()
  2026-10-09  7:55 [PATCH] ntfs: widen the sub-block offsets in ntfs_decompress() Zhan Xusheng
@ 2026-10-10  1:15 ` liubaolin
  2026-10-12  0:49 ` Hyunchul Lee
  1 sibling, 0 replies; 3+ messages in thread
From: liubaolin @ 2026-10-10  1:15 UTC (permalink / raw)
  To: Zhan Xusheng, Namjae Jeon, Hyunchul Lee; +Cc: ntfs, linux-kernel, Zhan Xusheng



在 2026/10/9 15:55, Zhan Xusheng 写道:
> ntfs_decompress() tracks where the current sub-block starts and ends
> inside the destination page with two u16s:
> 
> 	u16 do_sb_start;
> 	u16 do_sb_end;
> 	...
> 	do_sb_start = *dest_ofs;
> 	do_sb_end = do_sb_start + NTFS_SB_SIZE;
> 
> *dest_ofs is an int page offset masked with ~PAGE_MASK, and
> NTFS_SB_SIZE is 0x1000, so do_sb_end needs PAGE_SIZE + 0xfff of range.
> That exceeds u16 once PAGE_SIZE reaches 0x10000: the last sub-block of a
> page starts at 0xf000 and do_sb_end wraps from 0x10000 to 0.
> 
> The zero-fill that pads a short sub-block then computes a negative
> length:
> 
> 	if (dp_addr < dp_sb_end) {
> 		int nr_bytes = do_sb_end - *dest_ofs;
> 
> 		memset(dp_addr, 0, nr_bytes);
> 
> A sub-block made only of symbol tokens reaches this without consulting
> do_sb_end on the way, because that path just does
> 
> 	*dp_addr++ = *cb++;
> 	++*dest_ofs;
> 
> and is bounded by dp_addr against dp_sb_end.  With do_sb_start at 0xf000
> and eight literal bytes emitted, nr_bytes is -61448 and the memset()
> runs for 18446744073709490168 bytes.
> 
> Getting there is cheap: fifteen sub-blocks of eleven bytes each advance
> *dest_ofs to 0xf000, since the same zero-fill pads each of them out to
> 0x1000 while do_sb_end is still exact, and the sixteenth one triggers
> the wrap.  A compression block is 0x10000 bytes with 4 KiB clusters and
> the fixed compression unit of 4, so all sixteen fit with room to spare.
> 
> Phrase tokens are bounded against do_sb_end and so return -EOVERFLOW
> instead, and the
> 
> 	if (*dest_index == dest_max_index && do_sb_end > dest_max_ofs)
> 
> check at the top of the sub-block stops firing for the same reason.
> 
> Use int for both, matching *dest_ofs and dest_max_ofs.  The narrow type
> came in with the original import and survived the removal and revert of
> the driver.
> 
> Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
> Fixes: 1e9ea7e04472 ("Revert "fs: Remove NTFS classic"")
> ---
>   fs/ntfs/compress.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/ntfs/compress.c b/fs/ntfs/compress.c
> index 075b57fc1de61..82c7c7d290737 100644
> --- a/fs/ntfs/compress.c
> +++ b/fs/ntfs/compress.c
> @@ -182,8 +182,8 @@ static int ntfs_decompress(struct page *dest_pages[], int completed_pages[],
>   	u8 *dp_addr;		/* Current pointer into dp. */
>   	u8 *dp_sb_start;	/* Start of current sub-block in dp. */
>   	u8 *dp_sb_end;		/* End of current sb in dp (dp_sb_start + NTFS_SB_SIZE). */
> -	u16 do_sb_start;	/* @dest_ofs when starting this sub-block. */
> -	u16 do_sb_end;		/* @dest_ofs of end of this sb (do_sb_start + NTFS_SB_SIZE). */
> +	int do_sb_start;	/* @dest_ofs when starting this sub-block. */
> +	int do_sb_end;		/* @dest_ofs of end of this sb (do_sb_start + NTFS_SB_SIZE). */
>   
>   	/* Variables for tag and token parsing. */
>   	u8 tag;			/* Current tag. */
> 
> base-commit: 6c377d19d4a5116d9bec5203aa3c6c11523e7898

Looks good to me.

Reviewed-by: Baolin Liu <liubaolin@kylinos.cn>


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

* Re: [PATCH] ntfs: widen the sub-block offsets in ntfs_decompress()
  2026-10-09  7:55 [PATCH] ntfs: widen the sub-block offsets in ntfs_decompress() Zhan Xusheng
  2026-10-10  1:15 ` liubaolin
@ 2026-10-12  0:49 ` Hyunchul Lee
  1 sibling, 0 replies; 3+ messages in thread
From: Hyunchul Lee @ 2026-10-12  0:49 UTC (permalink / raw)
  To: Zhan Xusheng; +Cc: Namjae Jeon, ntfs, linux-kernel, Zhan Xusheng

On Fri, Oct 09, 2026 at 03:55:23PM +0800, Zhan Xusheng wrote:
> ntfs_decompress() tracks where the current sub-block starts and ends
> inside the destination page with two u16s:
> 
> 	u16 do_sb_start;
> 	u16 do_sb_end;
> 	...
> 	do_sb_start = *dest_ofs;
> 	do_sb_end = do_sb_start + NTFS_SB_SIZE;
> 
> *dest_ofs is an int page offset masked with ~PAGE_MASK, and
> NTFS_SB_SIZE is 0x1000, so do_sb_end needs PAGE_SIZE + 0xfff of range.
> That exceeds u16 once PAGE_SIZE reaches 0x10000: the last sub-block of a
> page starts at 0xf000 and do_sb_end wraps from 0x10000 to 0.
> 
> The zero-fill that pads a short sub-block then computes a negative
> length:
> 
> 	if (dp_addr < dp_sb_end) {
> 		int nr_bytes = do_sb_end - *dest_ofs;
> 
> 		memset(dp_addr, 0, nr_bytes);
> 
> A sub-block made only of symbol tokens reaches this without consulting
> do_sb_end on the way, because that path just does
> 
> 	*dp_addr++ = *cb++;
> 	++*dest_ofs;
> 
> and is bounded by dp_addr against dp_sb_end.  With do_sb_start at 0xf000
> and eight literal bytes emitted, nr_bytes is -61448 and the memset()
> runs for 18446744073709490168 bytes.
> 
> Getting there is cheap: fifteen sub-blocks of eleven bytes each advance
> *dest_ofs to 0xf000, since the same zero-fill pads each of them out to
> 0x1000 while do_sb_end is still exact, and the sixteenth one triggers
> the wrap.  A compression block is 0x10000 bytes with 4 KiB clusters and
> the fixed compression unit of 4, so all sixteen fit with room to spare.
> 
> Phrase tokens are bounded against do_sb_end and so return -EOVERFLOW
> instead, and the
> 
> 	if (*dest_index == dest_max_index && do_sb_end > dest_max_ofs)
> 
> check at the top of the sub-block stops firing for the same reason.
> 
> Use int for both, matching *dest_ofs and dest_max_ofs.  The narrow type
> came in with the original import and survived the removal and revert of
> the driver.
> 
> Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
> Fixes: 1e9ea7e04472 ("Revert "fs: Remove NTFS classic"")

Looks good to me.

Reviewed-by: Hyunchul Lee <hyc.lee@gmail.com>

-- 
Thanks,
Hyunchul

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

end of thread, other threads:[~2026-10-12  0:50 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-09  7:55 [PATCH] ntfs: widen the sub-block offsets in ntfs_decompress() Zhan Xusheng
2026-10-10  1:15 ` liubaolin
2026-10-12  0:49 ` Hyunchul Lee

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

all inboxes | Powered by JetHome®