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 <[email protected]> > > --- > > 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 |
