Re: [PATCH v2 1/4] fuse: validate outarg offset and size in notify store/retrieve

["Darrick J. Wong" <djwong@kernel.org>]

On Tue, Jan 20, 2026 at 04:03:10PM -0800, Darrick J. Wong wrote:
> On Tue, Jan 20, 2026 at 02:44:46PM -0800, Joanne Koong wrote:
> > Add validation checking for outarg offset and outarg size values passed
> > in by the server. MAX_LFS_FILESIZE is the maximum file size supported.
> > The fuse_notify_store_out and fuse_notify_retrieve_out structs take in
> > a uint64_t offset.
> > 
> > Add logic to ensure:
> > * outarg.offset is less than MAX_LFS_FILESIZE
> > * outarg.offset + outarg.size cannot exceed MAX_LFS_FILESIZE
> > * potential uint64_t overflow is fixed when adding outarg.offset and
> >   outarg.size.
> > 
> > Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
> > ---
> >  fs/fuse/dev.c | 14 ++++++++++----
> >  1 file changed, 10 insertions(+), 4 deletions(-)
> > 
> > diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> > index 6d59cbc877c6..7558ff337413 100644
> > --- a/fs/fuse/dev.c
> > +++ b/fs/fuse/dev.c
> > @@ -1781,7 +1781,11 @@ static int fuse_notify_store(struct fuse_conn *fc, unsigned int size,
> >  	if (size - sizeof(outarg) != outarg.size)
> >  		return -EINVAL;
> >  
> > +	if (outarg.offset >= MAX_LFS_FILESIZE)
> 
> Hrmm.  Normally I'd recommend generic_write_check_limits, but you don't
> actually have a struct file.
> 
> Being pedantic, you might want to check this against
> super_block::s_maxbytes, though the current fuse codebase doesn't
> support any value other than MAX_LFS_FILESIZE.
> 
> (fuse-iomap will allow servers to lower s_maxbytes)
> 
> > +		return -EINVAL;
> > +
> >  	nodeid = outarg.nodeid;
> > +	num = min(outarg.size, MAX_LFS_FILESIZE - outarg.offset);
> >  
> >  	down_read(&fc->killsb);
> >  
> > @@ -1794,13 +1798,12 @@ static int fuse_notify_store(struct fuse_conn *fc, unsigned int size,
> >  	index = outarg.offset >> PAGE_SHIFT;
> >  	offset = outarg.offset & ~PAGE_MASK;
> >  	file_size = i_size_read(inode);
> > -	end = outarg.offset + outarg.size;
> > +	end = outarg.offset + num;
> >  	if (end > file_size) {
> >  		file_size = end;
> > -		fuse_write_update_attr(inode, file_size, outarg.size);
> > +		fuse_write_update_attr(inode, file_size, num);
> >  	}
> >  
> > -	num = outarg.size;
> >  	while (num) {
> >  		struct folio *folio;
> >  		unsigned int folio_offset;
> > @@ -1880,7 +1883,7 @@ static int fuse_retrieve(struct fuse_mount *fm, struct inode *inode,
> >  	num = min(outarg->size, fc->max_write);
> >  	if (outarg->offset > file_size)
> >  		num = 0;
> > -	else if (outarg->offset + num > file_size)
> > +	else if (num > file_size - outarg->offset)
> >  		num = file_size - outarg->offset;
> >  
> >  	num_pages = (num + offset + PAGE_SIZE - 1) >> PAGE_SHIFT;
> > @@ -1962,6 +1965,9 @@ static int fuse_notify_retrieve(struct fuse_conn *fc, unsigned int size,
> >  
> >  	fuse_copy_finish(cs);
> >  
> > +	if (outarg.offset >= MAX_LFS_FILESIZE)
> 
> Can this actually happen?  It's strange to succeed at injecting data
> into the pagecache but then fail anyway.

Oh silly me, this is a different function.  I think the above hunk
prevents this scenario from happening, doesn't it?

--D

> --D
> 
> > +		return -EINVAL;
> > +
> >  	down_read(&fc->killsb);
> >  	err = -ENOENT;
> >  	nodeid = outarg.nodeid;
> > -- 
> > 2.47.3
> > 
> > 
>