From b10cdcdc2012b2c199077a0a648e8a7067e573bf Mon Sep 17 00:00:00 2001 From: Amir Goldstein Date: Mon, 8 Oct 2018 07:25:23 +0300 Subject: [PATCH] ovl: untangle copy up call chain In an attempt to dedup ~100 LOC, we ended up creating a tangled call chain, whose branches merge and diverge in several points according to the immutable c->tmpfile copy up mode. This call chain was hard to analyse for locking correctness because the locking requirements for the c->tmpfile flow were very different from the locking requirements for the !c->tmpfile flow (i.e. directory vs. regulare file copy up). Split the copy up helpers of the c->tmpfile flow from those of the !c->tmpfile (i.e. workdir) flow and remove the c->tmpfile mode from copy up context. Suggested-by: Al Viro Signed-off-by: Amir Goldstein Signed-off-by: Miklos Szeredi --- fs/overlayfs/copy_up.c | 245 ++++++++++++++++++++++++++--------------- 1 file changed, 159 insertions(+), 86 deletions(-) diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c index 989782a0c0c9..618cffc14f91 100644 --- a/fs/overlayfs/copy_up.c +++ b/fs/overlayfs/copy_up.c @@ -395,7 +395,6 @@ struct ovl_copy_up_ctx { struct dentry *destdir; struct qstr destname; struct dentry *workdir; - bool tmpfile; bool origin; bool indexed; bool metacopy; @@ -440,62 +439,6 @@ static int ovl_link_up(struct ovl_copy_up_ctx *c) return err; } -static int ovl_install_temp(struct ovl_copy_up_ctx *c, struct dentry *temp, - struct dentry **newdentry) -{ - int err; - struct dentry *upper; - struct inode *udir = d_inode(c->destdir); - - upper = lookup_one_len(c->destname.name, c->destdir, c->destname.len); - if (IS_ERR(upper)) - return PTR_ERR(upper); - - if (c->tmpfile) - err = ovl_do_link(temp, udir, upper); - else - err = ovl_do_rename(d_inode(c->workdir), temp, udir, upper, 0); - - if (!err) - *newdentry = dget(c->tmpfile ? upper : temp); - dput(upper); - - return err; -} - -static struct dentry *ovl_get_tmpfile(struct ovl_copy_up_ctx *c) -{ - int err; - struct dentry *temp; - const struct cred *old_creds = NULL; - struct cred *new_creds = NULL; - struct ovl_cattr cattr = { - /* Can't properly set mode on creation because of the umask */ - .mode = c->stat.mode & S_IFMT, - .rdev = c->stat.rdev, - .link = c->link - }; - - err = security_inode_copy_up(c->dentry, &new_creds); - if (err < 0) - return ERR_PTR(err); - - if (new_creds) - old_creds = override_creds(new_creds); - - if (c->tmpfile) - temp = ovl_do_tmpfile(c->workdir, c->stat.mode); - else - temp = ovl_create_temp(c->workdir, &cattr); - - if (new_creds) { - revert_creds(old_creds); - put_cred(new_creds); - } - - return temp; -} - static int ovl_copy_up_inode(struct ovl_copy_up_ctx *c, struct dentry *temp) { int err; @@ -547,37 +490,94 @@ static int ovl_copy_up_inode(struct ovl_copy_up_ctx *c, struct dentry *temp) return err; } -static int ovl_copy_up_locked(struct ovl_copy_up_ctx *c) +static struct dentry *ovl_get_workdir_temp(struct ovl_copy_up_ctx *c) +{ + int err; + struct dentry *temp; + const struct cred *old_creds = NULL; + struct cred *new_creds = NULL; + struct ovl_cattr cattr = { + /* Can't properly set mode on creation because of the umask */ + .mode = c->stat.mode & S_IFMT, + .rdev = c->stat.rdev, + .link = c->link + }; + + err = security_inode_copy_up(c->dentry, &new_creds); + if (err < 0) + return ERR_PTR(err); + + if (new_creds) + old_creds = override_creds(new_creds); + + temp = ovl_create_temp(c->workdir, &cattr); + + if (new_creds) { + revert_creds(old_creds); + put_cred(new_creds); + } + + return temp; +} + +/* + * Move temp file from workdir into place on upper dir. + * Used when copying up directories and when upper fs doesn't support O_TMPFILE. + * + * Caller must hold ovl_lock_rename_workdir(). + */ +static int ovl_rename_temp(struct ovl_copy_up_ctx *c, struct dentry *temp, + struct dentry **newdentry) +{ + int err; + struct dentry *upper; + struct inode *udir = d_inode(c->destdir); + + upper = lookup_one_len(c->destname.name, c->destdir, c->destname.len); + if (IS_ERR(upper)) + return PTR_ERR(upper); + + err = ovl_do_rename(d_inode(c->workdir), temp, udir, upper, 0); + if (!err) + *newdentry = dget(temp); + dput(upper); + + return err; +} + +/* + * Copyup using workdir to prepare temp file. Used when copying up directories, + * special files or when upper fs doesn't support O_TMPFILE. + */ +static int ovl_copy_up_workdir(struct ovl_copy_up_ctx *c) { - struct inode *udir = c->destdir->d_inode; struct inode *inode; struct dentry *newdentry = NULL; struct dentry *temp; int err; - temp = ovl_get_tmpfile(c); + err = ovl_lock_rename_workdir(c->workdir, c->destdir); + if (err) + return err; + + temp = ovl_get_workdir_temp(c); + err = PTR_ERR(temp); if (IS_ERR(temp)) - return PTR_ERR(temp); + goto unlock; err = ovl_copy_up_inode(c, temp); if (err) - goto out; + goto cleanup; if (S_ISDIR(c->stat.mode) && c->indexed) { err = ovl_create_index(c->dentry, c->lowerpath.dentry, temp); if (err) - goto out; + goto cleanup; } - if (c->tmpfile) { - inode_lock_nested(udir, I_MUTEX_PARENT); - err = ovl_install_temp(c, temp, &newdentry); - inode_unlock(udir); - } else { - err = ovl_install_temp(c, temp, &newdentry); - } + err = ovl_rename_temp(c, temp, &newdentry); if (err) - goto out; + goto cleanup; if (!c->metacopy) ovl_set_upperdata(d_inode(c->dentry)); @@ -585,13 +585,94 @@ static int ovl_copy_up_locked(struct ovl_copy_up_ctx *c) ovl_inode_update(inode, newdentry); if (S_ISDIR(inode->i_mode)) ovl_set_flag(OVL_WHITEOUTS, inode); +out_dput: + dput(temp); +unlock: + unlock_rename(c->workdir, c->destdir); + + return err; + +cleanup: + ovl_cleanup(d_inode(c->workdir), temp); + goto out_dput; +} + +static struct dentry *ovl_get_tmpfile(struct ovl_copy_up_ctx *c) +{ + int err; + struct dentry *temp; + const struct cred *old_creds = NULL; + struct cred *new_creds = NULL; + + err = security_inode_copy_up(c->dentry, &new_creds); + if (err < 0) + return ERR_PTR(err); + + if (new_creds) + old_creds = override_creds(new_creds); + + temp = ovl_do_tmpfile(c->workdir, c->stat.mode); + + if (new_creds) { + revert_creds(old_creds); + put_cred(new_creds); + } + + return temp; +} + +/* Link O_TMPFILE into place on upper dir */ +static int ovl_link_tmpfile(struct ovl_copy_up_ctx *c, struct dentry *temp, + struct dentry **newdentry) +{ + int err; + struct dentry *upper; + struct inode *udir = d_inode(c->destdir); + + inode_lock_nested(udir, I_MUTEX_PARENT); + + upper = lookup_one_len(c->destname.name, c->destdir, c->destname.len); + err = PTR_ERR(upper); + if (IS_ERR(upper)) + goto unlock; + + err = ovl_do_link(temp, udir, upper); + if (!err) + *newdentry = dget(upper); + dput(upper); + +unlock: + inode_unlock(udir); + + return err; +} + +/* Copyup using O_TMPFILE which does not require cross dir locking */ +static int ovl_copy_up_tmpfile(struct ovl_copy_up_ctx *c) +{ + struct dentry *newdentry = NULL; + struct dentry *temp; + int err; + + temp = ovl_get_tmpfile(c); + if (IS_ERR(temp)) + return PTR_ERR(temp); + + err = ovl_copy_up_inode(c, temp); + if (err) + goto out; + + err = ovl_link_tmpfile(c, temp, &newdentry); + if (err) + goto out; + + if (!c->metacopy) + ovl_set_upperdata(d_inode(c->dentry)); + ovl_inode_update(d_inode(c->dentry), newdentry); out: - if (err && !c->tmpfile) - ovl_cleanup(d_inode(c->workdir), temp); dput(temp); return err; - } /* @@ -645,18 +726,10 @@ static int ovl_do_copy_up(struct ovl_copy_up_ctx *c) } /* Should we copyup with O_TMPFILE or with workdir? */ - if (S_ISREG(c->stat.mode) && ofs->tmpfile) { - c->tmpfile = true; - err = ovl_copy_up_locked(c); - } else { - err = ovl_lock_rename_workdir(c->workdir, c->destdir); - if (!err) { - err = ovl_copy_up_locked(c); - unlock_rename(c->workdir, c->destdir); - } - } - - + if (S_ISREG(c->stat.mode) && ofs->tmpfile) + err = ovl_copy_up_tmpfile(c); + else + err = ovl_copy_up_workdir(c); if (err) goto out; -- 2.30.2