On Thu, 15 Aug 2024 at 16:45, Laura Promberger <laura.promberger@xxxxxxx> wrote:
> - But for corrupted symlinks `fuse_change_attributes()` exits before `fuse_change_attributes_common()` is called and as such the length stays the old one.
The reason is that the attr_version check fails. The trace logs show
a zero attr_version value, which suggests that the check can not fail.
But we know that fuse_dentry_revalidate() supplies a non-zero
attr_version to fuse_change_attributes() and if there's a racing
fuse_reverse_inval_inode() which updates the fuse_inode's
attr_version, then it would result in fuse_change_attributes() exiting
before updating the cached attributes, which is what you observe.
This is probably 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 for cvmfs it causes problems.
My proposed solution would be 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.
Can you please try the attached patch?
Thanks,
Miklos
diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index 54104dd48af7..70fb57714f79 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -1632,7 +1632,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 4a4a22a08ac2..6795600c5738 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -5300,10 +5300,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;
@@ -5322,8 +5321,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 eae5b67e4a15..fc90d1f6e8c7 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -3316,6 +3316,8 @@ extern const struct file_operations generic_ro_fops;
extern int readlink_copy(char __user *, int, const char *);
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 *);
