diff mbox series

[SRU,EOAN,2/2] UBUNTU: SAUCE: shiftfs: prevent ESTALE for LOOKUP_JUMP lookups

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

Commit Message

Christian Brauner June 24, 2020, 10:11 a.m. UTC
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(-)

Comments

Ian May June 24, 2020, 9:14 p.m. UTC | #1
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 mbox series

Patch

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;