Re: [PATCH v2] fuse: Set *nbytesp=0 in fuse_get_user_pages on allocation failure

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Dec 2, 2024 at 3:01 PM Bernd Schubert <bschubert@xxxxxxx> wrote:
>
> In fuse_get_user_pages(), set *nbytesp to 0 when struct page **pages
> allocation fails. This prevents the caller (fuse_direct_io) from making
> incorrect assumptions that could lead to NULL pointer dereferences
> when processing the request reply.
>
> Previously, *nbytesp was left unmodified on allocation failure, which
> could cause issues if the caller assumed pages had been added to
> ap->descs[] when they hadn't.
>
> Reported-by: syzbot+87b8e6ed25dbc41759f7@xxxxxxxxxxxxxxxxxxxxxxxxx
> Closes: https://syzkaller.appspot.com/bug?extid=87b8e6ed25dbc41759f7
> Fixes: 3b97c3652d91 ("fuse: convert direct io to use folios")
> Signed-off-by: Bernd Schubert <bschubert@xxxxxxx>
> Reviewed-by: Joanne Koong <joannelkoong@xxxxxxxxx>
> ---
> Changes in v2:
> - Set ret in the (!pages) condition only to avoid returning
>   ENOMEM when the while loop would not do anything
> - Remove the error check in fuse_copy_do(), that is a bit debatable.
>   I had added it to prevent kernel crashes on fuse error, but then
>   it causes 'visual clutter' (Joanne)
> - Link to v1: https://lore.kernel.org/r/20241202-fix-fuse_get_user_pages-v1-1-8b5cccaf5bbe@xxxxxxx
> ---
>  fs/fuse/file.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> index 88d0946b5bc98705e0d895bc798aa4d9df080c3c..ae74d2b7ad5be14e4d157495e7c00fcf3fc40625 100644
> --- a/fs/fuse/file.c
> +++ b/fs/fuse/file.c
> @@ -1541,8 +1541,10 @@ static int fuse_get_user_pages(struct fuse_args_pages *ap, struct iov_iter *ii,
>          */
>         struct page **pages = kzalloc(max_pages * sizeof(struct page *),
>                                       GFP_KERNEL);
> -       if (!pages)
> -               return -ENOMEM;
> +       if (!pages) {
> +               ret = -ENOMEM;
> +               goto out;
> +       }
>
>         while (nbytes < *nbytesp && nr_pages < max_pages) {
>                 unsigned nfolios, i;
> @@ -1584,6 +1586,7 @@ static int fuse_get_user_pages(struct fuse_args_pages *ap, struct iov_iter *ii,
>         else
>                 ap->args.out_pages = true;
>
> +out:
>         *nbytesp = nbytes;
>
>         return ret < 0 ? ret : 0;
>

LGTM!

Thanks,
Joanne
> ---
> base-commit: e70140ba0d2b1a30467d4af6bcfe761327b9ec95
> change-id: 20241202-fix-fuse_get_user_pages-6a920cb04184
>
> Best regards,
> --
> Bernd Schubert <bschubert@xxxxxxx>
>





[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux