From 32a3d848eb91a298334991f1891e12e0362f91db Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sun, 4 Dec 2016 17:33:17 +0000 Subject: [PATCH] ovl: clean up kstat usage FWIW, there's a bit of abuse of struct kstat in overlayfs object creation paths - for one thing, it ends up with a very small subset of struct kstat (mode + rdev), for another it also needs link in case of symlinks and ends up passing it separately. IMO it would be better to introduce a separate object for that. In principle, we might even lift that thing into general API and switch ->mkdir()/->mknod()/->symlink() to identical calling conventions. Hell knows, perhaps ->create() as well... Signed-off-by: Miklos Szeredi --- fs/overlayfs/copy_up.c | 12 +++++---- fs/overlayfs/dir.c | 56 ++++++++++++++++++++-------------------- fs/overlayfs/overlayfs.h | 7 ++++- fs/overlayfs/super.c | 9 +++---- 4 files changed, 45 insertions(+), 39 deletions(-) diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c index e8cacf7a8dcc..0e9940f9f34a 100644 --- a/fs/overlayfs/copy_up.c +++ b/fs/overlayfs/copy_up.c @@ -238,10 +238,15 @@ static int ovl_copy_up_locked(struct dentry *workdir, struct dentry *upperdir, struct inode *udir = upperdir->d_inode; struct dentry *newdentry = NULL; struct dentry *upper = NULL; - umode_t mode = stat->mode; int err; const struct cred *old_creds = NULL; struct cred *new_creds = NULL; + struct cattr cattr = { + /* Can't properly set mode on creation because of the umask */ + .mode = stat->mode & S_IFMT, + .rdev = stat->rdev, + .link = link + }; newdentry = ovl_lookup_temp(workdir, dentry); err = PTR_ERR(newdentry); @@ -261,10 +266,7 @@ static int ovl_copy_up_locked(struct dentry *workdir, struct dentry *upperdir, if (new_creds) old_creds = override_creds(new_creds); - /* Can't properly set mode on creation because of the umask */ - stat->mode &= S_IFMT; - err = ovl_create_real(wdir, newdentry, stat, link, NULL, true); - stat->mode = mode; + err = ovl_create_real(wdir, newdentry, &cattr, NULL, true); if (new_creds) { revert_creds(old_creds); diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c index 3cc65e60df02..16e06dd89457 100644 --- a/fs/overlayfs/dir.c +++ b/fs/overlayfs/dir.c @@ -82,8 +82,7 @@ static struct dentry *ovl_whiteout(struct dentry *workdir, } int ovl_create_real(struct inode *dir, struct dentry *newdentry, - struct kstat *stat, const char *link, - struct dentry *hardlink, bool debug) + struct cattr *attr, struct dentry *hardlink, bool debug) { int err; @@ -93,13 +92,13 @@ int ovl_create_real(struct inode *dir, struct dentry *newdentry, if (hardlink) { err = ovl_do_link(hardlink, dir, newdentry, debug); } else { - switch (stat->mode & S_IFMT) { + switch (attr->mode & S_IFMT) { case S_IFREG: - err = ovl_do_create(dir, newdentry, stat->mode, debug); + err = ovl_do_create(dir, newdentry, attr->mode, debug); break; case S_IFDIR: - err = ovl_do_mkdir(dir, newdentry, stat->mode, debug); + err = ovl_do_mkdir(dir, newdentry, attr->mode, debug); break; case S_IFCHR: @@ -107,11 +106,11 @@ int ovl_create_real(struct inode *dir, struct dentry *newdentry, case S_IFIFO: case S_IFSOCK: err = ovl_do_mknod(dir, newdentry, - stat->mode, stat->rdev, debug); + attr->mode, attr->rdev, debug); break; case S_IFLNK: - err = ovl_do_symlink(dir, newdentry, link, debug); + err = ovl_do_symlink(dir, newdentry, attr->link, debug); break; default: @@ -190,8 +189,7 @@ static bool ovl_type_merge(struct dentry *dentry) } static int ovl_create_upper(struct dentry *dentry, struct inode *inode, - struct kstat *stat, const char *link, - struct dentry *hardlink) + struct cattr *attr, struct dentry *hardlink) { struct dentry *upperdir = ovl_dentry_upper(dentry->d_parent); struct inode *udir = upperdir->d_inode; @@ -199,7 +197,7 @@ static int ovl_create_upper(struct dentry *dentry, struct inode *inode, int err; if (!hardlink && !IS_POSIXACL(udir)) - stat->mode &= ~current_umask(); + attr->mode &= ~current_umask(); inode_lock_nested(udir, I_MUTEX_PARENT); newdentry = lookup_one_len(dentry->d_name.name, upperdir, @@ -207,7 +205,7 @@ static int ovl_create_upper(struct dentry *dentry, struct inode *inode, err = PTR_ERR(newdentry); if (IS_ERR(newdentry)) goto out_unlock; - err = ovl_create_real(udir, newdentry, stat, link, hardlink, false); + err = ovl_create_real(udir, newdentry, attr, hardlink, false); if (err) goto out_dput; @@ -282,7 +280,8 @@ static struct dentry *ovl_clear_empty(struct dentry *dentry, if (IS_ERR(opaquedir)) goto out_unlock; - err = ovl_create_real(wdir, opaquedir, &stat, NULL, NULL, true); + err = ovl_create_real(wdir, opaquedir, + &(struct cattr){.mode = stat.mode}, NULL, true); if (err) goto out_dput; @@ -382,7 +381,7 @@ static int ovl_set_upper_acl(struct dentry *upperdentry, const char *name, } static int ovl_create_over_whiteout(struct dentry *dentry, struct inode *inode, - struct kstat *stat, const char *link, + struct cattr *cattr, struct dentry *hardlink) { struct dentry *workdir = ovl_workdir(dentry); @@ -399,7 +398,7 @@ static int ovl_create_over_whiteout(struct dentry *dentry, struct inode *inode, if (!hardlink) { err = posix_acl_create(dentry->d_parent->d_inode, - &stat->mode, &default_acl, &acl); + &cattr->mode, &default_acl, &acl); if (err) return err; } @@ -419,7 +418,7 @@ static int ovl_create_over_whiteout(struct dentry *dentry, struct inode *inode, if (IS_ERR(upper)) goto out_dput; - err = ovl_create_real(wdir, newdentry, stat, link, hardlink, true); + err = ovl_create_real(wdir, newdentry, cattr, hardlink, true); if (err) goto out_dput2; @@ -427,10 +426,11 @@ static int ovl_create_over_whiteout(struct dentry *dentry, struct inode *inode, * mode could have been mutilated due to umask (e.g. sgid directory) */ if (!hardlink && - !S_ISLNK(stat->mode) && newdentry->d_inode->i_mode != stat->mode) { + !S_ISLNK(cattr->mode) && + newdentry->d_inode->i_mode != cattr->mode) { struct iattr attr = { .ia_valid = ATTR_MODE, - .ia_mode = stat->mode, + .ia_mode = cattr->mode, }; inode_lock(newdentry->d_inode); err = notify_change(newdentry, &attr, NULL); @@ -450,7 +450,7 @@ static int ovl_create_over_whiteout(struct dentry *dentry, struct inode *inode, goto out_cleanup; } - if (!hardlink && S_ISDIR(stat->mode)) { + if (!hardlink && S_ISDIR(cattr->mode)) { err = ovl_set_opaque(dentry, newdentry); if (err) goto out_cleanup; @@ -487,8 +487,7 @@ static int ovl_create_over_whiteout(struct dentry *dentry, struct inode *inode, } static int ovl_create_or_link(struct dentry *dentry, struct inode *inode, - struct kstat *stat, const char *link, - struct dentry *hardlink) + struct cattr *attr, struct dentry *hardlink) { int err; const struct cred *old_cred; @@ -506,7 +505,7 @@ static int ovl_create_or_link(struct dentry *dentry, struct inode *inode, override_cred->fsgid = inode->i_gid; if (!hardlink) { err = security_dentry_create_files_as(dentry, - stat->mode, &dentry->d_name, old_cred, + attr->mode, &dentry->d_name, old_cred, override_cred); if (err) { put_cred(override_cred); @@ -517,11 +516,11 @@ static int ovl_create_or_link(struct dentry *dentry, struct inode *inode, put_cred(override_cred); if (!ovl_dentry_is_whiteout(dentry)) - err = ovl_create_upper(dentry, inode, stat, link, + err = ovl_create_upper(dentry, inode, attr, hardlink); else - err = ovl_create_over_whiteout(dentry, inode, stat, - link, hardlink); + err = ovl_create_over_whiteout(dentry, inode, attr, + hardlink); } out_revert_creds: revert_creds(old_cred); @@ -540,8 +539,9 @@ static int ovl_create_object(struct dentry *dentry, int mode, dev_t rdev, { int err; struct inode *inode; - struct kstat stat = { + struct cattr attr = { .rdev = rdev, + .link = link, }; err = ovl_want_write(dentry); @@ -554,9 +554,9 @@ static int ovl_create_object(struct dentry *dentry, int mode, dev_t rdev, goto out_drop_write; inode_init_owner(inode, dentry->d_parent->d_inode, mode); - stat.mode = inode->i_mode; + attr.mode = inode->i_mode; - err = ovl_create_or_link(dentry, inode, &stat, link, NULL); + err = ovl_create_or_link(dentry, inode, &attr, NULL); if (err) iput(inode); @@ -610,7 +610,7 @@ static int ovl_link(struct dentry *old, struct inode *newdir, inode = d_inode(old); ihold(inode); - err = ovl_create_or_link(new, inode, NULL, NULL, ovl_dentry_upper(old)); + err = ovl_create_or_link(new, inode, NULL, ovl_dentry_upper(old)); if (err) iput(inode); diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h index e07aa7b0ddb7..8af450b0e57a 100644 --- a/fs/overlayfs/overlayfs.h +++ b/fs/overlayfs/overlayfs.h @@ -212,8 +212,13 @@ static inline void ovl_copyattr(struct inode *from, struct inode *to) /* dir.c */ extern const struct inode_operations ovl_dir_inode_operations; struct dentry *ovl_lookup_temp(struct dentry *workdir, struct dentry *dentry); +struct cattr { + dev_t rdev; + umode_t mode; + const char *link; +}; int ovl_create_real(struct inode *dir, struct dentry *newdentry, - struct kstat *stat, const char *link, + struct cattr *attr, struct dentry *hardlink, bool debug); void ovl_cleanup(struct inode *dir, struct dentry *dentry); diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c index 4bd1e9c7246f..7da36ccda438 100644 --- a/fs/overlayfs/super.c +++ b/fs/overlayfs/super.c @@ -354,12 +354,9 @@ static struct dentry *ovl_workdir_create(struct vfsmount *mnt, strlen(OVL_WORKDIR_NAME)); if (!IS_ERR(work)) { - struct kstat stat = { - .mode = S_IFDIR | 0, - }; struct iattr attr = { .ia_valid = ATTR_MODE, - .ia_mode = stat.mode, + .ia_mode = S_IFDIR | 0, }; if (work->d_inode) { @@ -373,7 +370,9 @@ static struct dentry *ovl_workdir_create(struct vfsmount *mnt, goto retry; } - err = ovl_create_real(dir, work, &stat, NULL, NULL, true); + err = ovl_create_real(dir, work, + &(struct cattr){.mode = S_IFDIR | 0}, + NULL, true); if (err) goto out_dput; -- 2.45.2