ovl: initialize OVL_UPPERDATA in ovl_lookup()

Currently ovl_get_inode() initializes OVL_UPPERDATA flag and for that it
has to call ovl_check_metacopy_xattr() and check if metacopy xattr is
present or not.

yangerkun reported sometimes underlying filesystem might return -EIO and in
that case error handling path does not cleanup properly leading to various
warnings.

Run generic/461 with ext4 upper/lower layer sometimes may trigger the bug
as below(linux 4.19):

[  551.001349] overlayfs: failed to get metacopy (-5)
[  551.003464] overlayfs: failed to get inode (-5)
[  551.004243] overlayfs: cleanup of 'd44/fd51' failed (-5)
[  551.004941] overlayfs: failed to get origin (-5)
[  551.005199] ------------[ cut here ]------------
[  551.006697] WARNING: CPU: 3 PID: 24674 at fs/inode.c:1528 iput+0x33b/0x400
...
[  551.027219] Call Trace:
[  551.027623]  ovl_create_object+0x13f/0x170
[  551.028268]  ovl_create+0x27/0x30
[  551.028799]  path_openat+0x1a35/0x1ea0
[  551.029377]  do_filp_open+0xad/0x160
[  551.029944]  ? vfs_writev+0xe9/0x170
[  551.030499]  ? page_counter_try_charge+0x77/0x120
[  551.031245]  ? __alloc_fd+0x160/0x2a0
[  551.031832]  ? do_sys_open+0x189/0x340
[  551.032417]  ? get_unused_fd_flags+0x34/0x40
[  551.033081]  do_sys_open+0x189/0x340
[  551.033632]  __x64_sys_creat+0x24/0x30
[  551.034219]  do_syscall_64+0xd5/0x430
[  551.034800]  entry_SYSCALL_64_after_hwframe+0x44/0xa9

One solution is to improve error handling and call iget_failed() if error
is encountered.  Amir thinks that this path is little intricate and there
is not real need to check and initialize OVL_UPPERDATA in ovl_get_inode().
Instead caller of ovl_get_inode() can initialize this state.  And this will
avoid double checking of metacopy xattr lookup in ovl_lookup() and
ovl_get_inode().

OVL_UPPERDATA is inode flag.  So I was little concerned that initializing
it outside ovl_get_inode() might have some races.  But this is one way
transition.  That is once a file has been fully copied up, it can't go back
to metacopy file again.  And that seems to help avoid races.  So as of now
I can't see any races w.r.t OVL_UPPERDATA being set wrongly.  So move
settingof OVL_UPPERDATA inside the callers of ovl_get_inode().
ovl_obtain_alias() already does it.  So only two callers now left are
ovl_lookup() and ovl_instantiate().

Reported-by: yangerkun <yangerkun@huawei.com>
Suggested-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>

diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index 09faa63..1bba481 100644 (file)
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -286,6 +286,8 @@ static int ovl_instantiate(struct dentry *dentry, struct inode *inode,
                inode = ovl_get_inode(dentry->d_sb, &oip);
                if (IS_ERR(inode))
                        return PTR_ERR(inode);
+               if (inode == oip.newinode)
+                       ovl_set_flag(OVL_UPPERDATA, inode);
        } else {
                WARN_ON(ovl_inode_real(inode) != d_inode(newdentry));
                dput(newdentry);

diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index 981f11e..f2aaf00 100644 (file)
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -957,7 +957,7 @@ struct inode *ovl_get_inode(struct super_block *sb,
        bool bylower = ovl_hash_bylower(sb, upperdentry, lowerdentry,
                                        oip->index);
        int fsid = bylower ? lowerpath->layer->fsid : 0;
-       bool is_dir, metacopy = false;
+       bool is_dir;
        unsigned long ino = 0;
        int err = oip->newinode ? -EEXIST : -ENOMEM;
 
@@ -1018,15 +1018,6 @@ struct inode *ovl_get_inode(struct super_block *sb,
        if (oip->index)
                ovl_set_flag(OVL_INDEX, inode);
 
-       if (upperdentry) {
-               err = ovl_check_metacopy_xattr(upperdentry);
-               if (err < 0)
-                       goto out_err;
-               metacopy = err;
-               if (!metacopy)
-                       ovl_set_flag(OVL_UPPERDATA, inode);
-       }
-
        OVL_I(inode)->redirect = oip->redirect;
 
        if (bylower)

diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
index 59363c4..1687a77 100644 (file)
--- a/fs/overlayfs/namei.c
+++ b/fs/overlayfs/namei.c
@@ -1067,6 +1067,8 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
                err = PTR_ERR(inode);
                if (IS_ERR(inode))
                        goto out_free_oe;
+               if (upperdentry && !uppermetacopy)
+                       ovl_set_flag(OVL_UPPERDATA, inode);
        }
 
        ovl_dentry_update_reval(dentry, upperdentry,