Re: [PATCH] gfs2: validate stuffed inode size before unstuffing

On 05/07/2026 15:06, Jiaming Zhang wrote:
> A corrupted GFS2 image can store a dinode size that is larger than what VFS
> i_size can represent. gfs2_dinode_in() reads the on-disk di_size as a u64 and
> writes it directly into inode->i_size. If the value is larger than S64_MAX, the
> incore i_size becomes negative. That negative value can bypass the existing
> stuffed inode size check:
> 
> inode->i_size > gfs2_max_stuffed_size(ip)
> 
> Later, gfs2_quotad may try to sync the quota file and unstuff the quota inode.
> gfs2_unstuffer_folio() reads the negative i_size into an unsigned length and
> passes it to memcpy(), turning it into a huge copy size and triggering a
> out-of-bound issue.
> 
> Reject dinodes whose size exceeds sb->s_maxbytes before storing the value in
> inode->i_size. Also make the stuffed inode check use the raw on-disk size while
> it is still unsigned. As a defensive measure, validate the incore i_size again
> before unstuffing and pass the checked size down to gfs2_unstuffer_folio().
> 
> Fixes: 70376c7ff312 ("gfs2: Always check inode size of inline inodes")
> Closes: https://lore.kernel.org/lkml/CANypQFaF6bvORKKbRALvEL0k_epFaneFiOQqco4gjdmKVbdURg@mail.gmail.com/
> Assisted-by: Codex:gpt-5.5-xhigh
> Cc: [email protected]
> Signed-off-by: Jiaming Zhang <[email protected]>
> ---
>  fs/gfs2/bmap.c  | 18 ++++++++++++------
>  fs/gfs2/glops.c | 10 +++++++---
>  2 files changed, 19 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
> index 51ac1fd44f78..89c46c1d622c 100644
> --- a/fs/gfs2/bmap.c
> +++ b/fs/gfs2/bmap.c
> @@ -52,16 +52,15 @@ static int punch_hole(struct gfs2_inode *ip, u64 offset, u64 length);
>   * Returns: errno
>   */
>  static int gfs2_unstuffer_folio(struct gfs2_inode *ip, struct buffer_head *dibh,
> -			       u64 block, struct folio *folio)
> +			       u64 block, struct folio *folio, size_t size)
>  {
>  	struct inode *inode = &ip->i_inode;
>  
>  	if (!folio_test_uptodate(folio)) {
>  		void *kaddr = kmap_local_folio(folio, 0);
> -		u64 dsize = i_size_read(inode);
> - 
> -		memcpy(kaddr, dibh->b_data + sizeof(struct gfs2_dinode), dsize);
> -		memset(kaddr + dsize, 0, folio_size(folio) - dsize);
> +
> +		memcpy(kaddr, dibh->b_data + sizeof(struct gfs2_dinode), size);
> +		memset(kaddr + size, 0, folio_size(folio) - size);
>  		kunmap_local(kaddr);
>  
>  		folio_mark_uptodate(folio);
> @@ -92,9 +91,15 @@ static int __gfs2_unstuff_inode(struct gfs2_inode *ip, struct folio *folio)
>  	struct buffer_head *bh, *dibh;
>  	struct gfs2_dinode *di;
>  	u64 block = 0;
> +	loff_t size = i_size_read(&ip->i_inode);
>  	int isdir = gfs2_is_dir(ip);
>  	int error;
>  
> +	if (unlikely(size < 0 || size > gfs2_max_stuffed_size(ip))) {

How might the size be invalid here?

> +		gfs2_consist_inode(ip);
> +		return -EIO;
> +	}
> +
>  	error = gfs2_meta_inode_buffer(ip, &dibh);
>  	if (error)
>  		return error;
> @@ -116,7 +121,8 @@ static int __gfs2_unstuff_inode(struct gfs2_inode *ip, struct folio *folio)
>  					      dibh, sizeof(struct gfs2_dinode));
>  			brelse(bh);
>  		} else {
> -			error = gfs2_unstuffer_folio(ip, dibh, block, folio);
> +			error = gfs2_unstuffer_folio(ip, dibh, block, folio,
> +						     size);
>  			if (error)
>  				goto out_brelse;
>  		}
> diff --git a/fs/gfs2/glops.c b/fs/gfs2/glops.c
> index 28f32424ee64..33575fa681f5 100644
> --- a/fs/gfs2/glops.c
> +++ b/fs/gfs2/glops.c
> @@ -393,11 +393,16 @@ static int gfs2_dinode_in(struct gfs2_inode *ip, const void *buf)
>  	umode_t mode = be32_to_cpu(str->di_mode);
>  	struct inode *inode = &ip->i_inode;
>  	bool is_new = inode_state_read_once(inode) & I_NEW;
> +	u64 size = be64_to_cpu(str->di_size);
>  
>  	if (unlikely(ip->i_no_addr != be64_to_cpu(str->di_num.no_addr))) {
>  		gfs2_consist_inode(ip);
>  		return -EIO;
>  	}
> +	if (unlikely(size > (u64)inode->i_sb->s_maxbytes)) {
> +		gfs2_consist_inode(ip);
> +		return -EIO;
> +	}

This check is likely all that's needed.

Andy

>  	if (unlikely(!is_new && inode_wrong_type(inode, mode))) {
>  		gfs2_consist_inode(ip);
>  		return -EIO;
> @@ -418,7 +423,7 @@ static int gfs2_dinode_in(struct gfs2_inode *ip, const void *buf)
>  	i_uid_write(inode, be32_to_cpu(str->di_uid));
>  	i_gid_write(inode, be32_to_cpu(str->di_gid));
>  	set_nlink(inode, be32_to_cpu(str->di_nlink));
> -	i_size_write(inode, be64_to_cpu(str->di_size));
> +	i_size_write(inode, size);
>  	gfs2_set_inode_blocks(inode, be64_to_cpu(str->di_blocks));
>  	atime.tv_sec = be64_to_cpu(str->di_atime);
>  	atime.tv_nsec = be32_to_cpu(str->di_atime_nsec);
> @@ -462,7 +467,7 @@ static int gfs2_dinode_in(struct gfs2_inode *ip, const void *buf)
>  		return -EIO;
>  	}
>  
> -	if (gfs2_is_stuffed(ip) && inode->i_size > gfs2_max_stuffed_size(ip)) {
> +	if (gfs2_is_stuffed(ip) && size > gfs2_max_stuffed_size(ip)) {
>  		gfs2_consist_inode(ip);
>  		return -EIO;
>  	}
> @@ -707,4 +712,3 @@ const struct gfs2_glock_operations *gfs2_glops_list[] = {
>  	[LM_TYPE_QUOTA] = &gfs2_quota_glops,
>  	[LM_TYPE_JOURNAL] = &gfs2_journal_glops,
>  };
> -