Re: [PATCH] fuse: don't truncate cached, mutated symlink

[Luis Henriques <luis@igalia.com>]

On Thu, Feb 20 2025, Miklos Szeredi wrote:

> Fuse allows the value of a symlink to change and this property is exploited
> by some filesystems (e.g. CVMFS).
>
> It has been observed, that sometimes after changing the symlink contents,
> the value is truncated to the old size.
>
> This is caused by fuse_getattr() racing with fuse_reverse_inval_inode().
> fuse_reverse_inval_inode() updates the fuse_inode's attr_version, which
> results in fuse_change_attributes() exiting before updating the cached
> attributes
>
> This is okay, as the cached attributes remain invalid and the next call to
> fuse_change_attributes() will likely update the inode with the correct
> values.
>
> The reason this causes problems is that cached symlinks will be
> returned through page_get_link(), which truncates the symlink to
> inode->i_size.  This is correct for filesystems that don't mutate
> symlinks, but in this case it causes bad behavior.
>
> The solution is to just remove this truncation.  This can cause a
> regression in a filesystem that relies on supplying a symlink larger than
> the file size, but this is unlikely.  If that happens we'd need to make
> this behavior conditional.
>
> Reported-by: Laura Promberger <laura.promberger@cern.ch>
> Tested-by: Sam Lewis <samclewis@google.com>
> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>

OK, I finally managed to reproduce the bug (thanks for the hints, Sam!)
and I can also confirm this patch fixes it.

So, feel free to add my

Reviewed-by: Luis Henriques <luis@igalia.com>
Tested-by: Luis Henriques <luis@igalia.com>

Cheers,
-- 
Luís

> ---
>  fs/fuse/dir.c      |  2 +-
>  fs/namei.c         | 24 +++++++++++++++++++-----
>  include/linux/fs.h |  2 ++
>  3 files changed, 22 insertions(+), 6 deletions(-)
>
> diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
> index 589e88822368..83c56ce6ad20 100644
> --- a/fs/fuse/dir.c
> +++ b/fs/fuse/dir.c
> @@ -1645,7 +1645,7 @@ static const char *fuse_get_link(struct dentry *dentry, struct inode *inode,
>  		goto out_err;
>  
>  	if (fc->cache_symlinks)
> -		return page_get_link(dentry, inode, callback);
> +		return page_get_link_raw(dentry, inode, callback);
>  
>  	err = -ECHILD;
>  	if (!dentry)
> diff --git a/fs/namei.c b/fs/namei.c
> index 3ab9440c5b93..ecb7b95c2ca3 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -5356,10 +5356,9 @@ const char *vfs_get_link(struct dentry *dentry, struct delayed_call *done)
>  EXPORT_SYMBOL(vfs_get_link);
>  
>  /* get the link contents into pagecache */
> -const char *page_get_link(struct dentry *dentry, struct inode *inode,
> -			  struct delayed_call *callback)
> +static char *__page_get_link(struct dentry *dentry, struct inode *inode,
> +			     struct delayed_call *callback)
>  {
> -	char *kaddr;
>  	struct page *page;
>  	struct address_space *mapping = inode->i_mapping;
>  
> @@ -5378,8 +5377,23 @@ const char *page_get_link(struct dentry *dentry, struct inode *inode,
>  	}
>  	set_delayed_call(callback, page_put_link, page);
>  	BUG_ON(mapping_gfp_mask(mapping) & __GFP_HIGHMEM);
> -	kaddr = page_address(page);
> -	nd_terminate_link(kaddr, inode->i_size, PAGE_SIZE - 1);
> +	return page_address(page);
> +}
> +
> +const char *page_get_link_raw(struct dentry *dentry, struct inode *inode,
> +			      struct delayed_call *callback)
> +{
> +	return __page_get_link(dentry, inode, callback);
> +}
> +EXPORT_SYMBOL_GPL(page_get_link_raw);
> +
> +const char *page_get_link(struct dentry *dentry, struct inode *inode,
> +					struct delayed_call *callback)
> +{
> +	char *kaddr = __page_get_link(dentry, inode, callback);
> +
> +	if (!IS_ERR(kaddr))
> +		nd_terminate_link(kaddr, inode->i_size, PAGE_SIZE - 1);
>  	return kaddr;
>  }
>  
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 2c3b2f8a621f..9346adf28f7b 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -3452,6 +3452,8 @@ extern const struct file_operations generic_ro_fops;
>  
>  extern int readlink_copy(char __user *, int, const char *, int);
>  extern int page_readlink(struct dentry *, char __user *, int);
> +extern const char *page_get_link_raw(struct dentry *, struct inode *,
> +				     struct delayed_call *);
>  extern const char *page_get_link(struct dentry *, struct inode *,
>  				 struct delayed_call *);
>  extern void page_put_link(void *);
> -- 
>
> 2.48.1
>
>