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

["Sascha Hauer" <s.hauer@pengutronix.de>]

On 2026-09-04 13:05, Ahmad Fatoum wrote:
> 
> 
> 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?

Hm, right. At this point we cannot distinguish between a valid
FILE_SIZE_STREAM and a value from a corrupted filesystem. Maybe we
should make this an extra field in struct inode rather than overloading
i_size with this information.

Sascha

--
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 |