2022-12-09 10:53:18

by Siddh Raman Pant

[permalink] [raw]
Subject: [PATCH v2] erofs/zmap.c: Fix incorrect offset calculation

Effective offset to add to length was being incorrectly calculated,
which resulted in iomap->length being set to 0, triggering a WARN_ON
in iomap_iter_done().

Fix that, and describe it in comments.

This was reported as a crash by syzbot under an issue about a warning
encountered in iomap_iter_done(), but unrelated to erofs.

C reproducer: https://syzkaller.appspot.com/text?tag=ReproC&x=1037a6b2880000
Kernel config: https://syzkaller.appspot.com/text?tag=KernelConfig&x=e2021a61197ebe02
Dashboard link: https://syzkaller.appspot.com/bug?extid=a8e049cd3abd342936b6

Reported-by: [email protected]
Suggested-by: Gao Xiang <[email protected]>
Signed-off-by: Siddh Raman Pant <[email protected]>
---
Changes since v2:
- Fix the calculation instead of bailing out.

fs/erofs/zmap.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/fs/erofs/zmap.c b/fs/erofs/zmap.c
index 0bb66927e3d0..a171e4caba3c 100644
--- a/fs/erofs/zmap.c
+++ b/fs/erofs/zmap.c
@@ -790,12 +790,16 @@ static int z_erofs_iomap_begin_report(struct inode *inode, loff_t offset,
iomap->type = IOMAP_HOLE;
iomap->addr = IOMAP_NULL_ADDR;
/*
- * No strict rule how to describe extents for post EOF, yet
- * we need do like below. Otherwise, iomap itself will get
+ * No strict rule on how to describe extents for post EOF, yet
+ * we need to do like below. Otherwise, iomap itself will get
* into an endless loop on post EOF.
+ *
+ * Calculate the effective offset by subtracting extent start
+ * (map.m_la) from the requested offset, and add it to length.
+ * (NB: offset >= map.m_la always)
*/
if (iomap->offset >= inode->i_size)
- iomap->length = length + map.m_la - offset;
+ iomap->length = length + offset - map.m_la;
}
iomap->flags = 0;
return 0;
--
2.35.1



2022-12-10 01:06:26

by Gao Xiang

[permalink] [raw]
Subject: Re: [PATCH v2] erofs/zmap.c: Fix incorrect offset calculation

On Fri, Dec 09, 2022 at 03:51:51PM +0530, Siddh Raman Pant via Linux-erofs wrote:
> Effective offset to add to length was being incorrectly calculated,
> which resulted in iomap->length being set to 0, triggering a WARN_ON
> in iomap_iter_done().
>
> Fix that, and describe it in comments.
>
> This was reported as a crash by syzbot under an issue about a warning
> encountered in iomap_iter_done(), but unrelated to erofs.
>
> C reproducer: https://syzkaller.appspot.com/text?tag=ReproC&x=1037a6b2880000
> Kernel config: https://syzkaller.appspot.com/text?tag=KernelConfig&x=e2021a61197ebe02
> Dashboard link: https://syzkaller.appspot.com/bug?extid=a8e049cd3abd342936b6
>
> Reported-by: [email protected]
> Suggested-by: Gao Xiang <[email protected]>
> Signed-off-by: Siddh Raman Pant <[email protected]>

It looks good to me!

Reviewed-by: Gao Xiang <[email protected]>

Thanks,
Gao Xiang

> ---
> Changes since v2:
> - Fix the calculation instead of bailing out.
>
> fs/erofs/zmap.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/fs/erofs/zmap.c b/fs/erofs/zmap.c
> index 0bb66927e3d0..a171e4caba3c 100644
> --- a/fs/erofs/zmap.c
> +++ b/fs/erofs/zmap.c
> @@ -790,12 +790,16 @@ static int z_erofs_iomap_begin_report(struct inode *inode, loff_t offset,
> iomap->type = IOMAP_HOLE;
> iomap->addr = IOMAP_NULL_ADDR;
> /*
> - * No strict rule how to describe extents for post EOF, yet
> - * we need do like below. Otherwise, iomap itself will get
> + * No strict rule on how to describe extents for post EOF, yet
> + * we need to do like below. Otherwise, iomap itself will get
> * into an endless loop on post EOF.
> + *
> + * Calculate the effective offset by subtracting extent start
> + * (map.m_la) from the requested offset, and add it to length.
> + * (NB: offset >= map.m_la always)
> */
> if (iomap->offset >= inode->i_size)
> - iomap->length = length + map.m_la - offset;
> + iomap->length = length + offset - map.m_la;
> }
> iomap->flags = 0;
> return 0;
> --
> 2.35.1
>
>

2023-01-09 11:13:52

by Chao Yu

[permalink] [raw]
Subject: Re: [PATCH v2] erofs/zmap.c: Fix incorrect offset calculation

On 2022/12/9 18:21, Siddh Raman Pant wrote:
> Effective offset to add to length was being incorrectly calculated,
> which resulted in iomap->length being set to 0, triggering a WARN_ON
> in iomap_iter_done().
>
> Fix that, and describe it in comments.
>
> This was reported as a crash by syzbot under an issue about a warning
> encountered in iomap_iter_done(), but unrelated to erofs.
>
> C reproducer: https://syzkaller.appspot.com/text?tag=ReproC&x=1037a6b2880000
> Kernel config: https://syzkaller.appspot.com/text?tag=KernelConfig&x=e2021a61197ebe02
> Dashboard link: https://syzkaller.appspot.com/bug?extid=a8e049cd3abd342936b6
>
> Reported-by: [email protected]
> Suggested-by: Gao Xiang <[email protected]>
> Signed-off-by: Siddh Raman Pant <[email protected]>

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

Thanks,