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

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

Hello Sascha,

On 9/4/26 1:13 PM, Sascha Hauer wrote:
> 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.

Sounds good.

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

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