Re: [RFC PATCH] fuse: fix race in fuse_notify_store()

[Bernd Schubert <bernd@bsbernd.com>]

Hi Luis,

On 1/29/25 19:44, Luis Henriques wrote:
> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> index 27ccae63495d..9a0cd88a9bb9 100644
> --- a/fs/fuse/dev.c
> +++ b/fs/fuse/dev.c
> @@ -1630,6 +1630,7 @@ static int fuse_notify_store(struct fuse_conn *fc, unsigned int size,
>  	unsigned int num;
>  	loff_t file_size;
>  	loff_t end;
> +	int fgp_flags = FGP_LOCK | FGP_ACCESSED | FGP_CREAT;
>  
>  	err = -EINVAL;
>  	if (size < sizeof(outarg))
> @@ -1645,6 +1646,9 @@ static int fuse_notify_store(struct fuse_conn *fc, unsigned int size,
>  
>  	nodeid = outarg.nodeid;
>  
> +	if (outarg.flags & FUSE_NOTIFY_STORE_NOWAIT)
> +		fgp_flags |= FGP_NOWAIT;
> +
>  	down_read(&fc->killsb);
>  
>  	err = -ENOENT;
> @@ -1668,14 +1672,25 @@ static int fuse_notify_store(struct fuse_conn *fc, unsigned int size,
>  		struct page *page;
>  		unsigned int this_num;
>  
> -		folio = filemap_grab_folio(mapping, index);
> -		err = PTR_ERR(folio);
> -		if (IS_ERR(folio))
> -			goto out_iput;
> +		folio = __filemap_get_folio(mapping, index, fgp_flags,
> +					    mapping_gfp_mask(mapping));
> +		err = PTR_ERR_OR_ZERO(folio);
> +		if (err) {
> +			if (!(outarg.flags & FUSE_NOTIFY_STORE_NOWAIT))
> +				goto out_iput;
> +			page = NULL;
> +			/* XXX */

What is the XXX for? 
Also, I think you want to go to "skip" only on -EAGAIN? And if so, need
to unset err? 


> +			this_num = min_t(unsigned int, num, PAGE_SIZE - offset);
> +		} else {
> +			page = &folio->page;
> +			this_num = min_t(unsigned int, num,
> +					 folio_size(folio) - offset);
> +		}
>  
> -		page = &folio->page;
> -		this_num = min_t(unsigned, num, folio_size(folio) - offset);
>  		err = fuse_copy_page(cs, &page, offset, this_num, 0);
> +		if (!page)
> +			goto skip;
> +
>  		if (!folio_test_uptodate(folio) && !err && offset == 0 &&
>  		    (this_num == folio_size(folio) || file_size == end)) {
>  			folio_zero_segment(folio, this_num, folio_size(folio));
> @@ -1683,7 +1698,7 @@ static int fuse_notify_store(struct fuse_conn *fc, unsigned int size,
>  		}
>  		folio_unlock(folio);
>  		folio_put(folio);
> -
> +skip:
>  		if (err)
>  			goto out_iput;
>  
> diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
> index e9e78292d107..59725f89340e 100644
> --- a/include/uapi/linux/fuse.h
> +++ b/include/uapi/linux/fuse.h
> @@ -576,6 +576,12 @@ struct fuse_file_lock {
>   */
>  #define FUSE_EXPIRE_ONLY		(1 << 0)
>  
> +/**
> + * notify_store flags
> + * FUSE_NOTIFY_STORE_NOWAIT: skip locked pages
> + */
> +#define FUSE_NOTIFY_STORE_NOWAIT	(1 << 0)
> +
>  /**
>   * extension type
>   * FUSE_MAX_NR_SECCTX: maximum value of &fuse_secctx_header.nr_secctx
> @@ -1075,7 +1081,7 @@ struct fuse_notify_store_out {
>  	uint64_t	nodeid;
>  	uint64_t	offset;
>  	uint32_t	size;
> -	uint32_t	padding;
> +	uint32_t	flags;
>  };
>  
>  struct fuse_notify_retrieve_out {
> 

Thanks,
Bernd