linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 1/4] ext4: fix out of bounds punch offset
@ 2025-05-06  1:20 Zhang Yi
  2025-05-06  1:20 ` [PATCH v2 2/4] ext4: fix incorrect punch max_end Zhang Yi
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Zhang Yi @ 2025-05-06  1:20 UTC (permalink / raw)
  To: linux-ext4
  Cc: linux-fsdevel, linux-kernel, tytso, adilger.kernel, jack,
	wanghaichi0403, yi.zhang, yi.zhang, libaokun1, yukuai3, yangerkun

From: Zhang Yi <yi.zhang@huawei.com>

Punching a hole with a start offset that exceeds max_end is not
permitted and will result in a negative length in the
truncate_inode_partial_folio() function while truncating the page cache,
potentially leading to undesirable consequences.

A simple reproducer:

  truncate -s 9895604649994 /mnt/foo
  xfs_io -c "pwrite 8796093022208 4096" /mnt/foo
  xfs_io -c "fpunch 8796093022213 25769803777" /mnt/foo

  kernel BUG at include/linux/highmem.h:275!
  Oops: invalid opcode: 0000 [#1] SMP PTI
  CPU: 3 UID: 0 PID: 710 Comm: xfs_io Not tainted 6.15.0-rc3
  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.3-2.fc40 04/01/2014
  RIP: 0010:zero_user_segments.constprop.0+0xd7/0x110
  RSP: 0018:ffffc90001cf3b38 EFLAGS: 00010287
  RAX: 0000000000000005 RBX: ffffea0001485e40 RCX: 0000000000001000
  RDX: 000000000040b000 RSI: 0000000000000005 RDI: 000000000040b000
  RBP: 000000000040affb R08: ffff888000000000 R09: ffffea0000000000
  R10: 0000000000000003 R11: 00000000fffc7fc5 R12: 0000000000000005
  R13: 000000000040affb R14: ffffea0001485e40 R15: ffff888031cd3000
  FS:  00007f4f63d0b780(0000) GS:ffff8880d337d000(0000)
  knlGS:0000000000000000
  CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
  CR2: 000000001ae0b038 CR3: 00000000536aa000 CR4: 00000000000006f0
  DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
  DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
  Call Trace:
   <TASK>
   truncate_inode_partial_folio+0x3dd/0x620
   truncate_inode_pages_range+0x226/0x720
   ? bdev_getblk+0x52/0x3e0
   ? ext4_get_group_desc+0x78/0x150
   ? crc32c_arch+0xfd/0x180
   ? __ext4_get_inode_loc+0x18c/0x840
   ? ext4_inode_csum+0x117/0x160
   ? jbd2_journal_dirty_metadata+0x61/0x390
   ? __ext4_handle_dirty_metadata+0xa0/0x2b0
   ? kmem_cache_free+0x90/0x5a0
   ? jbd2_journal_stop+0x1d5/0x550
   ? __ext4_journal_stop+0x49/0x100
   truncate_pagecache_range+0x50/0x80
   ext4_truncate_page_cache_block_range+0x57/0x3a0
   ext4_punch_hole+0x1fe/0x670
   ext4_fallocate+0x792/0x17d0
   ? __count_memcg_events+0x175/0x2a0
   vfs_fallocate+0x121/0x560
   ksys_fallocate+0x51/0xc0
   __x64_sys_fallocate+0x24/0x40
   x64_sys_call+0x18d2/0x4170
   do_syscall_64+0xa7/0x220
   entry_SYSCALL_64_after_hwframe+0x76/0x7e

Fix this by filtering out cases where the punching start offset exceeds
max_end.

Fixes: 982bf37da09d ("ext4: refactor ext4_punch_hole()")
Reported-by: Liebes Wang <wanghaichi0403@gmail.com>
Closes: https://lore.kernel.org/linux-ext4/ac3a58f6-e686-488b-a9ee-fc041024e43d@huawei.com/
Tested-by: Liebes Wang <wanghaichi0403@gmail.com>
Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
Reviewed-by: Jan Kara <jack@suse.cz>
---
 fs/ext4/inode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 94c7d2d828a6..4ec4a80b6879 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -4016,7 +4016,7 @@ int ext4_punch_hole(struct file *file, loff_t offset, loff_t length)
 	WARN_ON_ONCE(!inode_is_locked(inode));
 
 	/* No need to punch hole beyond i_size */
-	if (offset >= inode->i_size)
+	if (offset >= inode->i_size || offset >= max_end)
 		return 0;
 
 	/*
-- 
2.46.1


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

* [PATCH v2 2/4] ext4: fix incorrect punch max_end
  2025-05-06  1:20 [PATCH v2 1/4] ext4: fix out of bounds punch offset Zhang Yi
@ 2025-05-06  1:20 ` Zhang Yi
  2025-05-06  7:39   ` Baokun Li
  2025-05-06 11:30   ` Jan Kara
  2025-05-06  1:20 ` [PATCH v2 3/4] ext4: factor out ext4_get_maxbytes() Zhang Yi
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 10+ messages in thread
From: Zhang Yi @ 2025-05-06  1:20 UTC (permalink / raw)
  To: linux-ext4
  Cc: linux-fsdevel, linux-kernel, tytso, adilger.kernel, jack,
	wanghaichi0403, yi.zhang, yi.zhang, libaokun1, yukuai3, yangerkun

From: Zhang Yi <yi.zhang@huawei.com>

For the extents based inodes, the maxbytes should be sb->s_maxbytes
instead of sbi->s_bitmap_maxbytes. Additionally, for the calculation of
max_end, the -sb->s_blocksize operation is necessary only for
indirect-block based inodes. Correct the maxbytes and max_end value to
correct the behavior of punch hole.

Fixes: 2da376228a24 ("ext4: limit length to bitmap_maxbytes - blocksize in punch_hole")
Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
---
 fs/ext4/inode.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 4ec4a80b6879..5691966a19e1 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -4006,7 +4006,7 @@ int ext4_punch_hole(struct file *file, loff_t offset, loff_t length)
 	struct inode *inode = file_inode(file);
 	struct super_block *sb = inode->i_sb;
 	ext4_lblk_t start_lblk, end_lblk;
-	loff_t max_end = EXT4_SB(sb)->s_bitmap_maxbytes - sb->s_blocksize;
+	loff_t max_end = sb->s_maxbytes;
 	loff_t end = offset + length;
 	handle_t *handle;
 	unsigned int credits;
@@ -4015,14 +4015,20 @@ int ext4_punch_hole(struct file *file, loff_t offset, loff_t length)
 	trace_ext4_punch_hole(inode, offset, length, 0);
 	WARN_ON_ONCE(!inode_is_locked(inode));
 
+	/*
+	 * For indirect-block based inodes, make sure that the hole within
+	 * one block before last range.
+	 */
+	if (!ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS))
+		max_end = EXT4_SB(sb)->s_bitmap_maxbytes - sb->s_blocksize;
+
 	/* No need to punch hole beyond i_size */
 	if (offset >= inode->i_size || offset >= max_end)
 		return 0;
 
 	/*
 	 * If the hole extends beyond i_size, set the hole to end after
-	 * the page that contains i_size, and also make sure that the hole
-	 * within one block before last range.
+	 * the page that contains i_size.
 	 */
 	if (end > inode->i_size)
 		end = round_up(inode->i_size, PAGE_SIZE);
-- 
2.46.1


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

* [PATCH v2 3/4] ext4: factor out ext4_get_maxbytes()
  2025-05-06  1:20 [PATCH v2 1/4] ext4: fix out of bounds punch offset Zhang Yi
  2025-05-06  1:20 ` [PATCH v2 2/4] ext4: fix incorrect punch max_end Zhang Yi
@ 2025-05-06  1:20 ` Zhang Yi
  2025-05-06  7:40   ` Baokun Li
  2025-05-06  1:20 ` [PATCH v2 4/4] ext4: ensure i_size is smaller than maxbytes Zhang Yi
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: Zhang Yi @ 2025-05-06  1:20 UTC (permalink / raw)
  To: linux-ext4
  Cc: linux-fsdevel, linux-kernel, tytso, adilger.kernel, jack,
	wanghaichi0403, yi.zhang, yi.zhang, libaokun1, yukuai3, yangerkun

From: Zhang Yi <yi.zhang@huawei.com>

There are several locations that get the correct maxbytes value based on
the inode's block type. It would be beneficial to extract a common
helper function to make the code more clear.

Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
Reviewed-by: Jan Kara <jack@suse.cz>
---
 fs/ext4/ext4.h    | 7 +++++++
 fs/ext4/extents.c | 7 +------
 fs/ext4/file.c    | 7 +------
 3 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 5a20e9cd7184..8664bb5367c5 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -3378,6 +3378,13 @@ static inline unsigned int ext4_flex_bg_size(struct ext4_sb_info *sbi)
 	return 1 << sbi->s_log_groups_per_flex;
 }
 
+static inline loff_t ext4_get_maxbytes(struct inode *inode)
+{
+	if (ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS))
+		return inode->i_sb->s_maxbytes;
+	return EXT4_SB(inode->i_sb)->s_bitmap_maxbytes;
+}
+
 #define ext4_std_error(sb, errno)				\
 do {								\
 	if ((errno))						\
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index c616a16a9f36..b294d2f35a26 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -4931,12 +4931,7 @@ static const struct iomap_ops ext4_iomap_xattr_ops = {
 
 static int ext4_fiemap_check_ranges(struct inode *inode, u64 start, u64 *len)
 {
-	u64 maxbytes;
-
-	if (ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS))
-		maxbytes = inode->i_sb->s_maxbytes;
-	else
-		maxbytes = EXT4_SB(inode->i_sb)->s_bitmap_maxbytes;
+	u64 maxbytes = ext4_get_maxbytes(inode);
 
 	if (*len == 0)
 		return -EINVAL;
diff --git a/fs/ext4/file.c b/fs/ext4/file.c
index beb078ee4811..b845a25f7932 100644
--- a/fs/ext4/file.c
+++ b/fs/ext4/file.c
@@ -929,12 +929,7 @@ static int ext4_file_open(struct inode *inode, struct file *filp)
 loff_t ext4_llseek(struct file *file, loff_t offset, int whence)
 {
 	struct inode *inode = file->f_mapping->host;
-	loff_t maxbytes;
-
-	if (!(ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS)))
-		maxbytes = EXT4_SB(inode->i_sb)->s_bitmap_maxbytes;
-	else
-		maxbytes = inode->i_sb->s_maxbytes;
+	loff_t maxbytes = ext4_get_maxbytes(inode);
 
 	switch (whence) {
 	default:
-- 
2.46.1


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

* [PATCH v2 4/4] ext4: ensure i_size is smaller than maxbytes
  2025-05-06  1:20 [PATCH v2 1/4] ext4: fix out of bounds punch offset Zhang Yi
  2025-05-06  1:20 ` [PATCH v2 2/4] ext4: fix incorrect punch max_end Zhang Yi
  2025-05-06  1:20 ` [PATCH v2 3/4] ext4: factor out ext4_get_maxbytes() Zhang Yi
@ 2025-05-06  1:20 ` Zhang Yi
  2025-05-06  7:42   ` Baokun Li
  2025-05-06  7:37 ` [PATCH v2 1/4] ext4: fix out of bounds punch offset Baokun Li
  2025-05-20 14:40 ` Theodore Ts'o
  4 siblings, 1 reply; 10+ messages in thread
From: Zhang Yi @ 2025-05-06  1:20 UTC (permalink / raw)
  To: linux-ext4
  Cc: linux-fsdevel, linux-kernel, tytso, adilger.kernel, jack,
	wanghaichi0403, yi.zhang, yi.zhang, libaokun1, yukuai3, yangerkun

From: Zhang Yi <yi.zhang@huawei.com>

The inode i_size cannot be larger than maxbytes, check it while loading
inode from the disk.

Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
Reviewed-by: Jan Kara <jack@suse.cz>
---
 fs/ext4/inode.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 5691966a19e1..072b61140d12 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -4922,7 +4922,8 @@ struct inode *__ext4_iget(struct super_block *sb, unsigned long ino,
 		ei->i_file_acl |=
 			((__u64)le16_to_cpu(raw_inode->i_file_acl_high)) << 32;
 	inode->i_size = ext4_isize(sb, raw_inode);
-	if ((size = i_size_read(inode)) < 0) {
+	size = i_size_read(inode);
+	if (size < 0 || size > ext4_get_maxbytes(inode)) {
 		ext4_error_inode(inode, function, line, 0,
 				 "iget: bad i_size value: %lld", size);
 		ret = -EFSCORRUPTED;
-- 
2.46.1


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

* Re: [PATCH v2 1/4] ext4: fix out of bounds punch offset
  2025-05-06  1:20 [PATCH v2 1/4] ext4: fix out of bounds punch offset Zhang Yi
                   ` (2 preceding siblings ...)
  2025-05-06  1:20 ` [PATCH v2 4/4] ext4: ensure i_size is smaller than maxbytes Zhang Yi
@ 2025-05-06  7:37 ` Baokun Li
  2025-05-20 14:40 ` Theodore Ts'o
  4 siblings, 0 replies; 10+ messages in thread
From: Baokun Li @ 2025-05-06  7:37 UTC (permalink / raw)
  To: Zhang Yi
  Cc: linux-ext4, linux-fsdevel, linux-kernel, tytso, adilger.kernel,
	jack, wanghaichi0403, yi.zhang, yukuai3, yangerkun

On 2025/5/6 9:20, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@huawei.com>
>
> Punching a hole with a start offset that exceeds max_end is not
> permitted and will result in a negative length in the
> truncate_inode_partial_folio() function while truncating the page cache,
> potentially leading to undesirable consequences.
>
> A simple reproducer:
>
>    truncate -s 9895604649994 /mnt/foo
>    xfs_io -c "pwrite 8796093022208 4096" /mnt/foo
>    xfs_io -c "fpunch 8796093022213 25769803777" /mnt/foo
>
>    kernel BUG at include/linux/highmem.h:275!
>    Oops: invalid opcode: 0000 [#1] SMP PTI
>    CPU: 3 UID: 0 PID: 710 Comm: xfs_io Not tainted 6.15.0-rc3
>    Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.3-2.fc40 04/01/2014
>    RIP: 0010:zero_user_segments.constprop.0+0xd7/0x110
>    RSP: 0018:ffffc90001cf3b38 EFLAGS: 00010287
>    RAX: 0000000000000005 RBX: ffffea0001485e40 RCX: 0000000000001000
>    RDX: 000000000040b000 RSI: 0000000000000005 RDI: 000000000040b000
>    RBP: 000000000040affb R08: ffff888000000000 R09: ffffea0000000000
>    R10: 0000000000000003 R11: 00000000fffc7fc5 R12: 0000000000000005
>    R13: 000000000040affb R14: ffffea0001485e40 R15: ffff888031cd3000
>    FS:  00007f4f63d0b780(0000) GS:ffff8880d337d000(0000)
>    knlGS:0000000000000000
>    CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>    CR2: 000000001ae0b038 CR3: 00000000536aa000 CR4: 00000000000006f0
>    DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
>    DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
>    Call Trace:
>     <TASK>
>     truncate_inode_partial_folio+0x3dd/0x620
>     truncate_inode_pages_range+0x226/0x720
>     ? bdev_getblk+0x52/0x3e0
>     ? ext4_get_group_desc+0x78/0x150
>     ? crc32c_arch+0xfd/0x180
>     ? __ext4_get_inode_loc+0x18c/0x840
>     ? ext4_inode_csum+0x117/0x160
>     ? jbd2_journal_dirty_metadata+0x61/0x390
>     ? __ext4_handle_dirty_metadata+0xa0/0x2b0
>     ? kmem_cache_free+0x90/0x5a0
>     ? jbd2_journal_stop+0x1d5/0x550
>     ? __ext4_journal_stop+0x49/0x100
>     truncate_pagecache_range+0x50/0x80
>     ext4_truncate_page_cache_block_range+0x57/0x3a0
>     ext4_punch_hole+0x1fe/0x670
>     ext4_fallocate+0x792/0x17d0
>     ? __count_memcg_events+0x175/0x2a0
>     vfs_fallocate+0x121/0x560
>     ksys_fallocate+0x51/0xc0
>     __x64_sys_fallocate+0x24/0x40
>     x64_sys_call+0x18d2/0x4170
>     do_syscall_64+0xa7/0x220
>     entry_SYSCALL_64_after_hwframe+0x76/0x7e
>
> Fix this by filtering out cases where the punching start offset exceeds
> max_end.
>
> Fixes: 982bf37da09d ("ext4: refactor ext4_punch_hole()")
> Reported-by: Liebes Wang <wanghaichi0403@gmail.com>
> Closes: https://lore.kernel.org/linux-ext4/ac3a58f6-e686-488b-a9ee-fc041024e43d@huawei.com/
> Tested-by: Liebes Wang <wanghaichi0403@gmail.com>
> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
> Reviewed-by: Jan Kara <jack@suse.cz>
Looks good to me.

Reviewed-by: Baokun Li <libaokun1@huawei.com>
> ---
>   fs/ext4/inode.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 94c7d2d828a6..4ec4a80b6879 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -4016,7 +4016,7 @@ int ext4_punch_hole(struct file *file, loff_t offset, loff_t length)
>   	WARN_ON_ONCE(!inode_is_locked(inode));
>   
>   	/* No need to punch hole beyond i_size */
> -	if (offset >= inode->i_size)
> +	if (offset >= inode->i_size || offset >= max_end)
>   		return 0;
>   
>   	/*



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

* Re: [PATCH v2 2/4] ext4: fix incorrect punch max_end
  2025-05-06  1:20 ` [PATCH v2 2/4] ext4: fix incorrect punch max_end Zhang Yi
@ 2025-05-06  7:39   ` Baokun Li
  2025-05-06 11:30   ` Jan Kara
  1 sibling, 0 replies; 10+ messages in thread
From: Baokun Li @ 2025-05-06  7:39 UTC (permalink / raw)
  To: Zhang Yi
  Cc: linux-ext4, linux-fsdevel, linux-kernel, tytso, adilger.kernel,
	jack, wanghaichi0403, yi.zhang, yukuai3, yangerkun

On 2025/5/6 9:20, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@huawei.com>
>
> For the extents based inodes, the maxbytes should be sb->s_maxbytes
> instead of sbi->s_bitmap_maxbytes. Additionally, for the calculation of
> max_end, the -sb->s_blocksize operation is necessary only for
> indirect-block based inodes. Correct the maxbytes and max_end value to
> correct the behavior of punch hole.
>
> Fixes: 2da376228a24 ("ext4: limit length to bitmap_maxbytes - blocksize in punch_hole")
> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
Looks good to me.

Reviewed-by: Baokun Li <libaokun1@huawei.com>
> ---
>   fs/ext4/inode.c | 12 +++++++++---
>   1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 4ec4a80b6879..5691966a19e1 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -4006,7 +4006,7 @@ int ext4_punch_hole(struct file *file, loff_t offset, loff_t length)
>   	struct inode *inode = file_inode(file);
>   	struct super_block *sb = inode->i_sb;
>   	ext4_lblk_t start_lblk, end_lblk;
> -	loff_t max_end = EXT4_SB(sb)->s_bitmap_maxbytes - sb->s_blocksize;
> +	loff_t max_end = sb->s_maxbytes;
>   	loff_t end = offset + length;
>   	handle_t *handle;
>   	unsigned int credits;
> @@ -4015,14 +4015,20 @@ int ext4_punch_hole(struct file *file, loff_t offset, loff_t length)
>   	trace_ext4_punch_hole(inode, offset, length, 0);
>   	WARN_ON_ONCE(!inode_is_locked(inode));
>   
> +	/*
> +	 * For indirect-block based inodes, make sure that the hole within
> +	 * one block before last range.
> +	 */
> +	if (!ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS))
> +		max_end = EXT4_SB(sb)->s_bitmap_maxbytes - sb->s_blocksize;
> +
>   	/* No need to punch hole beyond i_size */
>   	if (offset >= inode->i_size || offset >= max_end)
>   		return 0;
>   
>   	/*
>   	 * If the hole extends beyond i_size, set the hole to end after
> -	 * the page that contains i_size, and also make sure that the hole
> -	 * within one block before last range.
> +	 * the page that contains i_size.
>   	 */
>   	if (end > inode->i_size)
>   		end = round_up(inode->i_size, PAGE_SIZE);



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

* Re: [PATCH v2 3/4] ext4: factor out ext4_get_maxbytes()
  2025-05-06  1:20 ` [PATCH v2 3/4] ext4: factor out ext4_get_maxbytes() Zhang Yi
@ 2025-05-06  7:40   ` Baokun Li
  0 siblings, 0 replies; 10+ messages in thread
From: Baokun Li @ 2025-05-06  7:40 UTC (permalink / raw)
  To: Zhang Yi
  Cc: linux-ext4, linux-fsdevel, linux-kernel, tytso, adilger.kernel,
	jack, wanghaichi0403, yi.zhang, yukuai3, yangerkun

On 2025/5/6 9:20, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@huawei.com>
>
> There are several locations that get the correct maxbytes value based on
> the inode's block type. It would be beneficial to extract a common
> helper function to make the code more clear.
>
> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
> Reviewed-by: Jan Kara <jack@suse.cz>
Nice cleanup! Looks good to me.

Reviewed-by: Baokun Li <libaokun1@huawei.com>
> ---
>   fs/ext4/ext4.h    | 7 +++++++
>   fs/ext4/extents.c | 7 +------
>   fs/ext4/file.c    | 7 +------
>   3 files changed, 9 insertions(+), 12 deletions(-)
>
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 5a20e9cd7184..8664bb5367c5 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -3378,6 +3378,13 @@ static inline unsigned int ext4_flex_bg_size(struct ext4_sb_info *sbi)
>   	return 1 << sbi->s_log_groups_per_flex;
>   }
>   
> +static inline loff_t ext4_get_maxbytes(struct inode *inode)
> +{
> +	if (ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS))
> +		return inode->i_sb->s_maxbytes;
> +	return EXT4_SB(inode->i_sb)->s_bitmap_maxbytes;
> +}
> +
>   #define ext4_std_error(sb, errno)				\
>   do {								\
>   	if ((errno))						\
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index c616a16a9f36..b294d2f35a26 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -4931,12 +4931,7 @@ static const struct iomap_ops ext4_iomap_xattr_ops = {
>   
>   static int ext4_fiemap_check_ranges(struct inode *inode, u64 start, u64 *len)
>   {
> -	u64 maxbytes;
> -
> -	if (ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS))
> -		maxbytes = inode->i_sb->s_maxbytes;
> -	else
> -		maxbytes = EXT4_SB(inode->i_sb)->s_bitmap_maxbytes;
> +	u64 maxbytes = ext4_get_maxbytes(inode);
>   
>   	if (*len == 0)
>   		return -EINVAL;
> diff --git a/fs/ext4/file.c b/fs/ext4/file.c
> index beb078ee4811..b845a25f7932 100644
> --- a/fs/ext4/file.c
> +++ b/fs/ext4/file.c
> @@ -929,12 +929,7 @@ static int ext4_file_open(struct inode *inode, struct file *filp)
>   loff_t ext4_llseek(struct file *file, loff_t offset, int whence)
>   {
>   	struct inode *inode = file->f_mapping->host;
> -	loff_t maxbytes;
> -
> -	if (!(ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS)))
> -		maxbytes = EXT4_SB(inode->i_sb)->s_bitmap_maxbytes;
> -	else
> -		maxbytes = inode->i_sb->s_maxbytes;
> +	loff_t maxbytes = ext4_get_maxbytes(inode);
>   
>   	switch (whence) {
>   	default:



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

* Re: [PATCH v2 4/4] ext4: ensure i_size is smaller than maxbytes
  2025-05-06  1:20 ` [PATCH v2 4/4] ext4: ensure i_size is smaller than maxbytes Zhang Yi
@ 2025-05-06  7:42   ` Baokun Li
  0 siblings, 0 replies; 10+ messages in thread
From: Baokun Li @ 2025-05-06  7:42 UTC (permalink / raw)
  To: Zhang Yi
  Cc: linux-ext4, linux-fsdevel, linux-kernel, tytso, adilger.kernel,
	jack, wanghaichi0403, yi.zhang, yukuai3, yangerkun

On 2025/5/6 9:20, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@huawei.com>
>
> The inode i_size cannot be larger than maxbytes, check it while loading
> inode from the disk.
>
> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
> Reviewed-by: Jan Kara <jack@suse.cz>
Looks good to me.

Reviewed-by: Baokun Li <libaokun1@huawei.com>
> ---
>   fs/ext4/inode.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 5691966a19e1..072b61140d12 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -4922,7 +4922,8 @@ struct inode *__ext4_iget(struct super_block *sb, unsigned long ino,
>   		ei->i_file_acl |=
>   			((__u64)le16_to_cpu(raw_inode->i_file_acl_high)) << 32;
>   	inode->i_size = ext4_isize(sb, raw_inode);
> -	if ((size = i_size_read(inode)) < 0) {
> +	size = i_size_read(inode);
> +	if (size < 0 || size > ext4_get_maxbytes(inode)) {
>   		ext4_error_inode(inode, function, line, 0,
>   				 "iget: bad i_size value: %lld", size);
>   		ret = -EFSCORRUPTED;



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

* Re: [PATCH v2 2/4] ext4: fix incorrect punch max_end
  2025-05-06  1:20 ` [PATCH v2 2/4] ext4: fix incorrect punch max_end Zhang Yi
  2025-05-06  7:39   ` Baokun Li
@ 2025-05-06 11:30   ` Jan Kara
  1 sibling, 0 replies; 10+ messages in thread
From: Jan Kara @ 2025-05-06 11:30 UTC (permalink / raw)
  To: Zhang Yi
  Cc: linux-ext4, linux-fsdevel, linux-kernel, tytso, adilger.kernel,
	jack, wanghaichi0403, yi.zhang, libaokun1, yukuai3, yangerkun

On Tue 06-05-25 09:20:07, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@huawei.com>
> 
> For the extents based inodes, the maxbytes should be sb->s_maxbytes
> instead of sbi->s_bitmap_maxbytes. Additionally, for the calculation of
> max_end, the -sb->s_blocksize operation is necessary only for
> indirect-block based inodes. Correct the maxbytes and max_end value to
> correct the behavior of punch hole.
> 
> Fixes: 2da376228a24 ("ext4: limit length to bitmap_maxbytes - blocksize in punch_hole")
> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/ext4/inode.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 4ec4a80b6879..5691966a19e1 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -4006,7 +4006,7 @@ int ext4_punch_hole(struct file *file, loff_t offset, loff_t length)
>  	struct inode *inode = file_inode(file);
>  	struct super_block *sb = inode->i_sb;
>  	ext4_lblk_t start_lblk, end_lblk;
> -	loff_t max_end = EXT4_SB(sb)->s_bitmap_maxbytes - sb->s_blocksize;
> +	loff_t max_end = sb->s_maxbytes;
>  	loff_t end = offset + length;
>  	handle_t *handle;
>  	unsigned int credits;
> @@ -4015,14 +4015,20 @@ int ext4_punch_hole(struct file *file, loff_t offset, loff_t length)
>  	trace_ext4_punch_hole(inode, offset, length, 0);
>  	WARN_ON_ONCE(!inode_is_locked(inode));
>  
> +	/*
> +	 * For indirect-block based inodes, make sure that the hole within
> +	 * one block before last range.
> +	 */
> +	if (!ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS))
> +		max_end = EXT4_SB(sb)->s_bitmap_maxbytes - sb->s_blocksize;
> +
>  	/* No need to punch hole beyond i_size */
>  	if (offset >= inode->i_size || offset >= max_end)
>  		return 0;
>  
>  	/*
>  	 * If the hole extends beyond i_size, set the hole to end after
> -	 * the page that contains i_size, and also make sure that the hole
> -	 * within one block before last range.
> +	 * the page that contains i_size.
>  	 */
>  	if (end > inode->i_size)
>  		end = round_up(inode->i_size, PAGE_SIZE);
> -- 
> 2.46.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

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

* Re: [PATCH v2 1/4] ext4: fix out of bounds punch offset
  2025-05-06  1:20 [PATCH v2 1/4] ext4: fix out of bounds punch offset Zhang Yi
                   ` (3 preceding siblings ...)
  2025-05-06  7:37 ` [PATCH v2 1/4] ext4: fix out of bounds punch offset Baokun Li
@ 2025-05-20 14:40 ` Theodore Ts'o
  4 siblings, 0 replies; 10+ messages in thread
From: Theodore Ts'o @ 2025-05-20 14:40 UTC (permalink / raw)
  To: linux-ext4, Zhang Yi
  Cc: Theodore Ts'o, linux-fsdevel, linux-kernel, adilger.kernel,
	jack, wanghaichi0403, yi.zhang, libaokun1, yukuai3, yangerkun


On Tue, 06 May 2025 09:20:06 +0800, Zhang Yi wrote:
> Punching a hole with a start offset that exceeds max_end is not
> permitted and will result in a negative length in the
> truncate_inode_partial_folio() function while truncating the page cache,
> potentially leading to undesirable consequences.
> 
> A simple reproducer:
> 
> [...]

Applied, thanks!

[1/4] ext4: fix out of bounds punch offset
      commit: b5e58bcd79625423487fa3ecba8e8411b5396327
[2/4] ext4: fix incorrect punch max_end
      commit: 29ec9bed2395061350249ae356fb300dd82a78e7
[3/4] ext4: factor out ext4_get_maxbytes()
      commit: dbe27f06fa38b9bfc598f8864ae1c5d5831d9992
[4/4] ext4: ensure i_size is smaller than maxbytes
      commit: 1a77a028a392fab66dd637cdfac3f888450d00af

Best regards,
-- 
Theodore Ts'o <tytso@mit.edu>

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

end of thread, other threads:[~2025-05-20 14:40 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-06  1:20 [PATCH v2 1/4] ext4: fix out of bounds punch offset Zhang Yi
2025-05-06  1:20 ` [PATCH v2 2/4] ext4: fix incorrect punch max_end Zhang Yi
2025-05-06  7:39   ` Baokun Li
2025-05-06 11:30   ` Jan Kara
2025-05-06  1:20 ` [PATCH v2 3/4] ext4: factor out ext4_get_maxbytes() Zhang Yi
2025-05-06  7:40   ` Baokun Li
2025-05-06  1:20 ` [PATCH v2 4/4] ext4: ensure i_size is smaller than maxbytes Zhang Yi
2025-05-06  7:42   ` Baokun Li
2025-05-06  7:37 ` [PATCH v2 1/4] ext4: fix out of bounds punch offset Baokun Li
2025-05-20 14:40 ` Theodore Ts'o

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).