2023-07-10 09:51:44

by Chunhai Guo

Subject: [PATCH] erofs: fix to avoid infinite loop in z_erofs_do_read_page() when read page beyond EOF

z_erofs_do_read_page() may loop infinitely due to the inappropriate
truncation in the below statement. Since the offset is 64 bits and min_t()
truncates the result to 32 bits. The solution is to replace unsigned int
with a 64-bit type, such as erofs_off_t.
cur = end - min_t(unsigned int, offset + end - map->m_la, end);

- For example:
- offset = 0x400160000
- end = 0x370
- map->m_la = 0x160370
- offset + end - map->m_la = 0x400000000
- offset + end - map->m_la = 0x00000000 (truncated as unsigned int)
- Expected result:
- cur = 0
- Actual result:
- cur = 0x370

Signed-off-by: Chunhai Guo <[email protected]>
---
fs/erofs/zdata.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/erofs/zdata.c b/fs/erofs/zdata.c
index d9a0763f4595..b69d89a11dd0 100644
--- a/fs/erofs/zdata.c
+++ b/fs/erofs/zdata.c
@@ -1035,7 +1035,7 @@ static int z_erofs_do_read_page(struct z_erofs_decompress_frontend *fe,
*/
tight &= (fe->mode > Z_EROFS_PCLUSTER_FOLLOWED_NOINPLACE);

- cur = end - min_t(unsigned int, offset + end - map->m_la, end);
+ cur = end - min_t(erofs_off_t, offset + end - map->m_la, end);
if (!(map->m_flags & EROFS_MAP_MAPPED)) {
zero_user_segment(page, cur, end);
goto next_part;
--
2.25.1

2023-07-10 10:21:19

by Gao Xiang

Subject: Re: [PATCH] erofs: fix to avoid infinite loop in z_erofs_do_read_page() when read page beyond EOF

On 2023/7/10 17:34, Chunhai Guo wrote:
> z_erofs_do_read_page() may loop infinitely due to the inappropriate
> truncation in the below statement. Since the offset is 64 bits and min_t()
> truncates the result to 32 bits. The solution is to replace unsigned int
> with a 64-bit type, such as erofs_off_t.
> cur = end - min_t(unsigned int, offset + end - map->m_la, end);
>
> - For example:
> - offset = 0x400160000
> - end = 0x370
> - map->m_la = 0x160370
> - offset + end - map->m_la = 0x400000000
> - offset + end - map->m_la = 0x00000000 (truncated as unsigned int)
> - Expected result:
> - cur = 0
> - Actual result:
> - cur = 0x370
>
> Signed-off-by: Chunhai Guo <[email protected]>

I'd like to update the subject line manually to:

"erofs: avoid infinite loop in z_erofs_do_read_page() when reading beyond EOF"

since the original one is too long...

Otherwise it looks good to me,

Fixes: 3883a79abd02 ("staging: erofs: introduce VLE decompression support")
Reviewed-by: Gao Xiang <[email protected]>

Thanks,
Gao Xiang

2023-07-11 14:23:22

by Chao Yu

Subject: Re: [PATCH] erofs: fix to avoid infinite loop in z_erofs_do_read_page() when read page beyond EOF

On 2023/7/10 17:34, Chunhai Guo wrote:
> z_erofs_do_read_page() may loop infinitely due to the inappropriate
> truncation in the below statement. Since the offset is 64 bits and min_t()
> truncates the result to 32 bits. The solution is to replace unsigned int
> with a 64-bit type, such as erofs_off_t.
> cur = end - min_t(unsigned int, offset + end - map->m_la, end);
>
> - For example:
> - offset = 0x400160000
> - end = 0x370
> - map->m_la = 0x160370
> - offset + end - map->m_la = 0x400000000
> - offset + end - map->m_la = 0x00000000 (truncated as unsigned int)
> - Expected result:
> - cur = 0
> - Actual result:
> - cur = 0x370
>
> Signed-off-by: Chunhai Guo <[email protected]>

Reviewed-by: Chao Yu <[email protected]>

Thanks,