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

[Jingbo Xu <jefflexu@linux.alibaba.com>]

Hi Miklos and Christian,

I find that these two fixes for 6.13 [1]:

fuse: fix direct io folio offset and length calculation
fuse: Set *nbytesp=0 in fuse_get_user_pages on allocation failure

are still out of v6.13-rc6 mainline, neither are they in
"linux-next/pending-fixes".  FYI in case of they got missed.

[1]
https://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/fuse.git/log/?h=for-next


On 12/3/24 7:01 AM, Bernd Schubert 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@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=87b8e6ed25dbc41759f7
> Fixes: 3b97c3652d91 ("fuse: convert direct io to use folios")
> Signed-off-by: Bernd Schubert <bschubert@ddn.com>
> Reviewed-by: Joanne Koong <joannelkoong@gmail.com>
> ---
> 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@ddn.com
> ---
>  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;
> 
> ---
> base-commit: e70140ba0d2b1a30467d4af6bcfe761327b9ec95
> change-id: 20241202-fix-fuse_get_user_pages-6a920cb04184
> 
> Best regards,

-- 
Thanks,
Jingbo