On Tue, Oct 24, 2017 at 01:02:58PM +0300, Amir Goldstein wrote:
> Introduce a new config option OVERLAY_FS_INDEX_INCOMPAT.
>
> If this config option is enabled then inodes index is declared
> an incompatible feature and kernel will refuse to mount an overlay
> with inodes index off when a non-empty index directory exists.
>
> Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
> ---
> fs/overlayfs/Kconfig | 8 ++++++++
> fs/overlayfs/dir.c | 30 ++++++++++++++++++++++++++++++
> fs/overlayfs/overlayfs.h | 3 ++-
> fs/overlayfs/super.c | 24 +++++++++++++++++++++++-
> 4 files changed, 63 insertions(+), 2 deletions(-)
>
> diff --git a/fs/overlayfs/Kconfig b/fs/overlayfs/Kconfig
> index cbfc196e5dc5..e5e6dec7d177 100644
> --- a/fs/overlayfs/Kconfig
> +++ b/fs/overlayfs/Kconfig
> @@ -43,3 +43,11 @@ config OVERLAY_FS_INDEX
> outcomes. However, mounting the same overlay with an old kernel
> read-write and then mounting it again with a new kernel, will have
> unexpected results.
> +
> +config OVERLAY_FS_INDEX_INCOMPAT
> + bool "Overlayfs: support incompatible index feature"
> + depends on OVERLAY_FS_INDEX
> + help
> + If this config option is enabled then inodes index is declared an
> + incompatible feature and kernel will refuse to mount an overlay with
> + inodes index off when a non-empty index directory exists.
Hi Amir,
I don't know much about the issues you have faced. So I have few very
basic questions.
So the problem you are trying to fix is that if somebody mounted overlay
with index=on and later they try to mount it with index=off.
What problems happen if we allow that? If its a problem, why not always
disallow that instead of making it an option. IOW, is there a use case
where we will still let user mount later with index=off.
Vivek
> diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
> index cc961a3bd3bd..7e2f748d5a7c 100644
> --- a/fs/overlayfs/dir.c
> +++ b/fs/overlayfs/dir.c
> @@ -8,6 +8,7 @@
> */
>
> #include <linux/fs.h>
> +#include <linux/mount.h>
> #include <linux/namei.h>
> #include <linux/xattr.h>
> #include <linux/security.h>
> @@ -43,6 +44,35 @@ int ovl_cleanup(struct inode *wdir, struct dentry *wdentry)
> return err;
> }
>
> +int ovl_cleanup_path(struct path *path, const char *name)
> +{
> + struct inode *dir = path->dentry->d_inode;
> + struct dentry *dentry;
> + int err;
> +
> + err = mnt_want_write(path->mnt);
> + if (err)
> + return err;
> +
> + inode_lock_nested(dir, I_MUTEX_PARENT);
> +
> + dentry = lookup_one_len(name, path->dentry, strlen(name));
> + if (IS_ERR(dentry)) {
> + err = PTR_ERR(dentry);
> + dentry = NULL;
> + } else if (!dentry->d_inode) {
> + err = -ENOENT;
> + } else {
> + err = ovl_cleanup(dir, dentry);
> + }
> +
> + dput(dentry);
> + inode_unlock(dir);
> + mnt_drop_write(path->mnt);
> +
> + return err;
> +}
> +
> struct dentry *ovl_lookup_temp(struct dentry *workdir)
> {
> struct dentry *temp;
> diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
> index d9a0edd4e57e..3f9a7625bded 100644
> --- a/fs/overlayfs/overlayfs.h
> +++ b/fs/overlayfs/overlayfs.h
> @@ -300,6 +300,8 @@ static inline void ovl_copyattr(struct inode *from, struct inode *to)
>
> /* dir.c */
> extern const struct inode_operations ovl_dir_inode_operations;
> +int ovl_cleanup(struct inode *dir, struct dentry *dentry);
> +int ovl_cleanup_path(struct path *path, const char *name);
> struct dentry *ovl_lookup_temp(struct dentry *workdir);
> struct cattr {
> dev_t rdev;
> @@ -309,7 +311,6 @@ struct cattr {
> int ovl_create_real(struct inode *dir, struct dentry *newdentry,
> struct cattr *attr,
> struct dentry *hardlink, bool debug);
> -int ovl_cleanup(struct inode *dir, struct dentry *dentry);
>
> /* copy_up.c */
> int ovl_copy_up(struct dentry *dentry);
> diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> index f5738e96a052..0dc6d61f828a 100644
> --- a/fs/overlayfs/super.c
> +++ b/fs/overlayfs/super.c
> @@ -39,6 +39,9 @@ module_param_named(index, ovl_index_def, bool, 0644);
> MODULE_PARM_DESC(ovl_index_def,
> "Default to on or off for the inodes index feature");
>
> +static const bool ovl_incompat_index =
> + IS_ENABLED(CONFIG_OVERLAY_FS_INDEX_INCOMPAT);
> +
> static void ovl_dentry_release(struct dentry *dentry)
> {
> struct ovl_entry *oe = dentry->d_fsdata;
> @@ -1060,7 +1063,15 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
> else if (ufs->upper_mnt->mnt_sb != ufs->same_sb)
> ufs->same_sb = NULL;
>
> - if (!(ovl_force_readonly(ufs)) && ufs->config.index) {
> + if (ovl_force_readonly(ufs)) {
> + /*
> + * Cannot enable index without upperdir/workdir and cannot
> + * test for incompat index dir, but cannot corrupt incompat
> + * index dir either.
> + */
> + ufs->config.index = false;
> + pr_warn("overlayfs: no upperdir/workdir, falling back to index=off.\n");
> + } else if (ufs->config.index) {
> /* Verify lower root is upper root origin */
> err = ovl_verify_origin(upperpath.dentry, ufs->lower_mnt[0],
> stack[0].dentry, false, true);
> @@ -1088,6 +1099,17 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
> pr_warn("overlayfs: try deleting index dir or mounting with '-o index=off' to disable inodes index.\n");
> if (err)
> goto out_put_indexdir;
> +
> + } else if (ovl_incompat_index) {
> + /*
> + * Try to remove empty index dir if it exists -
> + * failure means we need to abort the mount.
> + */
> + err = ovl_cleanup_path(&workpath, OVL_INDEXDIR_NAME);
> + if (err && err != -ENOENT) {
> + pr_err("overlayfs: incompatible index dir exists, try deleting index dir to disable inodes index.\n");
> + goto out_put_lower_mnt;
> + }
> }
>
> /* Show index=off/on in /proc/mounts for any of the reasons above */
> --
> 2.7.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-unionfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-unionfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
