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@xxxxxxx>
> Tested-by: Sam Lewis <samclewis@xxxxxxxxxx>
> Signed-off-by: Miklos Szeredi <mszeredi@xxxxxxxxxx>
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@xxxxxxxxxx>
Tested-by: Luis Henriques <luis@xxxxxxxxxx>
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
>
>
