From 11f3710417d026ea2f4fcf362d866342c5274185 Mon Sep 17 00:00:00 2001 From: Miklos Szeredi Date: Mon, 21 Mar 2016 17:31:44 +0100 Subject: [PATCH 1/7] ovl: verify upper dentry before unlink and rename Unlink and rename in overlayfs checked the upper dentry for staleness by verifying upper->d_parent against upperdir. However the dentry can go stale also by being unhashed, for example. Expand the verification to actually look up the name again (under parent lock) and check if it matches the upper dentry. This matches what the VFS does before passing the dentry to filesytem's unlink/rename methods, which excludes any inconsistency caused by overlayfs. Signed-off-by: Miklos Szeredi --- fs/overlayfs/dir.c | 59 +++++++++++++++++++++++++++++----------------- 1 file changed, 38 insertions(+), 21 deletions(-) diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c index 52f6de5d40a9..bc1758a160f6 100644 --- a/fs/overlayfs/dir.c +++ b/fs/overlayfs/dir.c @@ -596,21 +596,25 @@ static int ovl_remove_upper(struct dentry *dentry, bool is_dir) { struct dentry *upperdir = ovl_dentry_upper(dentry->d_parent); struct inode *dir = upperdir->d_inode; - struct dentry *upper = ovl_dentry_upper(dentry); + struct dentry *upper; int err; inode_lock_nested(dir, I_MUTEX_PARENT); + upper = lookup_one_len(dentry->d_name.name, upperdir, + dentry->d_name.len); + err = PTR_ERR(upper); + if (IS_ERR(upper)) + goto out_unlock; + err = -ESTALE; - if (upper->d_parent == upperdir) { - /* Don't let d_delete() think it can reset d_inode */ - dget(upper); + if (upper == ovl_dentry_upper(dentry)) { if (is_dir) err = vfs_rmdir(dir, upper); else err = vfs_unlink(dir, upper, NULL); - dput(upper); ovl_dentry_version_inc(dentry->d_parent); } + dput(upper); /* * Keeping this dentry hashed would mean having to release @@ -620,6 +624,7 @@ static int ovl_remove_upper(struct dentry *dentry, bool is_dir) */ if (!err) d_drop(dentry); +out_unlock: inode_unlock(dir); return err; @@ -840,29 +845,39 @@ static int ovl_rename2(struct inode *olddir, struct dentry *old, trap = lock_rename(new_upperdir, old_upperdir); - olddentry = ovl_dentry_upper(old); - newdentry = ovl_dentry_upper(new); - if (newdentry) { + + olddentry = lookup_one_len(old->d_name.name, old_upperdir, + old->d_name.len); + err = PTR_ERR(olddentry); + if (IS_ERR(olddentry)) + goto out_unlock; + + err = -ESTALE; + if (olddentry != ovl_dentry_upper(old)) + goto out_dput_old; + + newdentry = lookup_one_len(new->d_name.name, new_upperdir, + new->d_name.len); + err = PTR_ERR(newdentry); + if (IS_ERR(newdentry)) + goto out_dput_old; + + err = -ESTALE; + if (ovl_dentry_upper(new)) { if (opaquedir) { - newdentry = opaquedir; - opaquedir = NULL; + if (newdentry != opaquedir) + goto out_dput; } else { - dget(newdentry); + if (newdentry != ovl_dentry_upper(new)) + goto out_dput; } } else { new_create = true; - newdentry = lookup_one_len(new->d_name.name, new_upperdir, - new->d_name.len); - err = PTR_ERR(newdentry); - if (IS_ERR(newdentry)) - goto out_unlock; + if (!d_is_negative(newdentry) && + (!new_opaque || !ovl_is_whiteout(newdentry))) + goto out_dput; } - err = -ESTALE; - if (olddentry->d_parent != old_upperdir) - goto out_dput; - if (newdentry->d_parent != new_upperdir) - goto out_dput; if (olddentry == trap) goto out_dput; if (newdentry == trap) @@ -925,6 +940,8 @@ static int ovl_rename2(struct inode *olddir, struct dentry *old, out_dput: dput(newdentry); +out_dput_old: + dput(olddentry); out_unlock: unlock_rename(new_upperdir, old_upperdir); out_revert_creds: From 07f2af7bfd247857b1bf16ae7f479b5b6f4ef305 Mon Sep 17 00:00:00 2001 From: Konstantin Khlebnikov Date: Mon, 29 Jun 2015 20:18:56 +0300 Subject: [PATCH 2/7] ovl: honor flag MS_SILENT at mount This patch hides error about missing lowerdir if MS_SILENT is set. We use mount(NULL, "/", "overlay", MS_SILENT, NULL) for testing support of overlayfs: syscall returns -ENODEV if it's not supported. Otherwise kernel automatically loads module and returns -EINVAL because lowerdir is missing. Signed-off-by: Konstantin Khlebnikov Signed-off-by: Miklos Szeredi --- fs/overlayfs/super.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c index 619ad4b016d2..6b0111ae2ceb 100644 --- a/fs/overlayfs/super.c +++ b/fs/overlayfs/super.c @@ -936,7 +936,8 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent) err = -EINVAL; if (!ufs->config.lowerdir) { - pr_err("overlayfs: missing 'lowerdir'\n"); + if (!silent) + pr_err("overlayfs: missing 'lowerdir'\n"); goto out_free_config; } From fb5bb2c3b73df060d588b6521de5ab03589283f7 Mon Sep 17 00:00:00 2001 From: David Howells Date: Tue, 7 Jul 2015 15:04:44 +0100 Subject: [PATCH 3/7] ovl: Warn on copy up if a process has a R/O fd open to the lower file Print a warning when overlayfs copies up a file if the process that triggered the copy up has a R/O fd open to the lower file being copied up. This can help catch applications that do things like the following: fd1 = open("foo", O_RDONLY); fd2 = open("foo", O_RDWR); where they expect fd1 and fd2 to refer to the same file - which will no longer be the case post-copy up. With this patch, the following commands: bash 5/mnt/a/foo128 assuming /mnt/a/foo128 to be an un-copied up file on an overlay will produce the following warning in the kernel log: overlayfs: Copying up foo129, but open R/O on fd 5 which will cease to be coherent [pid=3818 bash] This is enabled by setting: /sys/module/overlay/parameters/check_copy_up to 1. The warnings are ratelimited and are also limited to one warning per file - assuming the copy up completes in each case. Signed-off-by: David Howells Signed-off-by: Miklos Szeredi --- fs/overlayfs/copy_up.c | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c index d894e7cd9a86..959bdcf2f9e8 100644 --- a/fs/overlayfs/copy_up.c +++ b/fs/overlayfs/copy_up.c @@ -7,6 +7,7 @@ * the Free Software Foundation. */ +#include #include #include #include @@ -16,10 +17,41 @@ #include #include #include +#include +#include #include "overlayfs.h" #define OVL_COPY_UP_CHUNK_SIZE (1 << 20) +static bool __read_mostly ovl_check_copy_up; +module_param_named(check_copy_up, ovl_check_copy_up, bool, + S_IWUSR | S_IRUGO); +MODULE_PARM_DESC(ovl_check_copy_up, + "Warn on copy-up when causing process also has a R/O fd open"); + +static int ovl_check_fd(const void *data, struct file *f, unsigned int fd) +{ + const struct dentry *dentry = data; + + if (f->f_inode == d_inode(dentry)) + pr_warn_ratelimited("overlayfs: Warning: Copying up %pD, but open R/O on fd %u which will cease to be coherent [pid=%d %s]\n", + f, fd, current->pid, current->comm); + return 0; +} + +/* + * Check the fds open by this process and warn if something like the following + * scenario is about to occur: + * + * fd1 = open("foo", O_RDONLY); + * fd2 = open("foo", O_RDWR); + */ +static void ovl_do_check_copy_up(struct dentry *dentry) +{ + if (ovl_check_copy_up) + iterate_fd(current->files, 0, ovl_check_fd, dentry); +} + int ovl_copy_xattr(struct dentry *old, struct dentry *new) { ssize_t list_size, size, value_size = 0; @@ -309,6 +341,8 @@ int ovl_copy_up_one(struct dentry *parent, struct dentry *dentry, if (WARN_ON(!workdir)) return -EROFS; + ovl_do_check_copy_up(lowerpath->dentry); + ovl_path_upper(parent, &parentpath); upperdir = parentpath.dentry; From 45aebeaf4f67468f76bedf62923a576a519a9b68 Mon Sep 17 00:00:00 2001 From: Vivek Goyal Date: Mon, 22 Feb 2016 09:28:34 -0500 Subject: [PATCH 4/7] ovl: Ensure upper filesystem supports d_type In some instances xfs has been created with ftype=0 and there if a file on lower fs is removed, overlay leaves a whiteout in upper fs but that whiteout does not get filtered out and is visible to overlayfs users. And reason it does not get filtered out because upper filesystem does not report file type of whiteout as DT_CHR during iterate_dir(). So it seems to be a requirement that upper filesystem support d_type for overlayfs to work properly. Do this check during mount and fail if d_type is not supported. Suggested-by: Dave Chinner Signed-off-by: Vivek Goyal Signed-off-by: Miklos Szeredi --- fs/overlayfs/overlayfs.h | 1 + fs/overlayfs/readdir.c | 37 +++++++++++++++++++++++++++++++++++++ fs/overlayfs/super.c | 15 +++++++++++++++ 3 files changed, 53 insertions(+) diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h index 99b4168c36ff..6a7090f4a441 100644 --- a/fs/overlayfs/overlayfs.h +++ b/fs/overlayfs/overlayfs.h @@ -166,6 +166,7 @@ extern const struct file_operations ovl_dir_operations; int ovl_check_empty_dir(struct dentry *dentry, struct list_head *list); void ovl_cleanup_whiteouts(struct dentry *upper, struct list_head *list); void ovl_cache_free(struct list_head *list); +int ovl_check_d_type_supported(struct path *realpath); /* inode.c */ int ovl_setattr(struct dentry *dentry, struct iattr *attr); diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c index fdaf28f75e12..18d0a1625220 100644 --- a/fs/overlayfs/readdir.c +++ b/fs/overlayfs/readdir.c @@ -43,6 +43,7 @@ struct ovl_readdir_data { struct ovl_cache_entry *first_maybe_whiteout; int count; int err; + bool d_type_supported; }; struct ovl_dir_file { @@ -577,3 +578,39 @@ void ovl_cleanup_whiteouts(struct dentry *upper, struct list_head *list) } inode_unlock(upper->d_inode); } + +static int ovl_check_d_type(struct dir_context *ctx, const char *name, + int namelen, loff_t offset, u64 ino, + unsigned int d_type) +{ + struct ovl_readdir_data *rdd = + container_of(ctx, struct ovl_readdir_data, ctx); + + /* Even if d_type is not supported, DT_DIR is returned for . and .. */ + if (!strncmp(name, ".", namelen) || !strncmp(name, "..", namelen)) + return 0; + + if (d_type != DT_UNKNOWN) + rdd->d_type_supported = true; + + return 0; +} + +/* + * Returns 1 if d_type is supported, 0 not supported/unknown. Negative values + * if error is encountered. + */ +int ovl_check_d_type_supported(struct path *realpath) +{ + int err; + struct ovl_readdir_data rdd = { + .ctx.actor = ovl_check_d_type, + .d_type_supported = false, + }; + + err = ovl_dir_read(realpath, &rdd); + if (err) + return err; + + return rdd.d_type_supported; +} diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c index 6b0111ae2ceb..ef64984c9bbc 100644 --- a/fs/overlayfs/super.c +++ b/fs/overlayfs/super.c @@ -1029,6 +1029,21 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent) sb->s_flags |= MS_RDONLY; ufs->workdir = NULL; } + + /* + * Upper should support d_type, else whiteouts are visible. + * Given workdir and upper are on same fs, we can do + * iterate_dir() on workdir. + */ + err = ovl_check_d_type_supported(&workpath); + if (err < 0) + goto out_put_workdir; + + if (!err) { + pr_err("overlayfs: upper fs needs to support d_type.\n"); + err = -EINVAL; + goto out_put_workdir; + } } err = -ENOMEM; From f134f2446548267330f45b06f14d59aaf1641fdc Mon Sep 17 00:00:00 2001 From: Sohom Bhattacharjee Date: Tue, 15 Mar 2016 20:57:59 +0530 Subject: [PATCH 5/7] ovl: fixed coding style warning This patch fixes a newline warning found by the checkpatch.pl tool Signed-off-by: Sohom-Bhattacharjee Signed-off-by: Miklos Szeredi --- fs/overlayfs/copy_up.c | 1 + 1 file changed, 1 insertion(+) diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c index 959bdcf2f9e8..cc514da6f3e7 100644 --- a/fs/overlayfs/copy_up.c +++ b/fs/overlayfs/copy_up.c @@ -267,6 +267,7 @@ static int ovl_copy_up_locked(struct dentry *workdir, struct dentry *upperdir, if (S_ISREG(stat->mode)) { struct path upperpath; + ovl_path_upper(dentry, &upperpath); BUG_ON(upperpath.dentry != NULL); upperpath.dentry = newdentry; From 56656e960b555cb98bc414382566dcb59aae99a2 Mon Sep 17 00:00:00 2001 From: Miklos Szeredi Date: Mon, 21 Mar 2016 17:31:46 +0100 Subject: [PATCH 6/7] ovl: rename is_merge to is_lowest The 'is_merge' is an historical naming from when only a single lower layer could exist. With the introduction of multiple lower layers the meaning of this flag was changed to mean only the "lowest layer" (while all lower layers were being merged). So now 'is_merge' is inaccurate and hence renaming to 'is_lowest' Signed-off-by: Miklos Szeredi --- fs/overlayfs/readdir.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c index 18d0a1625220..6ec1e43a9a54 100644 --- a/fs/overlayfs/readdir.c +++ b/fs/overlayfs/readdir.c @@ -36,7 +36,7 @@ struct ovl_dir_cache { struct ovl_readdir_data { struct dir_context ctx; - bool is_merge; + bool is_lowest; struct rb_root root; struct list_head *list; struct list_head middle; @@ -140,9 +140,9 @@ static int ovl_cache_entry_add_rb(struct ovl_readdir_data *rdd, return 0; } -static int ovl_fill_lower(struct ovl_readdir_data *rdd, - const char *name, int namelen, - loff_t offset, u64 ino, unsigned int d_type) +static int ovl_fill_lowest(struct ovl_readdir_data *rdd, + const char *name, int namelen, + loff_t offset, u64 ino, unsigned int d_type) { struct ovl_cache_entry *p; @@ -194,10 +194,10 @@ static int ovl_fill_merge(struct dir_context *ctx, const char *name, container_of(ctx, struct ovl_readdir_data, ctx); rdd->count++; - if (!rdd->is_merge) + if (!rdd->is_lowest) return ovl_cache_entry_add_rb(rdd, name, namelen, ino, d_type); else - return ovl_fill_lower(rdd, name, namelen, offset, ino, d_type); + return ovl_fill_lowest(rdd, name, namelen, offset, ino, d_type); } static int ovl_check_whiteouts(struct dentry *dir, struct ovl_readdir_data *rdd) @@ -290,7 +290,7 @@ static int ovl_dir_read_merged(struct dentry *dentry, struct list_head *list) .ctx.actor = ovl_fill_merge, .list = list, .root = RB_ROOT, - .is_merge = false, + .is_lowest = false, }; int idx, next; @@ -307,7 +307,7 @@ static int ovl_dir_read_merged(struct dentry *dentry, struct list_head *list) * allows offsets to be reasonably constant */ list_add(&rdd.middle, rdd.list); - rdd.is_merge = true; + rdd.is_lowest = true; err = ovl_dir_read(&realpath, &rdd); list_del(&rdd.middle); } From 6986c012faa480fb0fda74eaae9abb22f7ad1004 Mon Sep 17 00:00:00 2001 From: Miklos Szeredi Date: Mon, 21 Mar 2016 17:31:46 +0100 Subject: [PATCH 7/7] ovl: cleanup unused var in rename2 Signed-off-by: Miklos Szeredi --- fs/overlayfs/dir.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c index bc1758a160f6..b3fc0a35bf62 100644 --- a/fs/overlayfs/dir.c +++ b/fs/overlayfs/dir.c @@ -719,7 +719,6 @@ static int ovl_rename2(struct inode *olddir, struct dentry *old, struct dentry *trap; bool old_opaque; bool new_opaque; - bool new_create = false; bool cleanup_whiteout = false; bool overwrite = !(flags & RENAME_EXCHANGE); bool is_dir = d_is_dir(old); @@ -872,7 +871,6 @@ static int ovl_rename2(struct inode *olddir, struct dentry *old, goto out_dput; } } else { - new_create = true; if (!d_is_negative(newdentry) && (!new_opaque || !ovl_is_whiteout(newdentry))) goto out_dput;