Re: [PATCH] fuse: change fuse_wr_pages() to avoid signedness error from min()

[David Laight]

On Fri, 19 Dec 2025 11:24:23 +0800
Joanne Koong <joannelkoong@xxxxxxxxx> wrote:

> On Wed, Dec 17, 2025 at 12:22 AM <david.laight.linux@xxxxxxxxx> wrote:
> >
> > From: David Laight <david.laight.linux@xxxxxxxxx>
> >
> > On 32bit builds the 'number of pages required' calculation is signed
> > and min() complains because max_pages is unsigned.
> > Change the calcualtion that determines the number of pages by adding the
> > 'offset in page' to 'len' rather than subtracting the end and start pages.
> > Although the 64bit value is still signed, the compiler knows it isn't
> > negative so min() doesn't complain.
> > The generated code is also slightly better.
> >
> > Forcing the calculation to 32 bits (eg len + (size_t)(pos & ...))
> > generates much better code and is probably safe because len should
> > be limited to 'INT_MAX - PAGE_SIZE).
> >
> > Fixes: 0f5bb0cfb0b4 ("fs: use min() or umin() instead of min_t()")
> > Reported-by: kernel test robot <lkp@xxxxxxxxx>
> > Closes: https://lore.kernel.org/oe-kbuild-all/202512160948.O7QqxHj2-lkp@xxxxxxxxx/
> > Signed-off-by: David Laight <david.laight.linux@xxxxxxxxx>
> > ---
> >  fs/fuse/file.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> > index 4f71eb5a9bac..98edb6a2255d 100644
> > --- a/fs/fuse/file.c
> > +++ b/fs/fuse/file.c
> > @@ -1323,7 +1323,7 @@ static ssize_t fuse_fill_write_pages(struct fuse_io_args *ia,
> >  static inline unsigned int fuse_wr_pages(loff_t pos, size_t len,
> >                                      unsigned int max_pages)
> >  {
> > -       return min(((pos + len - 1) >> PAGE_SHIFT) - (pos >> PAGE_SHIFT) + 1,
> > +       return min(((len + (pos & (PAGE_SIZE - 1)) - 1) >> PAGE_SHIFT) + 1,
> >                    max_pages);  
> 
> I find this logic a bit confusing to read still, what about something like:
> 
> unsigned int nr_pages = DIV_ROUND_UP(offset_in_page(pos) + len, PAGE_SIZE);
> return min(nr_pages, max_pages);

You can just do:
	return min(DIV_ROUND_UP(offset_in_page(pos) + len, PAGE_SIZE), max_pages);

or splitting the long line:
	len += offset_in_page(pos);
	return min(DIV_ROUND_UP(len, PAGE_SIZE), max_pages);

Using offset_in_page() and DIV_ROUND_UP adds the 'hidden' requirement that
	'len <= MAX_ULONG - 2 * PAGE_SIZE'.
(Should be true - read/write (etc) are bounded to MAX_INT - PAGE_SIZE.)

> instead? I think the compiler will automatically optimize the
> DIV_ROUND_UP to use a bit shift.

Provided it is an unsigned divide - and the LHS is unsigned.

DIV_ROUNDUP(a, b) is '(a + b - 1)/b' which can overflow for large 'a'.
The other option is '(a - 1)/b + 1' which is valid for non-zero 'a'.

	David

> 
> Thanks,
> Joanne
> >  }
> >
> > --
> > 2.39.5
> >
> >