Message ID | 20200624101156.993894-2-christian.brauner@ubuntu.com |
---|---|
State | New |
Headers | show |
Series | [SRU,EOAN,1/2] UBUNTU: SAUCE: Revert "UBUNTU: SAUCE: shiftfs: fix dentry revalidation" | expand |
On 6/24/20 5:11 AM, Christian Brauner wrote: > BugLink: https://bugs.launchpad.net/bugs/1872757 > > Users reported that creating temporary files shiftfs reports ESTALE. > This can be reproduced via: > > import tempfile > import os > > def test(): > with tempfile.TemporaryFile() as fd: > fd.write("data".encode('utf-8')) > # re-open the file to get a read-only file descriptor > return open(f"/proc/self/fd/{fd.fileno()}", "r") > > def main(): > fd = test() > fd.close() > > if __name__ == "__main__": > main() > > a similar issue was reported here: > https://github.com/systemd/systemd/issues/14861 > > Our revalidate methods were very opinionated about whether or not a > lower dentry was valid especially when it became unlinked we simply > invalidated the lower dentry which caused above bug to surface. This has > led to bugs where a ESTALE was returned for e.g. temporary files that > were created and directly re-opened afterwards through > /proc/<pid>/fd/<nr-of-deleted-file>. When a file is re-opened through > /proc/<pid>/fd/<nr> LOOKUP_JUMP is set and the vfs will revalidate via > d_weak_revalidate(). Since the file has been unhashed or even already > gone negative we'd fail the open when we should've succeeded. > > Reported-by: Christian Kellner <ckellner@redhat.com> > Reported-by: Evgeny Vereshchagin <evvers@ya.ru> > Cc: Seth Forshee <seth.forshee@canonical.com> > Link: https://github.com/systemd/systemd/issues/14861 > Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com> > --- > fs/shiftfs.c | 4 ---- > 1 file changed, 4 deletions(-) > > diff --git a/fs/shiftfs.c b/fs/shiftfs.c > index 5d88193b41db..6f40cb9272be 100644 > --- a/fs/shiftfs.c > +++ b/fs/shiftfs.c > @@ -251,8 +251,6 @@ static int shiftfs_d_weak_revalidate(struct dentry *dentry, unsigned int flags) > struct inode *loweri = d_inode(lowerd); > > shiftfs_copyattr(loweri, inode); > - if (!inode->i_nlink) > - err = 0; > } > > return err; > @@ -278,8 +276,6 @@ static int shiftfs_d_revalidate(struct dentry *dentry, unsigned int flags) > struct inode *loweri = d_inode(lowerd); > > shiftfs_copyattr(loweri, inode); > - if (!inode->i_nlink) > - err = 0; > } > > return err; LGTM Acked-by: Ian May <ian.may@canonical.com>
diff --git a/fs/shiftfs.c b/fs/shiftfs.c index 5d88193b41db..6f40cb9272be 100644 --- a/fs/shiftfs.c +++ b/fs/shiftfs.c @@ -251,8 +251,6 @@ static int shiftfs_d_weak_revalidate(struct dentry *dentry, unsigned int flags) struct inode *loweri = d_inode(lowerd); shiftfs_copyattr(loweri, inode); - if (!inode->i_nlink) - err = 0; } return err; @@ -278,8 +276,6 @@ static int shiftfs_d_revalidate(struct dentry *dentry, unsigned int flags) struct inode *loweri = d_inode(lowerd); shiftfs_copyattr(loweri, inode); - if (!inode->i_nlink) - err = 0; } return err;
BugLink: https://bugs.launchpad.net/bugs/1872757 Users reported that creating temporary files shiftfs reports ESTALE. This can be reproduced via: import tempfile import os def test(): with tempfile.TemporaryFile() as fd: fd.write("data".encode('utf-8')) # re-open the file to get a read-only file descriptor return open(f"/proc/self/fd/{fd.fileno()}", "r") def main(): fd = test() fd.close() if __name__ == "__main__": main() a similar issue was reported here: https://github.com/systemd/systemd/issues/14861 Our revalidate methods were very opinionated about whether or not a lower dentry was valid especially when it became unlinked we simply invalidated the lower dentry which caused above bug to surface. This has led to bugs where a ESTALE was returned for e.g. temporary files that were created and directly re-opened afterwards through /proc/<pid>/fd/<nr-of-deleted-file>. When a file is re-opened through /proc/<pid>/fd/<nr> LOOKUP_JUMP is set and the vfs will revalidate via d_weak_revalidate(). Since the file has been unhashed or even already gone negative we'd fail the open when we should've succeeded. Reported-by: Christian Kellner <ckellner@redhat.com> Reported-by: Evgeny Vereshchagin <evvers@ya.ru> Cc: Seth Forshee <seth.forshee@canonical.com> Link: https://github.com/systemd/systemd/issues/14861 Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com> --- fs/shiftfs.c | 4 ---- 1 file changed, 4 deletions(-)