Hi Miklos, On 9/25/24 14:20, Miklos Szeredi wrote: > 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. I'm a bit confused by this, especially due to "fuse_reverse_inval_inode()", isn't this about FUSE_NOTIFY_INVAL_ENTRY and the additional flag FUSE_EXPIRE_ONLY? I.e. the used code path is fuse_reverse_inval_entry()? And that path doesn't change the attr_version? Which I'm also confused about. > > 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. I wonder if we can just repeat operations if we detect changes in the middle. Hard started to work on a patch, but got distracted and I first would like to create a passthrough reproducer. Thanks, Bernd
