Re: [PATCH] fs: reject inodes whose size the superblock cannot address

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

On 9/4/26 11:15 AM, Sascha Hauer wrote:
> f_size is an alias for the inode's i_size, and filesystems read that
> size straight from untrusted media. ext4's ext4_isize() builds it as
> ((loff_t)size_high << 32) | size, so bit 31 of size_high lands in the
> sign bit; squashfs takes an unchecked le64 for LREG inodes. A negative
> i_size then feeds the offset arithmetic in __read(), __write() and
> friends, where it either wraps a size_t count to something huge or --
> depending on whether size_t is 32 or 64 bit -- turns the comparison
> unsigned and skips the EOF clamp altogether.
> 
> Catch this the way Linux does: give the superblock a ceiling and refuse
> sizes beyond it, instead of hardening every arithmetic site. Default
> s_maxbytes to MAX_LFS_FILESIZE in init_super(), which runs before the
> driver probe, so the filesystems that already lower it (jffs2, ubifs,
> squashfs, 9p) keep doing so and everyone else stops sitting at zero.
> 
> The check goes into do_dentry_open() rather than into iget: ten drivers
> only learn the size in their ->open() callback and write it to the
> inode through file->f_size, so a lookup time check would miss them.
> Casting to u64 makes a negative size exceed any sane s_maxbytes, so the
> sign is covered too; FILE_SIZE_STREAM is negative on purpose and has to
> be excluded.
> 
> Assisted-by: Claude:claude-opus-5
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  fs/fs.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/fs/fs.c b/fs/fs.c
> index ce41f23f88..9518f28539 100644
> --- a/fs/fs.c
> +++ b/fs/fs.c
> @@ -986,6 +986,7 @@ int fsdev_open_cdev(struct fs_device *fsdev)
>  static void init_super(struct super_block *sb)
>  {
>  	INIT_LIST_HEAD(&sb->s_inodes);
> +	sb->s_maxbytes = MAX_LFS_FILESIZE;
>  }
>  
>  static int fsdev_umount(struct fs_device *fsdev)
> @@ -2613,6 +2614,14 @@ static int rmdirat(int dirfd, const char *pathname)
>  	return errno_set(error);
>  }
>  
> +static bool i_size_valid(struct inode *inode)
> +{
> +	if (inode->i_size == FILE_SIZE_STREAM)
> +		return true;

Is FILE_SIZE_STREAM not a barebox convention? Couldn't a file system
have on disk FILE_SIZE_STREAM and trigger misbehavior this way?

> +
> +	return (u64)inode->i_size <= inode->i_sb->s_maxbytes;
> +}
> +
>  static int do_dentry_open(struct file *f)
>  {
>  	int error;
> @@ -2628,6 +2637,12 @@ static int do_dentry_open(struct file *f)
>  			return error;
>  	}
>  
> +	if (!i_size_valid(f->f_inode)) {
> +		dev_warn(&f->fsdev->dev, "%s: bad i_size value: %lld\n",
> +			 f->path, f->f_size);
> +		return -EUCLEAN;
> +	}
> +
>  	if (f->f_flags & O_TRUNC) {
>  		error = fsdev_truncate(f, 0);
>  		f->f_size = 0;

-- 
Pengutronix e.K.                  |                             |
Steuerwalder Str. 21              | http://www.pengutronix.de/  |
31137 Hildesheim, Germany         | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |