[bug report] fuse: support copying large folios

[bug report] fuse: support copying large folios

[Dan Carpenter]

Hello Joanne Koong,

This is a semi-automatic email about new static checker warnings.

Commit f008a4390bde ("fuse: support copying large folios") from May
12, 2025, leads to the following Smatch complaint:

    fs/fuse/dev.c:1103 fuse_copy_folio()
    warn: variable dereferenced before check 'folio' (see line 1101)

fs/fuse/dev.c
  1100		struct folio *folio = *foliop;
  1101		size_t size = folio_size(folio);
                                         ^^^^^
The patch adds an unchecked dereference

  1102	
  1103		if (folio && zeroing && count < size)
                    ^^^^^
and it also adds this check for NULL which is too late.

  1104			folio_zero_range(folio, 0, size);
  1105	

regards,
dan carpenter

Re: [bug report] fuse: support copying large folios

[Joanne Koong]

On Fri, May 23, 2025 at 8:59 AM Dan Carpenter <dan.carpenter@linaro.org> wrote:
>
> Hello Joanne Koong,
>
> This is a semi-automatic email about new static checker warnings.
>
> Commit f008a4390bde ("fuse: support copying large folios") from May
> 12, 2025, leads to the following Smatch complaint:
>
>     fs/fuse/dev.c:1103 fuse_copy_folio()
>     warn: variable dereferenced before check 'folio' (see line 1101)
>
> fs/fuse/dev.c
>   1100          struct folio *folio = *foliop;
>   1101          size_t size = folio_size(folio);
>                                          ^^^^^
> The patch adds an unchecked dereference
>
>   1102
>   1103          if (folio && zeroing && count < size)
>                     ^^^^^
> and it also adds this check for NULL which is too late.
>
>   1104                  folio_zero_range(folio, 0, size);
>   1105

Thanks for flagging. I looked through where we call fuse_copy_folio()
and we'll never run into the case where folio is null, so all the "if
folio" branches inside there can probably be cleaned up with a WARN_ON
check.

I'll submit a patch that fixes this commit and a separate patch that
cleans up the if folio check.

>
> regards,
> dan carpenter

Re: [bug report] fuse: support copying large folios

[Dan Carpenter]

On Fri, May 23, 2025 at 10:32:29AM -0700, Joanne Koong wrote:
> On Fri, May 23, 2025 at 8:59 AM Dan Carpenter <dan.carpenter@linaro.org> wrote:
> >
> > Hello Joanne Koong,
> >
> > This is a semi-automatic email about new static checker warnings.
> >
> > Commit f008a4390bde ("fuse: support copying large folios") from May
> > 12, 2025, leads to the following Smatch complaint:
> >
> >     fs/fuse/dev.c:1103 fuse_copy_folio()
> >     warn: variable dereferenced before check 'folio' (see line 1101)
> >
> > fs/fuse/dev.c
> >   1100          struct folio *folio = *foliop;
> >   1101          size_t size = folio_size(folio);
> >                                          ^^^^^
> > The patch adds an unchecked dereference
> >
> >   1102
> >   1103          if (folio && zeroing && count < size)
> >                     ^^^^^
> > and it also adds this check for NULL which is too late.
> >
> >   1104                  folio_zero_range(folio, 0, size);
> >   1105
> 
> Thanks for flagging. I looked through where we call fuse_copy_folio()
> and we'll never run into the case where folio is null, so all the "if
> folio" branches inside there can probably be cleaned up with a WARN_ON
> check.
> 
> I'll submit a patch that fixes this commit and a separate patch that
> cleans up the if folio check.

Another idea is to just crash when people pass a NULL pointer.  The stack
traces from NULL dereference bugs are normally easy to debug unless
they're caused by a race condition or memory corruption.

regards,
dan carpenter