From: Vivek Goyal vgoyal@redhat.com
mainline inclusion from mainline-v5.8-rc1 commit 21d8d66abffb0c7834c1b09b8b043ea6a66d6089 category: bugfix bugzilla: NA CVE: NA
-------------------------------------------------
Amir pointed me to metacopy test cases in unionmount-testsuite and I decided to run "./run --ov=10 --meta" and it failed while running test "rename-mass-5.py".
Problem is w.r.t absolute redirect traversal on intermediate metacopy dentry. We do not store intermediate metacopy dentries and also skip current loop/layer and move onto lookup in next layer. But at the end of loop, we have logic to reset "poe" and layer index if currnently looked up dentry has absolute redirect. We skip all that and that means lookup in next layer will fail.
Following is simple test case to reproduce this.
- mkdir -p lower upper work merged lower/a lower/b - touch lower/a/foo.txt - mount -t overlay -o lowerdir=lower,upperdir=upper,workdir=work,metacopy=on none merged
- mv merged/a/foo.txt merged/b/bar.txt
- umount merged - mv upper lower2 - rm -rf work; mkdir -p upper work - mount -t overlay -o lowerdir=lower2:lower,upperdir=upper,workdir=work,metacopy=on none merged
- chown bin:bin merged/b/bar.txt
- umount merged - mv upper lower3 - rm -rf work; mkdir -p upper work - mount -t overlay -o lowerdir=lower3:lower2:lower,upperdir=upper,workdir=work,metacopy=on none merged
ls: cannot access 'bar.txt': Input/output error
Intermediate lower layer (lower2) has metacopy dentry b/bar.txt with absolute redirect "/a/foo.txt". We skipped redirect processing at the end of loop which sets poe to roe and sets the appropriate next lower layer index. And that means lookup failed in next layer.
Fix this by continuing the loop for any intermediate dentries. We still do not save these at lower stack. With this fix applied unionmount-testsuite, "./run --ov-10 --meta" now passes.
Signed-off-by: Vivek Goyal vgoyal@redhat.com Reviewed-by: Amir Goldstein amir73il@gmail.com Signed-off-by: Miklos Szeredi mszeredi@redhat.com Signed-off-by: Zheng Liang zhengliang6@huawei.com Reviewed-by: Zhang Yi yi.zhang@huawei.com Signed-off-by: Yang Yingliang yangyingliang@huawei.com --- fs/overlayfs/namei.c | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-)
diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c index 032765c03fa46..07a14fb850162 100644 --- a/fs/overlayfs/namei.c +++ b/fs/overlayfs/namei.c @@ -921,15 +921,6 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry, goto out_put; }
- /* - * Do not store intermediate metacopy dentries in chain, - * except top most lower metacopy dentry - */ - if (d.metacopy && ctr) { - dput(this); - continue; - } - /* * If no origin fh is stored in upper of a merge dir, store fh * of lower dir and set upper parent "impure". @@ -964,9 +955,20 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry, origin = this; }
- stack[ctr].dentry = this; - stack[ctr].layer = lower.layer; - ctr++; + if (d.metacopy && ctr) { + /* + * Do not store intermediate metacopy dentries in + * lower chain, except top most lower metacopy dentry. + * Continue the loop so that if there is an absolute + * redirect on this dentry, poe can be reset to roe. + */ + dput(this); + this = NULL; + } else { + stack[ctr].dentry = this; + stack[ctr].layer = lower.layer; + ctr++; + }
/* * Following redirects can have security consequences: it's like