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

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

[Sascha Hauer]

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;
+
+	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;
-- 
2.47.3

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

[Ahmad Fatoum]

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 |

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

[Sascha Hauer]

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 |