* [RFC PATCH v2 1/9] ovl: store ovl upper root path in ovl_fs
2018-07-31 9:37 [RFC PATCH v2 0/9] ovl: add feature set support zhangyi (F)
@ 2018-07-31 9:37 ` zhangyi (F)
2018-08-01 6:48 ` Amir Goldstein
2018-07-31 9:37 ` [RFC PATCH v2 2/9] ovl: read feature set from each layer's root dir zhangyi (F)
` (7 subsequent siblings)
8 siblings, 1 reply; 26+ messages in thread
From: zhangyi (F) @ 2018-07-31 9:37 UTC (permalink / raw)
To: linux-unionfs; +Cc: miklos, amir73il, yi.zhang, miaoxie, yangerkun
Store upper layer's ovl_layer structure and root upper dentry into
ovl_fs sturcture for the upcoming "feature set" feature. "feature set"
feature will read upper layer's feature from upper root dentry and save
feature bit mask into upper layer's ovl_layer structure.
This patch relpace ->upper_mnt to ->upper_layer and add a helper func
ovl_upper_mnt() to get ->upper_mnt easily.
Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
---
fs/overlayfs/export.c | 12 +++++-------
fs/overlayfs/inode.c | 4 ++--
fs/overlayfs/namei.c | 2 +-
fs/overlayfs/ovl_entry.h | 9 ++++++++-
fs/overlayfs/readdir.c | 2 +-
fs/overlayfs/super.c | 39 ++++++++++++++++++++++++---------------
fs/overlayfs/util.c | 10 +++++-----
7 files changed, 46 insertions(+), 32 deletions(-)
diff --git a/fs/overlayfs/export.c b/fs/overlayfs/export.c
index 9941ece61a14..0a11bd46895a 100644
--- a/fs/overlayfs/export.c
+++ b/fs/overlayfs/export.c
@@ -207,7 +207,7 @@ static int ovl_check_encode_origin(struct dentry *dentry)
* ovl_connect_layer() will try to make origin's layer "connected" by
* copying up a "connectable" ancestor.
*/
- if (d_is_dir(dentry) && ofs->upper_mnt)
+ if (d_is_dir(dentry) && ofs->upper_layer)
return ovl_connect_layer(dentry);
/* Lower file handle for indexed and non-upper dir/non-dir */
@@ -434,7 +434,6 @@ static struct dentry *ovl_lookup_real_inode(struct super_block *sb,
struct ovl_layer *layer)
{
struct ovl_fs *ofs = sb->s_fs_info;
- struct ovl_layer upper_layer = { .mnt = ofs->upper_mnt };
struct dentry *index = NULL;
struct dentry *this = NULL;
struct inode *inode;
@@ -476,7 +475,7 @@ static struct dentry *ovl_lookup_real_inode(struct super_block *sb,
* recursive call walks back from indexed upper to the topmost
* connected/hashed upper parent (or up to root).
*/
- this = ovl_lookup_real(sb, upper, &upper_layer);
+ this = ovl_lookup_real(sb, upper, ofs->upper_layer);
dput(upper);
}
@@ -656,8 +655,7 @@ static struct dentry *ovl_get_dentry(struct super_block *sb,
struct dentry *index)
{
struct ovl_fs *ofs = sb->s_fs_info;
- struct ovl_layer upper_layer = { .mnt = ofs->upper_mnt };
- struct ovl_layer *layer = upper ? &upper_layer : lowerpath->layer;
+ struct ovl_layer *layer = upper ? ofs->upper_layer : lowerpath->layer;
struct dentry *real = upper ?: (index ?: lowerpath->dentry);
/*
@@ -685,10 +683,10 @@ static struct dentry *ovl_upper_fh_to_d(struct super_block *sb,
struct dentry *dentry;
struct dentry *upper;
- if (!ofs->upper_mnt)
+ if (!ofs->upper_layer)
return ERR_PTR(-EACCES);
- upper = ovl_decode_real_fh(fh, ofs->upper_mnt, true);
+ upper = ovl_decode_real_fh(fh, ovl_upper_mnt(ofs), true);
if (IS_ERR_OR_NULL(upper))
return upper;
diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index ed16a898caeb..0b6ecf26c066 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -421,7 +421,7 @@ int ovl_update_time(struct inode *inode, struct timespec64 *ts, int flags)
if (flags & S_ATIME) {
struct ovl_fs *ofs = inode->i_sb->s_fs_info;
struct path upperpath = {
- .mnt = ofs->upper_mnt,
+ .mnt = ovl_upper_mnt(ofs),
.dentry = ovl_upperdentry_dereference(OVL_I(inode)),
};
@@ -733,7 +733,7 @@ static bool ovl_hash_bylower(struct super_block *sb, struct dentry *upper,
return true;
/* Yes, if won't be copied up */
- if (!ofs->upper_mnt)
+ if (!ofs->upper_layer)
return true;
/* No, if lower hardlink is or will be broken on copy up */
diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
index c993dd8db739..bb2d347c0a7e 100644
--- a/fs/overlayfs/namei.c
+++ b/fs/overlayfs/namei.c
@@ -475,7 +475,7 @@ struct dentry *ovl_index_upper(struct ovl_fs *ofs, struct dentry *index)
if (IS_ERR_OR_NULL(fh))
return ERR_CAST(fh);
- upper = ovl_decode_real_fh(fh, ofs->upper_mnt, true);
+ upper = ovl_decode_real_fh(fh, ovl_upper_mnt(ofs), true);
kfree(fh);
if (IS_ERR_OR_NULL(upper))
diff --git a/fs/overlayfs/ovl_entry.h b/fs/overlayfs/ovl_entry.h
index 41655a7d6894..38ff6689a293 100644
--- a/fs/overlayfs/ovl_entry.h
+++ b/fs/overlayfs/ovl_entry.h
@@ -28,6 +28,7 @@ struct ovl_sb {
struct ovl_layer {
struct vfsmount *mnt;
+ /* Private fs info of this layer (upper fs == NULL) */
struct ovl_sb *fs;
/* Index of this layer in fs root (upper idx == 0) */
int idx;
@@ -42,7 +43,8 @@ struct ovl_path {
/* private information held for overlayfs's superblock */
struct ovl_fs {
- struct vfsmount *upper_mnt;
+ struct ovl_layer *upper_layer;
+ struct dentry *upperdir;
unsigned int numlower;
/* Number of unique lower sb that differ from upper sb */
unsigned int numlowerfs;
@@ -109,3 +111,8 @@ static inline struct dentry *ovl_upperdentry_dereference(struct ovl_inode *oi)
{
return READ_ONCE(oi->__upperdentry);
}
+
+static inline struct vfsmount *ovl_upper_mnt(struct ovl_fs *ofs)
+{
+ return ofs->upper_layer ? ofs->upper_layer->mnt : NULL;
+}
diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c
index ef1fe42ff7bb..1b70ffe4a633 100644
--- a/fs/overlayfs/readdir.c
+++ b/fs/overlayfs/readdir.c
@@ -1069,7 +1069,7 @@ int ovl_indexdir_cleanup(struct ovl_fs *ofs)
struct dentry *indexdir = ofs->indexdir;
struct dentry *index = NULL;
struct inode *dir = indexdir->d_inode;
- struct path path = { .mnt = ofs->upper_mnt, .dentry = indexdir };
+ struct path path = { .mnt = ovl_upper_mnt(ofs), .dentry = indexdir };
LIST_HEAD(list);
struct rb_root root = RB_ROOT;
struct ovl_cache_entry *p;
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index 704b37311467..cce4f58458dc 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -240,8 +240,10 @@ static void ovl_free_fs(struct ovl_fs *ofs)
ovl_inuse_unlock(ofs->workbasedir);
dput(ofs->workbasedir);
if (ofs->upperdir_locked)
- ovl_inuse_unlock(ofs->upper_mnt->mnt_root);
- mntput(ofs->upper_mnt);
+ ovl_inuse_unlock(ovl_upper_mnt(ofs)->mnt_root);
+ dput(ofs->upperdir);
+ mntput(ovl_upper_mnt(ofs));
+ kfree(ofs->upper_layer);
for (i = 0; i < ofs->numlower; i++)
mntput(ofs->lower_layers[i].mnt);
for (i = 0; i < ofs->numlowerfs; i++)
@@ -272,7 +274,7 @@ static int ovl_sync_fs(struct super_block *sb, int wait)
struct super_block *upper_sb;
int ret;
- if (!ofs->upper_mnt)
+ if (!ofs->upper_layer)
return 0;
/*
@@ -286,7 +288,7 @@ static int ovl_sync_fs(struct super_block *sb, int wait)
if (!wait)
return 0;
- upper_sb = ofs->upper_mnt->mnt_sb;
+ upper_sb = ovl_upper_mnt(ofs)->mnt_sb;
down_read(&upper_sb->s_umount);
ret = sync_filesystem(upper_sb);
@@ -324,7 +326,7 @@ static int ovl_statfs(struct dentry *dentry, struct kstatfs *buf)
/* Will this overlay be forced to mount/remount ro? */
static bool ovl_force_readonly(struct ovl_fs *ofs)
{
- return (!ofs->upper_mnt || !ofs->workdir);
+ return (!ofs->upper_layer || !ofs->workdir);
}
static const char *ovl_redirect_mode_def(void)
@@ -579,7 +581,7 @@ static struct dentry *ovl_workdir_create(struct ovl_fs *ofs,
const char *name, bool persist)
{
struct inode *dir = ofs->workbasedir->d_inode;
- struct vfsmount *mnt = ofs->upper_mnt;
+ struct vfsmount *mnt = ovl_upper_mnt(ofs);
struct dentry *work;
int err;
bool retried = false;
@@ -989,7 +991,14 @@ static int ovl_get_upper(struct ovl_fs *ofs, struct path *upperpath)
/* Don't inherit atime flags */
upper_mnt->mnt_flags &= ~(MNT_NOATIME | MNT_NODIRATIME | MNT_RELATIME);
- ofs->upper_mnt = upper_mnt;
+
+ err = -ENOMEM;
+ ofs->upper_layer = kzalloc(sizeof(struct ovl_layer), GFP_KERNEL);
+ if (ofs->upper_layer == NULL)
+ goto out;
+
+ ofs->upper_layer->mnt = upper_mnt;
+ ofs->upperdir = dget(upperpath->dentry);
err = 0;
out:
return err;
@@ -997,7 +1006,7 @@ static int ovl_get_upper(struct ovl_fs *ofs, struct path *upperpath)
static int ovl_make_workdir(struct ovl_fs *ofs, struct path *workpath)
{
- struct vfsmount *mnt = ofs->upper_mnt;
+ struct vfsmount *mnt = ovl_upper_mnt(ofs);
struct dentry *temp;
int fh_type;
int err;
@@ -1114,7 +1123,7 @@ static int ovl_get_workdir(struct ovl_fs *ofs, struct path *upperpath)
static int ovl_get_indexdir(struct ovl_fs *ofs, struct ovl_entry *oe,
struct path *upperpath)
{
- struct vfsmount *mnt = ofs->upper_mnt;
+ struct vfsmount *mnt = ovl_upper_mnt(ofs);
int err;
err = mnt_want_write(mnt);
@@ -1169,7 +1178,7 @@ static int ovl_get_fsid(struct ovl_fs *ofs, struct super_block *sb)
int err;
/* fsid 0 is reserved for upper fs even with non upper overlay */
- if (ofs->upper_mnt && ofs->upper_mnt->mnt_sb == sb)
+ if (ofs->upper_layer && ovl_upper_mnt(ofs)->mnt_sb == sb)
return 0;
for (i = 0; i < ofs->numlowerfs; i++) {
@@ -1246,7 +1255,7 @@ static int ovl_get_lower_layers(struct ovl_fs *ofs, struct path *stack,
* bits reserved for fsid, it emits a warning and uses the original
* inode number.
*/
- if (!ofs->numlowerfs || (ofs->numlowerfs == 1 && !ofs->upper_mnt)) {
+ if (!ofs->numlowerfs || (ofs->numlowerfs == 1 && !ofs->upper_layer)) {
ofs->xino_bits = 0;
ofs->config.xino = OVL_XINO_OFF;
} else if (ofs->config.xino == OVL_XINO_ON && !ofs->xino_bits) {
@@ -1409,8 +1418,8 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
if (!ofs->workdir)
sb->s_flags |= SB_RDONLY;
- sb->s_stack_depth = ofs->upper_mnt->mnt_sb->s_stack_depth;
- sb->s_time_gran = ofs->upper_mnt->mnt_sb->s_time_gran;
+ sb->s_stack_depth = ovl_upper_mnt(ofs)->mnt_sb->s_stack_depth;
+ sb->s_time_gran = ovl_upper_mnt(ofs)->mnt_sb->s_time_gran;
}
oe = ovl_get_lowerstack(sb, ofs);
@@ -1419,7 +1428,7 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
goto out_err;
/* If the upper fs is nonexistent, we mark overlayfs r/o too */
- if (!ofs->upper_mnt)
+ if (!ofs->upper_layer)
sb->s_flags |= SB_RDONLY;
if (!(ovl_force_readonly(ofs)) && ofs->config.index) {
@@ -1439,7 +1448,7 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
/* Show index=off in /proc/mounts for forced r/o mount */
if (!ofs->indexdir) {
ofs->config.index = false;
- if (ofs->upper_mnt && ofs->config.nfs_export) {
+ if (ofs->upper_layer && ofs->config.nfs_export) {
pr_warn("overlayfs: NFS export requires an index dir, falling back to nfs_export=off.\n");
ofs->config.nfs_export = false;
}
diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
index 6f1078028c66..6499f3b6fc5a 100644
--- a/fs/overlayfs/util.c
+++ b/fs/overlayfs/util.c
@@ -21,13 +21,13 @@
int ovl_want_write(struct dentry *dentry)
{
struct ovl_fs *ofs = dentry->d_sb->s_fs_info;
- return mnt_want_write(ofs->upper_mnt);
+ return mnt_want_write(ovl_upper_mnt(ofs));
}
void ovl_drop_write(struct dentry *dentry)
{
struct ovl_fs *ofs = dentry->d_sb->s_fs_info;
- mnt_drop_write(ofs->upper_mnt);
+ mnt_drop_write(ovl_upper_mnt(ofs));
}
struct dentry *ovl_workdir(struct dentry *dentry)
@@ -48,8 +48,8 @@ struct super_block *ovl_same_sb(struct super_block *sb)
struct ovl_fs *ofs = sb->s_fs_info;
if (!ofs->numlowerfs)
- return ofs->upper_mnt->mnt_sb;
- else if (ofs->numlowerfs == 1 && !ofs->upper_mnt)
+ return ovl_upper_mnt(ofs)->mnt_sb;
+ else if (ofs->numlowerfs == 1 && !ofs->upper_layer)
return ofs->lower_fs[0].sb;
else
return NULL;
@@ -148,7 +148,7 @@ void ovl_path_upper(struct dentry *dentry, struct path *path)
{
struct ovl_fs *ofs = dentry->d_sb->s_fs_info;
- path->mnt = ofs->upper_mnt;
+ path->mnt = ovl_upper_mnt(ofs);
path->dentry = ovl_dentry_upper(dentry);
}
--
2.13.6
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [RFC PATCH v2 1/9] ovl: store ovl upper root path in ovl_fs
2018-07-31 9:37 ` [RFC PATCH v2 1/9] ovl: store ovl upper root path in ovl_fs zhangyi (F)
@ 2018-08-01 6:48 ` Amir Goldstein
0 siblings, 0 replies; 26+ messages in thread
From: Amir Goldstein @ 2018-08-01 6:48 UTC (permalink / raw)
To: zhangyi (F); +Cc: overlayfs, Miklos Szeredi, Miao Xie, yangerkun
On Tue, Jul 31, 2018 at 12:37 PM, zhangyi (F) <yi.zhang@huawei.com> wrote:
> Store upper layer's ovl_layer structure and root upper dentry into
> ovl_fs sturcture for the upcoming "feature set" feature. "feature set"
> feature will read upper layer's feature from upper root dentry and save
> feature bit mask into upper layer's ovl_layer structure.
>
> This patch relpace ->upper_mnt to ->upper_layer and add a helper func
> ovl_upper_mnt() to get ->upper_mnt easily.
>
> Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
> ---
Looks good.
One minor fix.
> @@ -989,7 +991,14 @@ static int ovl_get_upper(struct ovl_fs *ofs, struct path *upperpath)
>
> /* Don't inherit atime flags */
> upper_mnt->mnt_flags &= ~(MNT_NOATIME | MNT_NODIRATIME | MNT_RELATIME);
> - ofs->upper_mnt = upper_mnt;
> +
> + err = -ENOMEM;
> + ofs->upper_layer = kzalloc(sizeof(struct ovl_layer), GFP_KERNEL);
> + if (ofs->upper_layer == NULL)
> + goto out;
> +
> + ofs->upper_layer->mnt = upper_mnt;
> + ofs->upperdir = dget(upperpath->dentry);
> err = 0;
> out:
You need to dput(upper_mnt) in case kzalloc fails. Probably better
to zalloc into local upper_layer var before clone_private_mount(),
kree(upper_layer) on goto out and return 0 instead of fallthough
to out: label on success.
Thanks,
Amir.
^ permalink raw reply [flat|nested] 26+ messages in thread
* [RFC PATCH v2 2/9] ovl: read feature set from each layer's root dir
2018-07-31 9:37 [RFC PATCH v2 0/9] ovl: add feature set support zhangyi (F)
2018-07-31 9:37 ` [RFC PATCH v2 1/9] ovl: store ovl upper root path in ovl_fs zhangyi (F)
@ 2018-07-31 9:37 ` zhangyi (F)
2018-08-01 7:44 ` Amir Goldstein
2018-07-31 9:37 ` [RFC PATCH v2 3/9] ovl: check file system features can be mounted properly zhangyi (F)
` (6 subsequent siblings)
8 siblings, 1 reply; 26+ messages in thread
From: zhangyi (F) @ 2018-07-31 9:37 UTC (permalink / raw)
To: linux-unionfs; +Cc: miklos, amir73il, yi.zhang, miaoxie, yangerkun
In order to distinguish the backward compatible and backward
incompatible features in overlay filesystem, we introduce compatible,
read-only compatible and incompatible these three kinds of features,
store them as __be64 bit mask on each layer's root directory via
"trusted.overlay.feature" xattr, which contains the features of this
layer.
This patch just read feature bitsets from each layer, doesn't add any
already known features and doesn't yet use the feature bitsets.
Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
Suggested-by: Miklos Szeredi <miklos@szeredi.hu>
Suggested-by: Amir Goldstein <amir73il@gmail.com>
---
fs/overlayfs/Makefile | 2 +-
fs/overlayfs/feature.c | 119 +++++++++++++++++++++++++++++++++++++++++++++++
fs/overlayfs/overlayfs.h | 15 ++++++
fs/overlayfs/ovl_entry.h | 5 ++
fs/overlayfs/super.c | 5 ++
5 files changed, 145 insertions(+), 1 deletion(-)
create mode 100644 fs/overlayfs/feature.c
diff --git a/fs/overlayfs/Makefile b/fs/overlayfs/Makefile
index 30802347a020..219c927069d2 100644
--- a/fs/overlayfs/Makefile
+++ b/fs/overlayfs/Makefile
@@ -5,4 +5,4 @@
obj-$(CONFIG_OVERLAY_FS) += overlay.o
overlay-objs := super.o namei.o util.o inode.o dir.o readdir.o copy_up.o \
- export.o
+ export.o feature.o
diff --git a/fs/overlayfs/feature.c b/fs/overlayfs/feature.c
new file mode 100644
index 000000000000..eed6644a2a31
--- /dev/null
+++ b/fs/overlayfs/feature.c
@@ -0,0 +1,119 @@
+/*
+ * Overlayfs feature sets support.
+ *
+ * Copyright (C) 2018 Huawei. All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ */
+
+#include <linux/fs.h>
+#include <linux/xattr.h>
+#include "overlayfs.h"
+
+/*
+ * Get overlay features from the layer's root dir.
+ *
+ * @realroot: real root dir dentry
+ *
+ * Return on-disk overlay feature struct if success, NULL if no feature
+ * and error ptr if error.
+ */
+static struct ovl_d_feature *ovl_get_feature(struct dentry *realroot)
+{
+ struct ovl_d_feature *odf = NULL;
+ int res;
+
+ res = vfs_getxattr(realroot, OVL_XATTR_FEATURE, NULL, 0);
+ if (res <= 0) {
+ if (res == -EOPNOTSUPP || res == -ENODATA)
+ return NULL;
+ goto fail;
+ }
+
+ odf = kzalloc(res, GFP_KERNEL);
+ if (!odf)
+ return ERR_PTR(-ENOMEM);
+
+ res = vfs_getxattr(realroot, OVL_XATTR_FEATURE, odf, res);
+ if (res < 0)
+ goto fail;
+
+ /* Check validity */
+ if (res < sizeof(struct ovl_d_feature)) {
+ res = -EINVAL;
+ goto invalid;
+ }
+ if (odf->magic != OVL_FEATURE_MAGIC) {
+ res = -EINVAL;
+ goto invalid;
+ }
+
+ return odf;
+out:
+ kfree(odf);
+ return ERR_PTR(res);
+fail:
+ pr_err("overlayfs: failed to get features (%i)\n", res);
+ goto out;
+invalid:
+ pr_err("overlayfs: invalid features (%*phN)\n", res, odf);
+ goto out;
+}
+
+/*
+ * Get features from each underlying root dir.
+ *
+ * @ofs: overlay filesystem information
+ * @oe: overlay lower stack
+ *
+ * Return 0 if success, errno otherwise.
+ */
+int ovl_init_feature_set(struct ovl_fs *ofs, struct ovl_entry *oe)
+{
+ struct ovl_layer *upper_layer = ofs->upper_layer;
+ struct dentry *upper = ofs->upperdir;
+ struct ovl_d_feature *odf;
+ int i;
+
+ /* Get features from the upper root dir */
+ if (upper_layer) {
+ odf = ovl_get_feature(upper);
+ if (IS_ERR(odf))
+ return PTR_ERR(odf);
+
+ if (odf) {
+ upper_layer->compat = be64_to_cpu(odf->compat);
+ upper_layer->ro_compat = be64_to_cpu(odf->ro_compat);
+ upper_layer->incompat = be64_to_cpu(odf->incompat);
+ upper_layer->feature = true;
+ kfree(odf);
+ } else {
+ upper_layer->feature = false;
+ }
+ }
+
+ /* Get features from each lower's root dir */
+ for (i = 0; i < oe->numlower; i++) {
+ struct ovl_path *lowerpath = &oe->lowerstack[i];
+ struct ovl_layer *lower_layer = &ofs->lower_layers[i];
+
+ odf = ovl_get_feature(lowerpath->dentry);
+ if (IS_ERR(odf))
+ return PTR_ERR(odf);
+
+ if (!odf) {
+ lower_layer->feature = false;
+ continue;
+ }
+
+ lower_layer->compat = be64_to_cpu(odf->compat);
+ lower_layer->ro_compat = be64_to_cpu(odf->ro_compat);
+ lower_layer->incompat = be64_to_cpu(odf->incompat);
+ lower_layer->feature = true;
+ kfree(odf);
+ }
+
+ return 0;
+}
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index 7538b9b56237..745f3b89a99e 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -28,6 +28,7 @@ enum ovl_path_type {
#define OVL_XATTR_IMPURE OVL_XATTR_PREFIX "impure"
#define OVL_XATTR_NLINK OVL_XATTR_PREFIX "nlink"
#define OVL_XATTR_UPPER OVL_XATTR_PREFIX "upper"
+#define OVL_XATTR_FEATURE OVL_XATTR_PREFIX "feature"
enum ovl_inode_flag {
/* Pure upper dir that may contain non pure upper entries */
@@ -43,6 +44,17 @@ enum ovl_entry_flag {
OVL_E_CONNECTED,
};
+/* Features */
+#define OVL_FEATURE_MAGIC 0xfe
+
+/* On-disk format of overlay layer features */
+struct ovl_d_feature {
+ u8 magic; /* 0xfe */
+ __be64 compat; /* compatible features */
+ __be64 ro_compat; /* read-only compatible features */
+ __be64 incompat; /* incompatible features */
+} __packed;
+
/*
* The tuple (fh,uuid) is a universal unique identifier for a copy up origin,
* where:
@@ -379,3 +391,6 @@ int ovl_set_origin(struct dentry *dentry, struct dentry *lower,
/* export.c */
extern const struct export_operations ovl_export_operations;
+
+/* feature.c */
+int ovl_init_feature_set(struct ovl_fs *ofs, struct ovl_entry *oe);
diff --git a/fs/overlayfs/ovl_entry.h b/fs/overlayfs/ovl_entry.h
index 38ff6689a293..b1b6627f3350 100644
--- a/fs/overlayfs/ovl_entry.h
+++ b/fs/overlayfs/ovl_entry.h
@@ -34,6 +34,11 @@ struct ovl_layer {
int idx;
/* One fsid per unique underlying sb (upper fsid == 0) */
int fsid;
+ /* Features of this layer */
+ u64 compat;
+ u64 ro_compat;
+ u64 incompat;
+ bool feature;
};
struct ovl_path {
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index cce4f58458dc..59b7d4f07b03 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -1427,6 +1427,11 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
if (IS_ERR(oe))
goto out_err;
+ /* Get features from each underlying layer's root dir */
+ err = ovl_init_feature_set(ofs, oe);
+ if (err)
+ goto out_free_oe;
+
/* If the upper fs is nonexistent, we mark overlayfs r/o too */
if (!ofs->upper_layer)
sb->s_flags |= SB_RDONLY;
--
2.13.6
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [RFC PATCH v2 2/9] ovl: read feature set from each layer's root dir
2018-07-31 9:37 ` [RFC PATCH v2 2/9] ovl: read feature set from each layer's root dir zhangyi (F)
@ 2018-08-01 7:44 ` Amir Goldstein
2018-08-03 12:11 ` zhangyi (F)
0 siblings, 1 reply; 26+ messages in thread
From: Amir Goldstein @ 2018-08-01 7:44 UTC (permalink / raw)
To: zhangyi (F); +Cc: overlayfs, Miklos Szeredi, Miao Xie, yangerkun
On Tue, Jul 31, 2018 at 12:37 PM, zhangyi (F) <yi.zhang@huawei.com> wrote:
> In order to distinguish the backward compatible and backward
> incompatible features in overlay filesystem, we introduce compatible,
> read-only compatible and incompatible these three kinds of features,
> store them as __be64 bit mask on each layer's root directory via
> "trusted.overlay.feature" xattr, which contains the features of this
> layer.
>
> This patch just read feature bitsets from each layer, doesn't add any
> already known features and doesn't yet use the feature bitsets.
>
> Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
> Suggested-by: Miklos Szeredi <miklos@szeredi.hu>
> Suggested-by: Amir Goldstein <amir73il@gmail.com>
> ---
> fs/overlayfs/Makefile | 2 +-
> fs/overlayfs/feature.c | 119 +++++++++++++++++++++++++++++++++++++++++++++++
> fs/overlayfs/overlayfs.h | 15 ++++++
> fs/overlayfs/ovl_entry.h | 5 ++
> fs/overlayfs/super.c | 5 ++
> 5 files changed, 145 insertions(+), 1 deletion(-)
> create mode 100644 fs/overlayfs/feature.c
>
> diff --git a/fs/overlayfs/Makefile b/fs/overlayfs/Makefile
> index 30802347a020..219c927069d2 100644
> --- a/fs/overlayfs/Makefile
> +++ b/fs/overlayfs/Makefile
> @@ -5,4 +5,4 @@
> obj-$(CONFIG_OVERLAY_FS) += overlay.o
>
> overlay-objs := super.o namei.o util.o inode.o dir.o readdir.o copy_up.o \
> - export.o
> + export.o feature.o
> diff --git a/fs/overlayfs/feature.c b/fs/overlayfs/feature.c
> new file mode 100644
> index 000000000000..eed6644a2a31
> --- /dev/null
> +++ b/fs/overlayfs/feature.c
> @@ -0,0 +1,119 @@
> +/*
> + * Overlayfs feature sets support.
> + *
> + * Copyright (C) 2018 Huawei. All Rights Reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published by
> + * the Free Software Foundation.
> + */
> +
> +#include <linux/fs.h>
> +#include <linux/xattr.h>
> +#include "overlayfs.h"
> +
> +/*
> + * Get overlay features from the layer's root dir.
> + *
> + * @realroot: real root dir dentry
> + *
> + * Return on-disk overlay feature struct if success, NULL if no feature
> + * and error ptr if error.
> + */
> +static struct ovl_d_feature *ovl_get_feature(struct dentry *realroot)
It may be more appropriate to use plural (features) for function, struct
and xattr name. Don't feel so strongly about it.
> +{
> + struct ovl_d_feature *odf = NULL;
> + int res;
> +
> + res = vfs_getxattr(realroot, OVL_XATTR_FEATURE, NULL, 0);
> + if (res <= 0) {
> + if (res == -EOPNOTSUPP || res == -ENODATA)
> + return NULL;
> + goto fail;
> + }
> +
> + odf = kzalloc(res, GFP_KERNEL);
Don't really see the point in allocating this struct. You can keep the
features buffer on the stack and check res != sizeof(struct ovl_d_feature)
before reading into the buffer.
> + if (!odf)
> + return ERR_PTR(-ENOMEM);
> +
> + res = vfs_getxattr(realroot, OVL_XATTR_FEATURE, odf, res);
> + if (res < 0)
> + goto fail;
> +
> + /* Check validity */
> + if (res < sizeof(struct ovl_d_feature)) {
> + res = -EINVAL;
> + goto invalid;
> + }
> + if (odf->magic != OVL_FEATURE_MAGIC) {
> + res = -EINVAL;
> + goto invalid;
> + }
> +
> + return odf;
> +out:
> + kfree(odf);
> + return ERR_PTR(res);
> +fail:
> + pr_err("overlayfs: failed to get features (%i)\n", res);
> + goto out;
> +invalid:
> + pr_err("overlayfs: invalid features (%*phN)\n", res, odf);
> + goto out;
> +}
> +
> +/*
> + * Get features from each underlying root dir.
> + *
> + * @ofs: overlay filesystem information
> + * @oe: overlay lower stack
> + *
> + * Return 0 if success, errno otherwise.
> + */
> +int ovl_init_feature_set(struct ovl_fs *ofs, struct ovl_entry *oe)
> +{
> + struct ovl_layer *upper_layer = ofs->upper_layer;
> + struct dentry *upper = ofs->upperdir;
> + struct ovl_d_feature *odf;
> + int i;
> +
> + /* Get features from the upper root dir */
> + if (upper_layer) {
> + odf = ovl_get_feature(upper);
> + if (IS_ERR(odf))
> + return PTR_ERR(odf);
> +
> + if (odf) {
> + upper_layer->compat = be64_to_cpu(odf->compat);
> + upper_layer->ro_compat = be64_to_cpu(odf->ro_compat);
> + upper_layer->incompat = be64_to_cpu(odf->incompat);
> + upper_layer->feature = true;
> + kfree(odf);
> + } else {
> + upper_layer->feature = false;
> + }
Please pass ovl_layer * into ovl_get_feature[s]() and do all this
inside the helper - it is repeated for lower layers.
> + }
> +
> + /* Get features from each lower's root dir */
> + for (i = 0; i < oe->numlower; i++) {
> + struct ovl_path *lowerpath = &oe->lowerstack[i];
> + struct ovl_layer *lower_layer = &ofs->lower_layers[i];
> +
> + odf = ovl_get_feature(lowerpath->dentry);
> + if (IS_ERR(odf))
> + return PTR_ERR(odf);
> +
> + if (!odf) {
> + lower_layer->feature = false;
> + continue;
> + }
> +
> + lower_layer->compat = be64_to_cpu(odf->compat);
> + lower_layer->ro_compat = be64_to_cpu(odf->ro_compat);
> + lower_layer->incompat = be64_to_cpu(odf->incompat);
> + lower_layer->feature = true;
> + kfree(odf);
> + }
> +
> + return 0;
> +}
> diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
> index 7538b9b56237..745f3b89a99e 100644
> --- a/fs/overlayfs/overlayfs.h
> +++ b/fs/overlayfs/overlayfs.h
> @@ -28,6 +28,7 @@ enum ovl_path_type {
> #define OVL_XATTR_IMPURE OVL_XATTR_PREFIX "impure"
> #define OVL_XATTR_NLINK OVL_XATTR_PREFIX "nlink"
> #define OVL_XATTR_UPPER OVL_XATTR_PREFIX "upper"
> +#define OVL_XATTR_FEATURE OVL_XATTR_PREFIX "feature"
>
> enum ovl_inode_flag {
> /* Pure upper dir that may contain non pure upper entries */
> @@ -43,6 +44,17 @@ enum ovl_entry_flag {
> OVL_E_CONNECTED,
> };
>
> +/* Features */
> +#define OVL_FEATURE_MAGIC 0xfe
> +
> +/* On-disk format of overlay layer features */
> +struct ovl_d_feature {
u8 version; (it may be 2 if we want to relate to ovl format v2)
> + u8 magic; /* 0xfe */
u16 pad; (just for semantically defining the on-disk format zero padding)
> + __be64 compat; /* compatible features */
> + __be64 ro_compat; /* read-only compatible features */
> + __be64 incompat; /* incompatible features */
> +} __packed;
> +
> /*
> * The tuple (fh,uuid) is a universal unique identifier for a copy up origin,
> * where:
> @@ -379,3 +391,6 @@ int ovl_set_origin(struct dentry *dentry, struct dentry *lower,
>
> /* export.c */
> extern const struct export_operations ovl_export_operations;
> +
> +/* feature.c */
> +int ovl_init_feature_set(struct ovl_fs *ofs, struct ovl_entry *oe);
> diff --git a/fs/overlayfs/ovl_entry.h b/fs/overlayfs/ovl_entry.h
> index 38ff6689a293..b1b6627f3350 100644
> --- a/fs/overlayfs/ovl_entry.h
> +++ b/fs/overlayfs/ovl_entry.h
> @@ -34,6 +34,11 @@ struct ovl_layer {
> int idx;
> /* One fsid per unique underlying sb (upper fsid == 0) */
> int fsid;
> + /* Features of this layer */
> + u64 compat;
> + u64 ro_compat;
> + u64 incompat;
> + bool feature;
Not that size of ovl_layer matters so much, but technically, the
boolean 'feature' can be represented with a single compat feature bit
if we define that the on-disk format is never initialized with all zeros,
so checking !layer->feature is equivalent to checking !layer->compat.
For example the feature OVL_FEATURE_COMPAT_V2
will always be set if features xattrs exists.
I write 'compat' only because old kernel *will* mount an overlay
with this feature set (not knowing to check it), but fsck.overlay
should not have any "official" version that does not check the
"V2" feature.
I did not yet look at all the patches to determine if the 'feature'
boolean is useful by itself, so not ruling it out completely.
Thanks,
Amir.
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [RFC PATCH v2 2/9] ovl: read feature set from each layer's root dir
2018-08-01 7:44 ` Amir Goldstein
@ 2018-08-03 12:11 ` zhangyi (F)
0 siblings, 0 replies; 26+ messages in thread
From: zhangyi (F) @ 2018-08-03 12:11 UTC (permalink / raw)
To: Amir Goldstein; +Cc: overlayfs, Miklos Szeredi, Miao Xie, yangerkun
On 2018/8/1 15:44, Amir Goldstein Wrote:
> On Tue, Jul 31, 2018 at 12:37 PM, zhangyi (F) <yi.zhang@huawei.com> wrote:
>> In order to distinguish the backward compatible and backward
>> incompatible features in overlay filesystem, we introduce compatible,
>> read-only compatible and incompatible these three kinds of features,
>> store them as __be64 bit mask on each layer's root directory via
>> "trusted.overlay.feature" xattr, which contains the features of this
>> layer.
>>
>> This patch just read feature bitsets from each layer, doesn't add any
>> already known features and doesn't yet use the feature bitsets.
>>
>> Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
>> Suggested-by: Miklos Szeredi <miklos@szeredi.hu>
>> Suggested-by: Amir Goldstein <amir73il@gmail.com>
>> ---
[..]
>> +
>> +/*
>> + * Get overlay features from the layer's root dir.
>> + *
>> + * @realroot: real root dir dentry
>> + *
>> + * Return on-disk overlay feature struct if success, NULL if no feature
>> + * and error ptr if error.
>> + */
>> +static struct ovl_d_feature *ovl_get_feature(struct dentry *realroot)
>
> It may be more appropriate to use plural (features) for function, struct
> and xattr name. Don't feel so strongly about it.
>
Will do.
>> +{
>> + struct ovl_d_feature *odf = NULL;
>> + int res;
>> +
>> + res = vfs_getxattr(realroot, OVL_XATTR_FEATURE, NULL, 0);
>> + if (res <= 0) {
>> + if (res == -EOPNOTSUPP || res == -ENODATA)
>> + return NULL;
>> + goto fail;
>> + }
>> +
>> + odf = kzalloc(res, GFP_KERNEL);
>
> Don't really see the point in allocating this struct. You can keep the
> features buffer on the stack and check res != sizeof(struct ovl_d_feature)
> before reading into the buffer.
>
Will do.
>> + if (!odf)
>> + return ERR_PTR(-ENOMEM);
>> +
>> + res = vfs_getxattr(realroot, OVL_XATTR_FEATURE, odf, res);
>> + if (res < 0)
>> + goto fail;
>> +
>> + /* Check validity */
>> + if (res < sizeof(struct ovl_d_feature)) {
>> + res = -EINVAL;
>> + goto invalid;
>> + }
>> + if (odf->magic != OVL_FEATURE_MAGIC) {
>> + res = -EINVAL;
>> + goto invalid;
>> + }
>> +
>> + return odf;
>> +out:
>> + kfree(odf);
>> + return ERR_PTR(res);
>> +fail:
>> + pr_err("overlayfs: failed to get features (%i)\n", res);
>> + goto out;
>> +invalid:
>> + pr_err("overlayfs: invalid features (%*phN)\n", res, odf);
>> + goto out;
>> +}
>> +
>> +/*
>> + * Get features from each underlying root dir.
>> + *
>> + * @ofs: overlay filesystem information
>> + * @oe: overlay lower stack
>> + *
>> + * Return 0 if success, errno otherwise.
>> + */
>> +int ovl_init_feature_set(struct ovl_fs *ofs, struct ovl_entry *oe)
>> +{
>> + struct ovl_layer *upper_layer = ofs->upper_layer;
>> + struct dentry *upper = ofs->upperdir;
>> + struct ovl_d_feature *odf;
>> + int i;
>> +
>> + /* Get features from the upper root dir */
>> + if (upper_layer) {
>> + odf = ovl_get_feature(upper);
>> + if (IS_ERR(odf))
>> + return PTR_ERR(odf);
>> +
>> + if (odf) {
>> + upper_layer->compat = be64_to_cpu(odf->compat);
>> + upper_layer->ro_compat = be64_to_cpu(odf->ro_compat);
>> + upper_layer->incompat = be64_to_cpu(odf->incompat);
>> + upper_layer->feature = true;
>> + kfree(odf);
>> + } else {
>> + upper_layer->feature = false;
>> + }
>
> Please pass ovl_layer * into ovl_get_feature[s]() and do all this
> inside the helper - it is repeated for lower layers.
>
Will do.
>> + }
>> +
>> + /* Get features from each lower's root dir */
>> + for (i = 0; i < oe->numlower; i++) {
>> + struct ovl_path *lowerpath = &oe->lowerstack[i];
>> + struct ovl_layer *lower_layer = &ofs->lower_layers[i];
>> +
>> + odf = ovl_get_feature(lowerpath->dentry);
>> + if (IS_ERR(odf))
>> + return PTR_ERR(odf);
>> +
>> + if (!odf) {
>> + lower_layer->feature = false;
>> + continue;
>> + }
>> +
>> + lower_layer->compat = be64_to_cpu(odf->compat);
>> + lower_layer->ro_compat = be64_to_cpu(odf->ro_compat);
>> + lower_layer->incompat = be64_to_cpu(odf->incompat);
>> + lower_layer->feature = true;
>> + kfree(odf);
>> + }
>> +
>> + return 0;
>> +}
>> diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
>> index 7538b9b56237..745f3b89a99e 100644
>> --- a/fs/overlayfs/overlayfs.h
>> +++ b/fs/overlayfs/overlayfs.h
>> @@ -28,6 +28,7 @@ enum ovl_path_type {
>> #define OVL_XATTR_IMPURE OVL_XATTR_PREFIX "impure"
>> #define OVL_XATTR_NLINK OVL_XATTR_PREFIX "nlink"
>> #define OVL_XATTR_UPPER OVL_XATTR_PREFIX "upper"
>> +#define OVL_XATTR_FEATURE OVL_XATTR_PREFIX "feature"
>>
>> enum ovl_inode_flag {
>> /* Pure upper dir that may contain non pure upper entries */
>> @@ -43,6 +44,17 @@ enum ovl_entry_flag {
>> OVL_E_CONNECTED,
>> };
>>
>> +/* Features */
>> +#define OVL_FEATURE_MAGIC 0xfe
>> +
>> +/* On-disk format of overlay layer features */
>> +struct ovl_d_feature {
> u8 version; (it may be 2 if we want to relate to ovl format v2)
>
IIUC, we refer the layer contains this feature set xattr is format v2,
and the layer doesn't contains is format v1. So why we need this "version"
parameter, always 2 when we set feature set xattr and prepare for the
future when we want to change this sturcture ?
>> + u8 magic; /* 0xfe */
>
> u16 pad; (just for semantically defining the on-disk format zero padding)
>
>> + __be64 compat; /* compatible features */
>> + __be64 ro_compat; /* read-only compatible features */
>> + __be64 incompat; /* incompatible features */
>> +} __packed;
>> +
>> /*
>> * The tuple (fh,uuid) is a universal unique identifier for a copy up origin,
>> * where:
>> @@ -379,3 +391,6 @@ int ovl_set_origin(struct dentry *dentry, struct dentry *lower,
>>
>> /* export.c */
>> extern const struct export_operations ovl_export_operations;
>> +
>> +/* feature.c */
>> +int ovl_init_feature_set(struct ovl_fs *ofs, struct ovl_entry *oe);
>> diff --git a/fs/overlayfs/ovl_entry.h b/fs/overlayfs/ovl_entry.h
>> index 38ff6689a293..b1b6627f3350 100644
>> --- a/fs/overlayfs/ovl_entry.h
>> +++ b/fs/overlayfs/ovl_entry.h
>> @@ -34,6 +34,11 @@ struct ovl_layer {
>> int idx;
>> /* One fsid per unique underlying sb (upper fsid == 0) */
>> int fsid;
>> + /* Features of this layer */
>> + u64 compat;
>> + u64 ro_compat;
>> + u64 incompat;
>> + bool feature;
>
> Not that size of ovl_layer matters so much, but technically, the
> boolean 'feature' can be represented with a single compat feature bit
> if we define that the on-disk format is never initialized with all zeros,
> so checking !layer->feature is equivalent to checking !layer->compat.
>
> For example the feature OVL_FEATURE_COMPAT_V2
> will always be set if features xattrs exists.
> I write 'compat' only because old kernel *will* mount an overlay
> with this feature set (not knowing to check it), but fsck.overlay
> should not have any "official" version that does not check the
> "V2" feature.
>
> I did not yet look at all the patches to determine if the 'feature'
> boolean is useful by itself, so not ruling it out completely.
>
Sounds good, will do.
Thanks,
Yi.
^ permalink raw reply [flat|nested] 26+ messages in thread
* [RFC PATCH v2 3/9] ovl: check file system features can be mounted properly
2018-07-31 9:37 [RFC PATCH v2 0/9] ovl: add feature set support zhangyi (F)
2018-07-31 9:37 ` [RFC PATCH v2 1/9] ovl: store ovl upper root path in ovl_fs zhangyi (F)
2018-07-31 9:37 ` [RFC PATCH v2 2/9] ovl: read feature set from each layer's root dir zhangyi (F)
@ 2018-07-31 9:37 ` zhangyi (F)
2018-08-01 8:06 ` Amir Goldstein
2018-07-31 9:37 ` [RFC PATCH v2 4/9] ovl: add helper funcs to set upper layer feature set zhangyi (F)
` (5 subsequent siblings)
8 siblings, 1 reply; 26+ messages in thread
From: zhangyi (F) @ 2018-07-31 9:37 UTC (permalink / raw)
To: linux-unionfs; +Cc: miklos, amir73il, yi.zhang, miaoxie, yangerkun
Mount operation should fail if unknown incompatible feature is
detected on any one of the underlying layers, and read-write mount
operation should fail if unknown read-only compatible feature is
detected on the upper layer.
This patch doesn't add any features, so if any feature xattrs is
detected on the underlying root dir, it will be recognized as
unknown feature.
Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
---
fs/overlayfs/feature.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++
fs/overlayfs/overlayfs.h | 29 +++++++++++++++++++++
fs/overlayfs/super.c | 8 +++++-
3 files changed, 101 insertions(+), 1 deletion(-)
diff --git a/fs/overlayfs/feature.c b/fs/overlayfs/feature.c
index eed6644a2a31..fac4d7544475 100644
--- a/fs/overlayfs/feature.c
+++ b/fs/overlayfs/feature.c
@@ -117,3 +117,68 @@ int ovl_init_feature_set(struct ovl_fs *ofs, struct ovl_entry *oe)
return 0;
}
+
+/*
+ * Check whether this overlay layer can be mounted based on
+ * the features present and the read-only/read-write mount requested.
+ * Returns true if this layer can be mounted as requested,
+ * false if it cannot be.
+ */
+static bool ovl_check_layer_feature(struct ovl_layer *layer, bool readonly)
+{
+ if (ovl_has_unknown_incompat_features(layer)) {
+ pr_err("overlayfs: unknown optional incompat features "
+ "(0x%llx) enabled on layer %d\n",
+ layer->incompat & OVL_FEATURE_INCOMPAT_UNKNOWN,
+ layer->idx);
+ pr_err("overlayfs: filesystem cannot not be safely mounted "
+ "by this kernel\n");
+ return false;
+ }
+
+ if (ovl_has_unknown_ro_compat_features(layer)) {
+ pr_err("overlayfs: unknown optional ro_compat features "
+ "(0x%llx) enabled on layer %d\n",
+ layer->ro_compat & OVL_FEATURE_RO_COMPAT_UNKNOWN,
+ layer->idx);
+
+ if (readonly)
+ return true;
+
+ pr_err("overlayfs: filesystem cannot not be safely mounted "
+ "read-write by this kernel\n");
+ return false;
+ }
+
+ if (ovl_has_unknown_compat_features(layer)) {
+ pr_warn("overlayfs: unknown optional compat features "
+ "(0x%llx) enabled on layer %d\n",
+ layer->compat & OVL_FEATURE_COMPAT_UNKNOWN,
+ layer->idx);
+ }
+
+ return true;
+}
+
+/*
+ * Check all layers of overlay filesystem to make sure whether this
+ * filesystem can be mounted. Returns true if all layers can be
+ * mounted as requested, false if any one of them cannot be.
+ */
+bool ovl_check_feature_ok(struct ovl_fs *ofs, bool readonly)
+{
+ int i;
+
+ if (ofs->upper_layer) {
+ if (!ovl_check_layer_feature(ofs->upper_layer, readonly))
+ return false;
+ }
+
+ for (i = 0; i < ofs->numlower; i++) {
+ /* Lower layer is always read-only */
+ if (!ovl_check_layer_feature(&ofs->lower_layers[i], true))
+ return false;
+ }
+
+ return true;
+}
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index 745f3b89a99e..f1bf21d030ac 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -55,6 +55,34 @@ struct ovl_d_feature {
__be64 incompat; /* incompatible features */
} __packed;
+
+#define OVL_FEATURE_COMPAT_SUPP (0)
+#define OVL_FEATURE_COMPAT_UNKNOWN (~OVL_FEATURE_COMPAT_SUPP)
+
+#define OVL_FEATURE_RO_COMPAT_SUPP (0)
+#define OVL_FEATURE_RO_COMPAT_UNKNOWN (~OVL_FEATURE_RO_COMPAT_SUPP)
+
+#define OVL_FEATURE_INCOMPAT_SUPP (0)
+#define OVL_FEATURE_INCOMPAT_UNKNOWN (~OVL_FEATURE_INCOMPAT_SUPP)
+
+static inline bool ovl_has_unknown_compat_features(struct ovl_layer *layer)
+{
+ return ((layer->feature) && \
+ ((layer->compat & OVL_FEATURE_COMPAT_UNKNOWN) != 0));
+}
+
+static inline bool ovl_has_unknown_ro_compat_features(struct ovl_layer *layer)
+{
+ return ((layer->feature) && \
+ ((layer->ro_compat & OVL_FEATURE_RO_COMPAT_UNKNOWN) != 0));
+}
+
+static inline bool ovl_has_unknown_incompat_features(struct ovl_layer *layer)
+{
+ return ((layer->feature) && \
+ ((layer->incompat & OVL_FEATURE_INCOMPAT_UNKNOWN) != 0));
+}
+
/*
* The tuple (fh,uuid) is a universal unique identifier for a copy up origin,
* where:
@@ -394,3 +422,4 @@ extern const struct export_operations ovl_export_operations;
/* feature.c */
int ovl_init_feature_set(struct ovl_fs *ofs, struct ovl_entry *oe);
+bool ovl_check_feature_ok(struct ovl_fs *ofs, bool readonly);
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index 59b7d4f07b03..f8e516656104 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -385,7 +385,8 @@ static int ovl_remount(struct super_block *sb, int *flags, char *data)
{
struct ovl_fs *ofs = sb->s_fs_info;
- if (!(*flags & SB_RDONLY) && ovl_force_readonly(ofs))
+ if (!(*flags & SB_RDONLY) &&
+ (ovl_force_readonly(ofs) || !ovl_check_feature_ok(ofs, false)))
return -EROFS;
return 0;
@@ -1450,6 +1451,11 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
}
+ /* Check the features could be mounted properly by this kernel */
+ err = -EINVAL;
+ if (!ovl_check_feature_ok(ofs, (sb_rdonly(sb))))
+ goto out_free_oe;
+
/* Show index=off in /proc/mounts for forced r/o mount */
if (!ofs->indexdir) {
ofs->config.index = false;
--
2.13.6
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [RFC PATCH v2 3/9] ovl: check file system features can be mounted properly
2018-07-31 9:37 ` [RFC PATCH v2 3/9] ovl: check file system features can be mounted properly zhangyi (F)
@ 2018-08-01 8:06 ` Amir Goldstein
0 siblings, 0 replies; 26+ messages in thread
From: Amir Goldstein @ 2018-08-01 8:06 UTC (permalink / raw)
To: zhangyi (F); +Cc: overlayfs, Miklos Szeredi, Miao Xie, yangerkun
On Tue, Jul 31, 2018 at 12:37 PM, zhangyi (F) <yi.zhang@huawei.com> wrote:
> Mount operation should fail if unknown incompatible feature is
> detected on any one of the underlying layers, and read-write mount
> operation should fail if unknown read-only compatible feature is
> detected on the upper layer.
>
> This patch doesn't add any features, so if any feature xattrs is
> detected on the underlying root dir, it will be recognized as
> unknown feature.
>
> Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
> ---
> fs/overlayfs/feature.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++
> fs/overlayfs/overlayfs.h | 29 +++++++++++++++++++++
> fs/overlayfs/super.c | 8 +++++-
> 3 files changed, 101 insertions(+), 1 deletion(-)
>
> diff --git a/fs/overlayfs/feature.c b/fs/overlayfs/feature.c
> index eed6644a2a31..fac4d7544475 100644
> --- a/fs/overlayfs/feature.c
> +++ b/fs/overlayfs/feature.c
> @@ -117,3 +117,68 @@ int ovl_init_feature_set(struct ovl_fs *ofs, struct ovl_entry *oe)
>
> return 0;
> }
> +
> +/*
> + * Check whether this overlay layer can be mounted based on
> + * the features present and the read-only/read-write mount requested.
> + * Returns true if this layer can be mounted as requested,
> + * false if it cannot be.
> + */
> +static bool ovl_check_layer_feature(struct ovl_layer *layer, bool readonly)
> +{
> + if (ovl_has_unknown_incompat_features(layer)) {
> + pr_err("overlayfs: unknown optional incompat features "
> + "(0x%llx) enabled on layer %d\n",
I think it will be more informative to user to print layer root dentry
than layer idx on error message for mount failure.
The word "optional" doesn't seem to add much information to user.
> + layer->incompat & OVL_FEATURE_INCOMPAT_UNKNOWN,
> + layer->idx);
> + pr_err("overlayfs: filesystem cannot not be safely mounted "
> + "by this kernel\n");
> + return false;
> + }
> +
> + if (ovl_has_unknown_ro_compat_features(layer)) {
> + pr_err("overlayfs: unknown optional ro_compat features "
> + "(0x%llx) enabled on layer %d\n",
> + layer->ro_compat & OVL_FEATURE_RO_COMPAT_UNKNOWN,
> + layer->idx);
> +
> + if (readonly)
> + return true;
if (readonly), then:
- error above should probably be a warning
- we should not skip warning of unknown compat features below
I don't know if those are so important. I wonder how ext4 behaves
in those cases?
> +
> + pr_err("overlayfs: filesystem cannot not be safely mounted "
> + "read-write by this kernel\n");
> + return false;
> + }
> +
> + if (ovl_has_unknown_compat_features(layer)) {
> + pr_warn("overlayfs: unknown optional compat features "
> + "(0x%llx) enabled on layer %d\n",
> + layer->compat & OVL_FEATURE_COMPAT_UNKNOWN,
> + layer->idx);
> + }
> +
> + return true;
> +}
> +
> +/*
> + * Check all layers of overlay filesystem to make sure whether this
> + * filesystem can be mounted. Returns true if all layers can be
> + * mounted as requested, false if any one of them cannot be.
> + */
> +bool ovl_check_feature_ok(struct ovl_fs *ofs, bool readonly)
> +{
> + int i;
> +
> + if (ofs->upper_layer) {
> + if (!ovl_check_layer_feature(ofs->upper_layer, readonly))
> + return false;
> + }
> +
> + for (i = 0; i < ofs->numlower; i++) {
> + /* Lower layer is always read-only */
> + if (!ovl_check_layer_feature(&ofs->lower_layers[i], true))
> + return false;
> + }
> +
> + return true;
> +}
> diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
> index 745f3b89a99e..f1bf21d030ac 100644
> --- a/fs/overlayfs/overlayfs.h
> +++ b/fs/overlayfs/overlayfs.h
> @@ -55,6 +55,34 @@ struct ovl_d_feature {
> __be64 incompat; /* incompatible features */
> } __packed;
>
> +
> +#define OVL_FEATURE_COMPAT_SUPP (0)
> +#define OVL_FEATURE_COMPAT_UNKNOWN (~OVL_FEATURE_COMPAT_SUPP)
> +
> +#define OVL_FEATURE_RO_COMPAT_SUPP (0)
> +#define OVL_FEATURE_RO_COMPAT_UNKNOWN (~OVL_FEATURE_RO_COMPAT_SUPP)
> +
> +#define OVL_FEATURE_INCOMPAT_SUPP (0)
> +#define OVL_FEATURE_INCOMPAT_UNKNOWN (~OVL_FEATURE_INCOMPAT_SUPP)
> +
> +static inline bool ovl_has_unknown_compat_features(struct ovl_layer *layer)
> +{
> + return ((layer->feature) && \
> + ((layer->compat & OVL_FEATURE_COMPAT_UNKNOWN) != 0));
Do we need != 0? does compiler complain about conversion to bool??
> +}
> +
> +static inline bool ovl_has_unknown_ro_compat_features(struct ovl_layer *layer)
> +{
> + return ((layer->feature) && \
> + ((layer->ro_compat & OVL_FEATURE_RO_COMPAT_UNKNOWN) != 0));
> +}
> +
> +static inline bool ovl_has_unknown_incompat_features(struct ovl_layer *layer)
> +{
> + return ((layer->feature) && \
> + ((layer->incompat & OVL_FEATURE_INCOMPAT_UNKNOWN) != 0));
> +}
> +
> /*
> * The tuple (fh,uuid) is a universal unique identifier for a copy up origin,
> * where:
> @@ -394,3 +422,4 @@ extern const struct export_operations ovl_export_operations;
>
> /* feature.c */
> int ovl_init_feature_set(struct ovl_fs *ofs, struct ovl_entry *oe);
> +bool ovl_check_feature_ok(struct ovl_fs *ofs, bool readonly);
> diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> index 59b7d4f07b03..f8e516656104 100644
> --- a/fs/overlayfs/super.c
> +++ b/fs/overlayfs/super.c
> @@ -385,7 +385,8 @@ static int ovl_remount(struct super_block *sb, int *flags, char *data)
> {
> struct ovl_fs *ofs = sb->s_fs_info;
>
> - if (!(*flags & SB_RDONLY) && ovl_force_readonly(ofs))
> + if (!(*flags & SB_RDONLY) &&
> + (ovl_force_readonly(ofs) || !ovl_check_feature_ok(ofs, false)))
> return -EROFS;
>
> return 0;
> @@ -1450,6 +1451,11 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
>
> }
>
> + /* Check the features could be mounted properly by this kernel */
> + err = -EINVAL;
> + if (!ovl_check_feature_ok(ofs, (sb_rdonly(sb))))
> + goto out_free_oe;
> +
Unneeded () around sb_rdonly(sb).
Thanks,
Amir.
^ permalink raw reply [flat|nested] 26+ messages in thread
* [RFC PATCH v2 4/9] ovl: add helper funcs to set upper layer feature set
2018-07-31 9:37 [RFC PATCH v2 0/9] ovl: add feature set support zhangyi (F)
` (2 preceding siblings ...)
2018-07-31 9:37 ` [RFC PATCH v2 3/9] ovl: check file system features can be mounted properly zhangyi (F)
@ 2018-07-31 9:37 ` zhangyi (F)
2018-08-01 8:51 ` Amir Goldstein
2018-07-31 9:37 ` [RFC PATCH v2 5/9] ovl: add redirect dir feature when set redirect xattr to dir zhangyi (F)
` (4 subsequent siblings)
8 siblings, 1 reply; 26+ messages in thread
From: zhangyi (F) @ 2018-07-31 9:37 UTC (permalink / raw)
To: linux-unionfs; +Cc: miklos, amir73il, yi.zhang, miaoxie, yangerkun
overlayfs will want to add the upper layer's feature bitset if some
overlay file system features were set. For example: redirect dir feature
will be set when user rename a lower/merge dir if redirect dir feature
is enabled. So add helper functions to set feature sets to the upper
layer for future use.
This patch introduce helper functions only, does not set any feature
bits.
Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
---
fs/overlayfs/feature.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++
fs/overlayfs/overlayfs.h | 46 ++++++++++++++++++++++++++
fs/overlayfs/ovl_entry.h | 2 ++
fs/overlayfs/super.c | 1 +
4 files changed, 134 insertions(+)
diff --git a/fs/overlayfs/feature.c b/fs/overlayfs/feature.c
index fac4d7544475..7c66e71bdd98 100644
--- a/fs/overlayfs/feature.c
+++ b/fs/overlayfs/feature.c
@@ -10,6 +10,8 @@
#include <linux/fs.h>
#include <linux/xattr.h>
+#include <linux/mount.h>
+#include <linux/ratelimit.h>
#include "overlayfs.h"
/*
@@ -63,6 +65,89 @@ static struct ovl_d_feature *ovl_get_feature(struct dentry *realroot)
}
/*
+ * Set overlay features to the upper root dir.
+ *
+ * @upper_layer: overlay upper layer information,
+ * @upper: upper root dentry.
+ *
+ * Return 0 if success, error number otherwise.
+ */
+static int ovl_set_feature(struct ovl_layer *upper_layer,
+ struct dentry *upper)
+{
+ struct vfsmount *upper_mnt = upper_layer->mnt;
+ struct ovl_d_feature odf = {};
+ int err;
+
+ odf.magic = OVL_FEATURE_MAGIC;
+ odf.compat = cpu_to_be64(upper_layer->compat);
+ odf.ro_compat = cpu_to_be64(upper_layer->ro_compat);
+ odf.incompat = cpu_to_be64(upper_layer->incompat);
+
+ err = mnt_want_write(upper_mnt);
+ if (err)
+ return err;
+
+ err = ovl_do_setxattr(upper, OVL_XATTR_FEATURE,
+ &odf, sizeof(odf), 0);
+ if (err)
+ pr_err_ratelimited("overlayfs: failed to set features (%i)\n", err);
+
+ mnt_drop_write(upper_mnt);
+ return err;
+}
+
+/*
+ * Add feature bit mask to the upper root dir.
+ *
+ * @ofs: overlay filesystem information
+ * @type: feature set type, compat, ro compat or incompat
+ * @mask: new features mask want to add
+ *
+ * Return 0 if success, error number otherwise.
+ */
+int ovl_set_upper_feature(struct ovl_fs *ofs,
+ enum ovl_feature_type type,
+ u64 mask)
+{
+ struct ovl_layer *upper_layer = ofs->upper_layer;
+ u64 *features;
+ int err;
+
+ if (!upper_layer)
+ return -EINVAL;
+
+ switch (type) {
+ case OVL_FEATURE_COMPAT:
+ features = &upper_layer->compat;
+ break;
+ case OVL_FEATURE_RO_COMPAT:
+ features = &upper_layer->ro_compat;
+ break;
+ case OVL_FEATURE_INCOMPAT:
+ features = &upper_layer->incompat;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ spin_lock(&upper_layer->lock);
+ if ((*features) & mask) {
+ spin_unlock(&upper_layer->lock);
+ return 0;
+ }
+
+ (*features) |= mask;
+ spin_unlock(&upper_layer->lock);
+
+ err = ovl_set_feature(upper_layer, ofs->upperdir);
+ if (err)
+ return err;
+
+ return 0;
+}
+
+/*
* Get features from each underlying root dir.
*
* @ofs: overlay filesystem information
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index f1bf21d030ac..d6231acc660a 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -83,6 +83,52 @@ static inline bool ovl_has_unknown_incompat_features(struct ovl_layer *layer)
((layer->incompat & OVL_FEATURE_INCOMPAT_UNKNOWN) != 0));
}
+enum ovl_feature_type {
+ OVL_FEATURE_COMPAT,
+ OVL_FEATURE_RO_COMPAT,
+ OVL_FEATURE_INCOMPAT
+};
+
+int ovl_set_upper_feature(struct ovl_fs *ofs,
+ enum ovl_feature_type type,
+ u64 mask);
+
+#define OVL_FEATURE_COMPAT_FUNCS(name, flagname) \
+static inline bool ovl_has_feature_##name(struct ovl_layer *layer) \
+{ \
+ return ((layer->feature) && \
+ ((layer->compat & OVL_FEATURE_COMPAT_##flagname) != 0); \
+} \
+static inline int ovl_set_feature_##name(struct ovl_fs *ofs) \
+{ \
+ return ovl_set_upper_feature(ofs, OVL_FEATURE_COMPAT, \
+ OVL_FEATURE_COMPAT_##flagname); \
+} \
+
+#define OVL_FEATURE_RO_COMPAT_FUNCS(name, flagname) \
+static inline bool ovl_has_feature_##name(struct ovl_layer *layer) \
+{ \
+ return ((layer->feature) && \
+ (layer->ro_compat & OVL_FEATURE_RO_COMPAT_##flagname) != 0); \
+} \
+static inline int ovl_set_feature_##name(struct ovl_fs *ofs) \
+{ \
+ return ovl_set_upper_feature(ofs, OVL_FEATURE_RO_COMPAT, \
+ OVL_FEATURE_RO_COMPAT_##flagname); \
+} \
+
+#define OVL_FEATURE_INCOMPAT_FUNCS(name, flagname) \
+static inline bool ovl_has_feature_##name(struct ovl_layer *layer) \
+{ \
+ return ((layer->feature) && \
+ (layer->incompat & OVL_FEATURE_INCOMPAT_##flagname) != 0); \
+} \
+static inline int ovl_set_feature_##name(struct ovl_fs *ofs) \
+{ \
+ return ovl_set_upper_feature(ofs, OVL_FEATURE_INCOMPAT, \
+ OVL_FEATURE_INCOMPAT_##flagname); \
+} \
+
/*
* The tuple (fh,uuid) is a universal unique identifier for a copy up origin,
* where:
diff --git a/fs/overlayfs/ovl_entry.h b/fs/overlayfs/ovl_entry.h
index b1b6627f3350..8a28e24dd149 100644
--- a/fs/overlayfs/ovl_entry.h
+++ b/fs/overlayfs/ovl_entry.h
@@ -39,6 +39,8 @@ struct ovl_layer {
u64 ro_compat;
u64 incompat;
bool feature;
+ /* Protect features when updating (upper use only) */
+ spinlock_t lock;
};
struct ovl_path {
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index f8e516656104..a5bbbddf741c 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -998,6 +998,7 @@ static int ovl_get_upper(struct ovl_fs *ofs, struct path *upperpath)
if (ofs->upper_layer == NULL)
goto out;
+ spin_lock_init(&ofs->upper_layer->lock);
ofs->upper_layer->mnt = upper_mnt;
ofs->upperdir = dget(upperpath->dentry);
err = 0;
--
2.13.6
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [RFC PATCH v2 4/9] ovl: add helper funcs to set upper layer feature set
2018-07-31 9:37 ` [RFC PATCH v2 4/9] ovl: add helper funcs to set upper layer feature set zhangyi (F)
@ 2018-08-01 8:51 ` Amir Goldstein
2018-08-01 11:04 ` Amir Goldstein
0 siblings, 1 reply; 26+ messages in thread
From: Amir Goldstein @ 2018-08-01 8:51 UTC (permalink / raw)
To: zhangyi (F); +Cc: overlayfs, Miklos Szeredi, Miao Xie, yangerkun
On Tue, Jul 31, 2018 at 12:37 PM, zhangyi (F) <yi.zhang@huawei.com> wrote:
> overlayfs will want to add the upper layer's feature bitset if some
> overlay file system features were set. For example: redirect dir feature
> will be set when user rename a lower/merge dir if redirect dir feature
> is enabled. So add helper functions to set feature sets to the upper
> layer for future use.
>
> This patch introduce helper functions only, does not set any feature
> bits.
>
> Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
> ---
> fs/overlayfs/feature.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++
> fs/overlayfs/overlayfs.h | 46 ++++++++++++++++++++++++++
> fs/overlayfs/ovl_entry.h | 2 ++
> fs/overlayfs/super.c | 1 +
> 4 files changed, 134 insertions(+)
>
> diff --git a/fs/overlayfs/feature.c b/fs/overlayfs/feature.c
> index fac4d7544475..7c66e71bdd98 100644
> --- a/fs/overlayfs/feature.c
> +++ b/fs/overlayfs/feature.c
> @@ -10,6 +10,8 @@
>
> #include <linux/fs.h>
> #include <linux/xattr.h>
> +#include <linux/mount.h>
> +#include <linux/ratelimit.h>
> #include "overlayfs.h"
>
> /*
> @@ -63,6 +65,89 @@ static struct ovl_d_feature *ovl_get_feature(struct dentry *realroot)
> }
>
> /*
> + * Set overlay features to the upper root dir.
> + *
> + * @upper_layer: overlay upper layer information,
> + * @upper: upper root dentry.
> + *
> + * Return 0 if success, error number otherwise.
> + */
> +static int ovl_set_feature(struct ovl_layer *upper_layer,
> + struct dentry *upper)
> +{
> + struct vfsmount *upper_mnt = upper_layer->mnt;
> + struct ovl_d_feature odf = {};
> + int err;
> +
> + odf.magic = OVL_FEATURE_MAGIC;
> + odf.compat = cpu_to_be64(upper_layer->compat);
> + odf.ro_compat = cpu_to_be64(upper_layer->ro_compat);
> + odf.incompat = cpu_to_be64(upper_layer->incompat);
> +
> + err = mnt_want_write(upper_mnt);
> + if (err)
> + return err;
> +
> + err = ovl_do_setxattr(upper, OVL_XATTR_FEATURE,
> + &odf, sizeof(odf), 0);
> + if (err)
> + pr_err_ratelimited("overlayfs: failed to set features (%i)\n", err);
> +
> + mnt_drop_write(upper_mnt);
> + return err;
> +}
> +
> +/*
> + * Add feature bit mask to the upper root dir.
> + *
> + * @ofs: overlay filesystem information
> + * @type: feature set type, compat, ro compat or incompat
> + * @mask: new features mask want to add
> + *
> + * Return 0 if success, error number otherwise.
> + */
> +int ovl_set_upper_feature(struct ovl_fs *ofs,
> + enum ovl_feature_type type,
> + u64 mask)
> +{
> + struct ovl_layer *upper_layer = ofs->upper_layer;
> + u64 *features;
> + int err;
> +
> + if (!upper_layer)
> + return -EINVAL;
> +
> + switch (type) {
> + case OVL_FEATURE_COMPAT:
> + features = &upper_layer->compat;
> + break;
> + case OVL_FEATURE_RO_COMPAT:
> + features = &upper_layer->ro_compat;
> + break;
> + case OVL_FEATURE_INCOMPAT:
> + features = &upper_layer->incompat;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + spin_lock(&upper_layer->lock);
> + if ((*features) & mask) {
> + spin_unlock(&upper_layer->lock);
> + return 0;
> + }
> +
> + (*features) |= mask;
> + spin_unlock(&upper_layer->lock);
> +
Seems like you can move the above test and set into
ovl_set_feature() and you can also use upperdir->d_lock
and don't need to introduce a new spinlock for this.
Thanks,
Amir.
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [RFC PATCH v2 4/9] ovl: add helper funcs to set upper layer feature set
2018-08-01 8:51 ` Amir Goldstein
@ 2018-08-01 11:04 ` Amir Goldstein
0 siblings, 0 replies; 26+ messages in thread
From: Amir Goldstein @ 2018-08-01 11:04 UTC (permalink / raw)
To: zhangyi (F); +Cc: overlayfs, Miklos Szeredi, Miao Xie, yangerkun
On Wed, Aug 1, 2018 at 11:51 AM, Amir Goldstein <amir73il@gmail.com> wrote:
> On Tue, Jul 31, 2018 at 12:37 PM, zhangyi (F) <yi.zhang@huawei.com> wrote:
>> overlayfs will want to add the upper layer's feature bitset if some
>> overlay file system features were set. For example: redirect dir feature
>> will be set when user rename a lower/merge dir if redirect dir feature
>> is enabled. So add helper functions to set feature sets to the upper
>> layer for future use.
>>
>> This patch introduce helper functions only, does not set any feature
>> bits.
>>
>> Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
>> ---
>> fs/overlayfs/feature.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++
>> fs/overlayfs/overlayfs.h | 46 ++++++++++++++++++++++++++
>> fs/overlayfs/ovl_entry.h | 2 ++
>> fs/overlayfs/super.c | 1 +
>> 4 files changed, 134 insertions(+)
>>
>> diff --git a/fs/overlayfs/feature.c b/fs/overlayfs/feature.c
>> index fac4d7544475..7c66e71bdd98 100644
>> --- a/fs/overlayfs/feature.c
>> +++ b/fs/overlayfs/feature.c
>> @@ -10,6 +10,8 @@
>>
>> #include <linux/fs.h>
>> #include <linux/xattr.h>
>> +#include <linux/mount.h>
>> +#include <linux/ratelimit.h>
>> #include "overlayfs.h"
>>
>> /*
>> @@ -63,6 +65,89 @@ static struct ovl_d_feature *ovl_get_feature(struct dentry *realroot)
>> }
>>
>> /*
>> + * Set overlay features to the upper root dir.
>> + *
>> + * @upper_layer: overlay upper layer information,
>> + * @upper: upper root dentry.
>> + *
>> + * Return 0 if success, error number otherwise.
>> + */
>> +static int ovl_set_feature(struct ovl_layer *upper_layer,
>> + struct dentry *upper)
>> +{
>> + struct vfsmount *upper_mnt = upper_layer->mnt;
>> + struct ovl_d_feature odf = {};
>> + int err;
>> +
>> + odf.magic = OVL_FEATURE_MAGIC;
>> + odf.compat = cpu_to_be64(upper_layer->compat);
>> + odf.ro_compat = cpu_to_be64(upper_layer->ro_compat);
>> + odf.incompat = cpu_to_be64(upper_layer->incompat);
>> +
>> + err = mnt_want_write(upper_mnt);
>> + if (err)
>> + return err;
>> +
>> + err = ovl_do_setxattr(upper, OVL_XATTR_FEATURE,
>> + &odf, sizeof(odf), 0);
>> + if (err)
>> + pr_err_ratelimited("overlayfs: failed to set features (%i)\n", err);
>> +
>> + mnt_drop_write(upper_mnt);
>> + return err;
>> +}
>> +
>> +/*
>> + * Add feature bit mask to the upper root dir.
>> + *
>> + * @ofs: overlay filesystem information
>> + * @type: feature set type, compat, ro compat or incompat
>> + * @mask: new features mask want to add
>> + *
>> + * Return 0 if success, error number otherwise.
>> + */
>> +int ovl_set_upper_feature(struct ovl_fs *ofs,
>> + enum ovl_feature_type type,
>> + u64 mask)
>> +{
>> + struct ovl_layer *upper_layer = ofs->upper_layer;
>> + u64 *features;
>> + int err;
>> +
>> + if (!upper_layer)
>> + return -EINVAL;
>> +
>> + switch (type) {
>> + case OVL_FEATURE_COMPAT:
>> + features = &upper_layer->compat;
>> + break;
>> + case OVL_FEATURE_RO_COMPAT:
>> + features = &upper_layer->ro_compat;
>> + break;
>> + case OVL_FEATURE_INCOMPAT:
>> + features = &upper_layer->incompat;
>> + break;
>> + default:
>> + return -EINVAL;
>> + }
>> +
>> + spin_lock(&upper_layer->lock);
>> + if ((*features) & mask) {
>> + spin_unlock(&upper_layer->lock);
>> + return 0;
>> + }
>> +
>> + (*features) |= mask;
>> + spin_unlock(&upper_layer->lock);
>> +
>
> Seems like you can move the above test and set into
> ovl_set_feature() and you can also use upperdir->d_lock
> and don't need to introduce a new spinlock for this.
>
Also, since there is no clear bit functionality, it is worth
testing the bit before the lock and then repeat the test under
lock if first test fails.
Thanks,
Amir.
^ permalink raw reply [flat|nested] 26+ messages in thread
* [RFC PATCH v2 5/9] ovl: add redirect dir feature when set redirect xattr to dir
2018-07-31 9:37 [RFC PATCH v2 0/9] ovl: add feature set support zhangyi (F)
` (3 preceding siblings ...)
2018-07-31 9:37 ` [RFC PATCH v2 4/9] ovl: add helper funcs to set upper layer feature set zhangyi (F)
@ 2018-07-31 9:37 ` zhangyi (F)
2018-08-01 11:03 ` Amir Goldstein
2018-07-31 9:37 ` [RFC PATCH v2 6/9] ovl: add index feature if index dir exists zhangyi (F)
` (3 subsequent siblings)
8 siblings, 1 reply; 26+ messages in thread
From: zhangyi (F) @ 2018-07-31 9:37 UTC (permalink / raw)
To: linux-unionfs; +Cc: miklos, amir73il, yi.zhang, miaoxie, yangerkun
Redirect dir feature is backward incompatible because old kernel (which
not support this feature) may corrupt the merge relationship between
directories when change the directory which have redirect xattr.
This patch check and set the upper layer's redirect dir feature when
kernel set redirect xattr to directory in the upper layer.
Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
---
fs/overlayfs/dir.c | 11 +++++++++++
fs/overlayfs/overlayfs.h | 5 ++++-
2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index f480b1a2cd2e..238999e5f47b 100644
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -969,6 +969,7 @@ static int ovl_rename(struct inode *olddir, struct dentry *old,
bool is_dir = d_is_dir(old);
bool new_is_dir = d_is_dir(new);
bool samedir = olddir == newdir;
+ struct ovl_fs* ofs = old->d_sb->s_fs_info;
struct dentry *opaquedir = NULL;
const struct cred *old_cred = NULL;
LIST_HEAD(list);
@@ -1061,6 +1062,16 @@ static int ovl_rename(struct inode *olddir, struct dentry *old,
}
}
+ if ((is_dir && ovl_type_merge_or_lower(old)) ||
+ (!overwrite && new_is_dir && ovl_type_merge_or_lower(new))) {
+ /*
+ * Set redirect dir feature to the upper root dir if this
+ * feature doesn't exist.
+ */
+ if (!ovl_has_feature_redirect_dir(ofs->upper_layer))
+ err = ovl_set_feature_redirect_dir(ofs);
+ }
+
trap = lock_rename(new_upperdir, old_upperdir);
olddentry = lookup_one_len(old->d_name.name, old_upperdir,
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index d6231acc660a..7ed8ed49266f 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -62,7 +62,8 @@ struct ovl_d_feature {
#define OVL_FEATURE_RO_COMPAT_SUPP (0)
#define OVL_FEATURE_RO_COMPAT_UNKNOWN (~OVL_FEATURE_RO_COMPAT_SUPP)
-#define OVL_FEATURE_INCOMPAT_SUPP (0)
+#define OVL_FEATURE_INCOMPAT_REDIRECT_DIR (1 << 0)
+#define OVL_FEATURE_INCOMPAT_SUPP (OVL_FEATURE_INCOMPAT_REDIRECT_DIR)
#define OVL_FEATURE_INCOMPAT_UNKNOWN (~OVL_FEATURE_INCOMPAT_SUPP)
static inline bool ovl_has_unknown_compat_features(struct ovl_layer *layer)
@@ -129,6 +130,8 @@ static inline int ovl_set_feature_##name(struct ovl_fs *ofs) \
OVL_FEATURE_INCOMPAT_##flagname); \
} \
+OVL_FEATURE_INCOMPAT_FUNCS(redirect_dir, REDIRECT_DIR)
+
/*
* The tuple (fh,uuid) is a universal unique identifier for a copy up origin,
* where:
--
2.13.6
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [RFC PATCH v2 5/9] ovl: add redirect dir feature when set redirect xattr to dir
2018-07-31 9:37 ` [RFC PATCH v2 5/9] ovl: add redirect dir feature when set redirect xattr to dir zhangyi (F)
@ 2018-08-01 11:03 ` Amir Goldstein
2018-08-03 11:52 ` zhangyi (F)
0 siblings, 1 reply; 26+ messages in thread
From: Amir Goldstein @ 2018-08-01 11:03 UTC (permalink / raw)
To: zhangyi (F); +Cc: overlayfs, Miklos Szeredi, Miao Xie, yangerkun
On Tue, Jul 31, 2018 at 12:37 PM, zhangyi (F) <yi.zhang@huawei.com> wrote:
> Redirect dir feature is backward incompatible because old kernel (which
> not support this feature) may corrupt the merge relationship between
> directories when change the directory which have redirect xattr.
>
> This patch check and set the upper layer's redirect dir feature when
> kernel set redirect xattr to directory in the upper layer.
>
> Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
> ---
> fs/overlayfs/dir.c | 11 +++++++++++
> fs/overlayfs/overlayfs.h | 5 ++++-
> 2 files changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
> index f480b1a2cd2e..238999e5f47b 100644
> --- a/fs/overlayfs/dir.c
> +++ b/fs/overlayfs/dir.c
> @@ -969,6 +969,7 @@ static int ovl_rename(struct inode *olddir, struct dentry *old,
> bool is_dir = d_is_dir(old);
> bool new_is_dir = d_is_dir(new);
> bool samedir = olddir == newdir;
> + struct ovl_fs* ofs = old->d_sb->s_fs_info;
> struct dentry *opaquedir = NULL;
> const struct cred *old_cred = NULL;
> LIST_HEAD(list);
> @@ -1061,6 +1062,16 @@ static int ovl_rename(struct inode *olddir, struct dentry *old,
> }
> }
>
> + if ((is_dir && ovl_type_merge_or_lower(old)) ||
> + (!overwrite && new_is_dir && ovl_type_merge_or_lower(new))) {
> + /*
> + * Set redirect dir feature to the upper root dir if this
> + * feature doesn't exist.
> + */
> + if (!ovl_has_feature_redirect_dir(ofs->upper_layer))
> + err = ovl_set_feature_redirect_dir(ofs);
> + }
> +
Please just place a call to ovl_set_feature_redirect_dir(ofs)
inside ovl_set_redirect(). There is no need to check all those
conditions.
Also, there is no need to check ovl_has_feature_redirect_dir()
as ovl_set_feature() already checks for existing bit.
Thanks,
Amir.
> trap = lock_rename(new_upperdir, old_upperdir);
>
> olddentry = lookup_one_len(old->d_name.name, old_upperdir,
> diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
> index d6231acc660a..7ed8ed49266f 100644
> --- a/fs/overlayfs/overlayfs.h
> +++ b/fs/overlayfs/overlayfs.h
> @@ -62,7 +62,8 @@ struct ovl_d_feature {
> #define OVL_FEATURE_RO_COMPAT_SUPP (0)
> #define OVL_FEATURE_RO_COMPAT_UNKNOWN (~OVL_FEATURE_RO_COMPAT_SUPP)
>
> -#define OVL_FEATURE_INCOMPAT_SUPP (0)
> +#define OVL_FEATURE_INCOMPAT_REDIRECT_DIR (1 << 0)
> +#define OVL_FEATURE_INCOMPAT_SUPP (OVL_FEATURE_INCOMPAT_REDIRECT_DIR)
> #define OVL_FEATURE_INCOMPAT_UNKNOWN (~OVL_FEATURE_INCOMPAT_SUPP)
>
> static inline bool ovl_has_unknown_compat_features(struct ovl_layer *layer)
> @@ -129,6 +130,8 @@ static inline int ovl_set_feature_##name(struct ovl_fs *ofs) \
> OVL_FEATURE_INCOMPAT_##flagname); \
> } \
>
> +OVL_FEATURE_INCOMPAT_FUNCS(redirect_dir, REDIRECT_DIR)
> +
> /*
> * The tuple (fh,uuid) is a universal unique identifier for a copy up origin,
> * where:
> --
> 2.13.6
>
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [RFC PATCH v2 5/9] ovl: add redirect dir feature when set redirect xattr to dir
2018-08-01 11:03 ` Amir Goldstein
@ 2018-08-03 11:52 ` zhangyi (F)
2018-08-03 20:38 ` Amir Goldstein
0 siblings, 1 reply; 26+ messages in thread
From: zhangyi (F) @ 2018-08-03 11:52 UTC (permalink / raw)
To: Amir Goldstein; +Cc: overlayfs, Miklos Szeredi, Miao Xie, yangerkun
On 2018/8/1 19:03, Amir Goldstein Wrote:
> On Tue, Jul 31, 2018 at 12:37 PM, zhangyi (F) <yi.zhang@huawei.com> wrote:
>> Redirect dir feature is backward incompatible because old kernel (which
>> not support this feature) may corrupt the merge relationship between
>> directories when change the directory which have redirect xattr.
>>
>> This patch check and set the upper layer's redirect dir feature when
>> kernel set redirect xattr to directory in the upper layer.
>>
>> Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
>> ---
>> fs/overlayfs/dir.c | 11 +++++++++++
>> fs/overlayfs/overlayfs.h | 5 ++++-
>> 2 files changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
>> index f480b1a2cd2e..238999e5f47b 100644
>> --- a/fs/overlayfs/dir.c
>> +++ b/fs/overlayfs/dir.c
>> @@ -969,6 +969,7 @@ static int ovl_rename(struct inode *olddir, struct dentry *old,
>> bool is_dir = d_is_dir(old);
>> bool new_is_dir = d_is_dir(new);
>> bool samedir = olddir == newdir;
>> + struct ovl_fs* ofs = old->d_sb->s_fs_info;
>> struct dentry *opaquedir = NULL;
>> const struct cred *old_cred = NULL;
>> LIST_HEAD(list);
>> @@ -1061,6 +1062,16 @@ static int ovl_rename(struct inode *olddir, struct dentry *old,
>> }
>> }
>>
>> + if ((is_dir && ovl_type_merge_or_lower(old)) ||
>> + (!overwrite && new_is_dir && ovl_type_merge_or_lower(new))) {
>> + /*
>> + * Set redirect dir feature to the upper root dir if this
>> + * feature doesn't exist.
>> + */
>> + if (!ovl_has_feature_redirect_dir(ofs->upper_layer))
>> + err = ovl_set_feature_redirect_dir(ofs);
>> + }
>> +
>
> Please just place a call to ovl_set_feature_redirect_dir(ofs)
> inside ovl_set_redirect(). There is no need to check all those
> conditions.
> Also, there is no need to check ovl_has_feature_redirect_dir()
> as ovl_set_feature() already checks for existing bit.
>
I understand check those condition is not perfect, but place it
to ovl_set_redirect() will lead to inode AA dead lock if the
parent dir of the renamed dir is the upper root dir, because
lock_rename() and vfs_setxattr() will get the same inode lock.
ovl_rename()
->lock_rename(upperdir) //get the upper root inode lock
->ovl_set_redirect()
->ovl_set_feature_redirect_dir()
->vfs_setxattr(upperdir) //dead lock in inode_lock()
Thanks,
Yi.
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [RFC PATCH v2 5/9] ovl: add redirect dir feature when set redirect xattr to dir
2018-08-03 11:52 ` zhangyi (F)
@ 2018-08-03 20:38 ` Amir Goldstein
0 siblings, 0 replies; 26+ messages in thread
From: Amir Goldstein @ 2018-08-03 20:38 UTC (permalink / raw)
To: zhangyi (F); +Cc: overlayfs, Miklos Szeredi, Miao Xie, yangerkun, Vivek Goyal
[-- Attachment #1: Type: text/plain, Size: 2937 bytes --]
On Fri, Aug 3, 2018, 1:52 PM zhangyi (F) <yi.zhang@huawei.com> wrote:
> On 2018/8/1 19:03, Amir Goldstein Wrote:
> > On Tue, Jul 31, 2018 at 12:37 PM, zhangyi (F) <yi.zhang@huawei.com>
> wrote:
> >> Redirect dir feature is backward incompatible because old kernel (which
> >> not support this feature) may corrupt the merge relationship between
> >> directories when change the directory which have redirect xattr.
> >>
> >> This patch check and set the upper layer's redirect dir feature when
> >> kernel set redirect xattr to directory in the upper layer.
> >>
> >> Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
> >> ---
> >> fs/overlayfs/dir.c | 11 +++++++++++
> >> fs/overlayfs/overlayfs.h | 5 ++++-
> >> 2 files changed, 15 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
> >> index f480b1a2cd2e..238999e5f47b 100644
> >> --- a/fs/overlayfs/dir.c
> >> +++ b/fs/overlayfs/dir.c
> >> @@ -969,6 +969,7 @@ static int ovl_rename(struct inode *olddir, struct
> dentry *old,
> >> bool is_dir = d_is_dir(old);
> >> bool new_is_dir = d_is_dir(new);
> >> bool samedir = olddir == newdir;
> >> + struct ovl_fs* ofs = old->d_sb->s_fs_info;
> >> struct dentry *opaquedir = NULL;
> >> const struct cred *old_cred = NULL;
> >> LIST_HEAD(list);
> >> @@ -1061,6 +1062,16 @@ static int ovl_rename(struct inode *olddir,
> struct dentry *old,
> >> }
> >> }
> >>
> >> + if ((is_dir && ovl_type_merge_or_lower(old)) ||
> >> + (!overwrite && new_is_dir && ovl_type_merge_or_lower(new)))
> {
> >> + /*
> >> + * Set redirect dir feature to the upper root dir if
> this
> >> + * feature doesn't exist.
> >> + */
> >> + if (!ovl_has_feature_redirect_dir(ofs->upper_layer))
> >> + err = ovl_set_feature_redirect_dir(ofs);
> >> + }
> >> +
> >
> > Please just place a call to ovl_set_feature_redirect_dir(ofs)
> > inside ovl_set_redirect(). There is no need to check all those
> > conditions.
> > Also, there is no need to check ovl_has_feature_redirect_dir()
> > as ovl_set_feature() already checks for existing bit.
> >
>
> I understand check those condition is not perfect, but place it
> to ovl_set_redirect() will lead to inode AA dead lock if the
> parent dir of the renamed dir is the upper root dir, because
> lock_rename() and vfs_setxattr() will get the same inode lock.
>
> ovl_rename()
> ->lock_rename(upperdir) //get the upper root inode lock
> ->ovl_set_redirect()
> ->ovl_set_feature_redirect_dir()
> ->vfs_setxattr(upperdir) //dead lock in inode_lock()
>
Very well, so you can move ovl_set_redirect before lock_rename()
I'm pretty sure that vivek already posted a patch like this in one of the
revisions of metacopy, but dropped it because it wasn't a must.
Thanks,
Amir.
[-- Attachment #2: Type: text/html, Size: 4133 bytes --]
^ permalink raw reply [flat|nested] 26+ messages in thread
* [RFC PATCH v2 6/9] ovl: add index feature if index dir exists
2018-07-31 9:37 [RFC PATCH v2 0/9] ovl: add feature set support zhangyi (F)
` (4 preceding siblings ...)
2018-07-31 9:37 ` [RFC PATCH v2 5/9] ovl: add redirect dir feature when set redirect xattr to dir zhangyi (F)
@ 2018-07-31 9:37 ` zhangyi (F)
2018-08-01 11:15 ` Amir Goldstein
2018-07-31 9:37 ` [RFC PATCH v2 7/9] ovl: add nfs_export feature when nfs_export is enabled zhangyi (F)
` (2 subsequent siblings)
8 siblings, 1 reply; 26+ messages in thread
From: zhangyi (F) @ 2018-07-31 9:37 UTC (permalink / raw)
To: linux-unionfs; +Cc: miklos, amir73il, yi.zhang, miaoxie, yangerkun
Index feature is backward read-only compatible becasue it only affect
the upper layer, and old kernel (which not support this feature) may
corrupt the relationship between files in upper dir and index dir when
change the copied-up linked files.
If the index feature is enabled, kernel will create an index dir in the
work base dir, so this patch check and set upper layer's index feature
when overlayfs get or create the index dir.
Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
---
fs/overlayfs/overlayfs.h | 4 +++-
fs/overlayfs/super.c | 11 +++++++++--
2 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index 7ed8ed49266f..21a41c12168c 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -59,7 +59,8 @@ struct ovl_d_feature {
#define OVL_FEATURE_COMPAT_SUPP (0)
#define OVL_FEATURE_COMPAT_UNKNOWN (~OVL_FEATURE_COMPAT_SUPP)
-#define OVL_FEATURE_RO_COMPAT_SUPP (0)
+#define OVL_FEATURE_RO_COMPAT_INDEX (1 << 0)
+#define OVL_FEATURE_RO_COMPAT_SUPP (OVL_FEATURE_RO_COMPAT_INDEX)
#define OVL_FEATURE_RO_COMPAT_UNKNOWN (~OVL_FEATURE_RO_COMPAT_SUPP)
#define OVL_FEATURE_INCOMPAT_REDIRECT_DIR (1 << 0)
@@ -130,6 +131,7 @@ static inline int ovl_set_feature_##name(struct ovl_fs *ofs) \
OVL_FEATURE_INCOMPAT_##flagname); \
} \
+OVL_FEATURE_RO_COMPAT_FUNCS(index, INDEX)
OVL_FEATURE_INCOMPAT_FUNCS(redirect_dir, REDIRECT_DIR)
/*
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index a5bbbddf741c..197329f7d284 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -1443,8 +1443,15 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
if (err)
goto out_free_oe;
- /* Force r/o mount with no index dir */
- if (!ofs->indexdir) {
+ if (ofs->indexdir) {
+ /* Set index feature if index dir exists */
+ if (!ovl_has_feature_index(ofs->upper_layer)) {
+ err = ovl_set_feature_index(ofs);
+ if (err)
+ goto out_free_oe;
+ }
+ } else {
+ /* Force r/o mount with no index dir */
dput(ofs->workdir);
ofs->workdir = NULL;
sb->s_flags |= SB_RDONLY;
--
2.13.6
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [RFC PATCH v2 6/9] ovl: add index feature if index dir exists
2018-07-31 9:37 ` [RFC PATCH v2 6/9] ovl: add index feature if index dir exists zhangyi (F)
@ 2018-08-01 11:15 ` Amir Goldstein
0 siblings, 0 replies; 26+ messages in thread
From: Amir Goldstein @ 2018-08-01 11:15 UTC (permalink / raw)
To: zhangyi (F); +Cc: overlayfs, Miklos Szeredi, Miao Xie, yangerkun
On Tue, Jul 31, 2018 at 12:37 PM, zhangyi (F) <yi.zhang@huawei.com> wrote:
> Index feature is backward read-only compatible becasue it only affect
> the upper layer, and old kernel (which not support this feature) may
> corrupt the relationship between files in upper dir and index dir when
> change the copied-up linked files.
>
> If the index feature is enabled, kernel will create an index dir in the
> work base dir, so this patch check and set upper layer's index feature
> when overlayfs get or create the index dir.
>
> Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
> ---
> fs/overlayfs/overlayfs.h | 4 +++-
> fs/overlayfs/super.c | 11 +++++++++--
> 2 files changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
> index 7ed8ed49266f..21a41c12168c 100644
> --- a/fs/overlayfs/overlayfs.h
> +++ b/fs/overlayfs/overlayfs.h
> @@ -59,7 +59,8 @@ struct ovl_d_feature {
> #define OVL_FEATURE_COMPAT_SUPP (0)
> #define OVL_FEATURE_COMPAT_UNKNOWN (~OVL_FEATURE_COMPAT_SUPP)
>
> -#define OVL_FEATURE_RO_COMPAT_SUPP (0)
> +#define OVL_FEATURE_RO_COMPAT_INDEX (1 << 0)
> +#define OVL_FEATURE_RO_COMPAT_SUPP (OVL_FEATURE_RO_COMPAT_INDEX)
> #define OVL_FEATURE_RO_COMPAT_UNKNOWN (~OVL_FEATURE_RO_COMPAT_SUPP)
>
> #define OVL_FEATURE_INCOMPAT_REDIRECT_DIR (1 << 0)
> @@ -130,6 +131,7 @@ static inline int ovl_set_feature_##name(struct ovl_fs *ofs) \
> OVL_FEATURE_INCOMPAT_##flagname); \
> } \
>
> +OVL_FEATURE_RO_COMPAT_FUNCS(index, INDEX)
> OVL_FEATURE_INCOMPAT_FUNCS(redirect_dir, REDIRECT_DIR)
>
> /*
> diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> index a5bbbddf741c..197329f7d284 100644
> --- a/fs/overlayfs/super.c
> +++ b/fs/overlayfs/super.c
> @@ -1443,8 +1443,15 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
> if (err)
> goto out_free_oe;
>
> - /* Force r/o mount with no index dir */
> - if (!ofs->indexdir) {
> + if (ofs->indexdir) {
> + /* Set index feature if index dir exists */
> + if (!ovl_has_feature_index(ofs->upper_layer)) {
> + err = ovl_set_feature_index(ofs);
ovl_set_feature_index() is enough. it should test the bit internally.
Thanks,
Amir.
^ permalink raw reply [flat|nested] 26+ messages in thread
* [RFC PATCH v2 7/9] ovl: add nfs_export feature when nfs_export is enabled
2018-07-31 9:37 [RFC PATCH v2 0/9] ovl: add feature set support zhangyi (F)
` (5 preceding siblings ...)
2018-07-31 9:37 ` [RFC PATCH v2 6/9] ovl: add index feature if index dir exists zhangyi (F)
@ 2018-07-31 9:37 ` zhangyi (F)
2018-08-01 11:20 ` Amir Goldstein
2018-07-31 9:37 ` [RFC PATCH v2 8/9] ovl: force mount underlying layers which have feature sets zhangyi (F)
2018-07-31 9:37 ` [RFC PATCH v2 9/9] ovl: check redirect_dir feature when detect redirect xattr on dir zhangyi (F)
8 siblings, 1 reply; 26+ messages in thread
From: zhangyi (F) @ 2018-07-31 9:37 UTC (permalink / raw)
To: linux-unionfs; +Cc: miklos, amir73il, yi.zhang, miaoxie, yangerkun
nfs_export feature is base on index feature and also backward read-only
compatible. Check the upper layer's nfs_export feature when index dir
exists and nfs_export was enabled through mount option, set this feature
if it was not yet set.
Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
---
fs/overlayfs/overlayfs.h | 5 ++++-
fs/overlayfs/super.c | 11 +++++++++++
2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index 21a41c12168c..0f95e60cae69 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -60,7 +60,9 @@ struct ovl_d_feature {
#define OVL_FEATURE_COMPAT_UNKNOWN (~OVL_FEATURE_COMPAT_SUPP)
#define OVL_FEATURE_RO_COMPAT_INDEX (1 << 0)
-#define OVL_FEATURE_RO_COMPAT_SUPP (OVL_FEATURE_RO_COMPAT_INDEX)
+#define OVL_FEATURE_RO_COMPAT_NFS_EXPORT (1 << 1)
+#define OVL_FEATURE_RO_COMPAT_SUPP (OVL_FEATURE_RO_COMPAT_INDEX | \
+ OVL_FEATURE_RO_COMPAT_NFS_EXPORT)
#define OVL_FEATURE_RO_COMPAT_UNKNOWN (~OVL_FEATURE_RO_COMPAT_SUPP)
#define OVL_FEATURE_INCOMPAT_REDIRECT_DIR (1 << 0)
@@ -132,6 +134,7 @@ static inline int ovl_set_feature_##name(struct ovl_fs *ofs) \
} \
OVL_FEATURE_RO_COMPAT_FUNCS(index, INDEX)
+OVL_FEATURE_RO_COMPAT_FUNCS(nfs_export, NFS_EXPORT)
OVL_FEATURE_INCOMPAT_FUNCS(redirect_dir, REDIRECT_DIR)
/*
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index 197329f7d284..860a533ae5a9 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -1450,6 +1450,17 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
if (err)
goto out_free_oe;
}
+
+ /*
+ * Set nfs_export feature if index dir exists and
+ * nfs_export mount option enabled.
+ */
+ if (ofs->config.nfs_export &&
+ !ovl_has_feature_nfs_export(ofs->upper_layer)) {
+ err = ovl_set_feature_nfs_export(ofs);
+ if (err)
+ goto out_free_oe;
+ }
} else {
/* Force r/o mount with no index dir */
dput(ofs->workdir);
--
2.13.6
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [RFC PATCH v2 7/9] ovl: add nfs_export feature when nfs_export is enabled
2018-07-31 9:37 ` [RFC PATCH v2 7/9] ovl: add nfs_export feature when nfs_export is enabled zhangyi (F)
@ 2018-08-01 11:20 ` Amir Goldstein
0 siblings, 0 replies; 26+ messages in thread
From: Amir Goldstein @ 2018-08-01 11:20 UTC (permalink / raw)
To: zhangyi (F); +Cc: overlayfs, Miklos Szeredi, Miao Xie, yangerkun
On Tue, Jul 31, 2018 at 12:37 PM, zhangyi (F) <yi.zhang@huawei.com> wrote:
> nfs_export feature is base on index feature and also backward read-only
> compatible. Check the upper layer's nfs_export feature when index dir
> exists and nfs_export was enabled through mount option, set this feature
> if it was not yet set.
>
> Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
> ---
> fs/overlayfs/overlayfs.h | 5 ++++-
> fs/overlayfs/super.c | 11 +++++++++++
> 2 files changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
> index 21a41c12168c..0f95e60cae69 100644
> --- a/fs/overlayfs/overlayfs.h
> +++ b/fs/overlayfs/overlayfs.h
> @@ -60,7 +60,9 @@ struct ovl_d_feature {
> #define OVL_FEATURE_COMPAT_UNKNOWN (~OVL_FEATURE_COMPAT_SUPP)
>
> #define OVL_FEATURE_RO_COMPAT_INDEX (1 << 0)
> -#define OVL_FEATURE_RO_COMPAT_SUPP (OVL_FEATURE_RO_COMPAT_INDEX)
> +#define OVL_FEATURE_RO_COMPAT_NFS_EXPORT (1 << 1)
> +#define OVL_FEATURE_RO_COMPAT_SUPP (OVL_FEATURE_RO_COMPAT_INDEX | \
> + OVL_FEATURE_RO_COMPAT_NFS_EXPORT)
> #define OVL_FEATURE_RO_COMPAT_UNKNOWN (~OVL_FEATURE_RO_COMPAT_SUPP)
>
> #define OVL_FEATURE_INCOMPAT_REDIRECT_DIR (1 << 0)
> @@ -132,6 +134,7 @@ static inline int ovl_set_feature_##name(struct ovl_fs *ofs) \
> } \
>
> OVL_FEATURE_RO_COMPAT_FUNCS(index, INDEX)
> +OVL_FEATURE_RO_COMPAT_FUNCS(nfs_export, NFS_EXPORT)
> OVL_FEATURE_INCOMPAT_FUNCS(redirect_dir, REDIRECT_DIR)
>
> /*
> diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> index 197329f7d284..860a533ae5a9 100644
> --- a/fs/overlayfs/super.c
> +++ b/fs/overlayfs/super.c
> @@ -1450,6 +1450,17 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
> if (err)
> goto out_free_oe;
> }
> +
> + /*
> + * Set nfs_export feature if index dir exists and
> + * nfs_export mount option enabled.
> + */
> + if (ofs->config.nfs_export &&
> + !ovl_has_feature_nfs_export(ofs->upper_layer)) {
> + err = ovl_set_feature_nfs_export(ofs);
> + if (err)
> + goto out_free_oe;
> + }
With metacopy patches, nfs_export can be disabled after this point
and it is anyway nicer to set the bit just before enabling nfs export,
i.e.:
if (ofs->config.nfs_export) {
+ err = ovl_set_feature_nfs_export(ofs);
+ if (err)
+ goto out_free_oe;
sb->s_export_op = &ovl_export_operations;
}
Thanks,
Amir.
^ permalink raw reply [flat|nested] 26+ messages in thread
* [RFC PATCH v2 8/9] ovl: force mount underlying layers which have feature sets
2018-07-31 9:37 [RFC PATCH v2 0/9] ovl: add feature set support zhangyi (F)
` (6 preceding siblings ...)
2018-07-31 9:37 ` [RFC PATCH v2 7/9] ovl: add nfs_export feature when nfs_export is enabled zhangyi (F)
@ 2018-07-31 9:37 ` zhangyi (F)
[not found] ` <CAOQ4uxgZcwxENyf5STfNgL6juLoHvOBnWkORpifp0pv04oi8Ww@mail.gmail.com>
2018-07-31 9:37 ` [RFC PATCH v2 9/9] ovl: check redirect_dir feature when detect redirect xattr on dir zhangyi (F)
8 siblings, 1 reply; 26+ messages in thread
From: zhangyi (F) @ 2018-07-31 9:37 UTC (permalink / raw)
To: linux-unionfs; +Cc: miklos, amir73il, yi.zhang, miaoxie, yangerkun
Now, overlay try its best to add feature bitset to the upper layer's
root directory, but it still cannot guarantee the feature set are
consistent for compatibility check.
- Some features in the feature set xattr or the whole xattr may be
missing if some features have already been set by the old kernel.
- Although we don't set lower layer's feature set xattr directly
(it inherit from previous mount when it was upper layer), but the
whole feature set xattr may be lost or corrupt when user change the
underlying layers.
So, feature set xattr are optionally for overlayfs now, but we should
be able to ensure the consistency of feature set for backward
compatibility check accurately.
After we introduce mkfs.overlay and fsck.overlay tools, we can use
these tools to add, check and fix feature set xattr. So it's time to
add options to enforce kernel to mount overlayfs with the base on the
layers that must have feature set xattr, even if it is empty.
After this patch, we refer the underlying layer which have feature set
xattr as on-disk format v2, and the layer that don't have as on-disk
format v1. We introduce two Kconfig options: OVERLAY_FS_V2 and
OVERLAY_FS_UPPER_V2, and the counterpart module and mount options
"overlayfs_v2=<off/on/upper>".
If "OVERLAY_FS_V2=n" or "-o overlayfs_v2=off", feature set xattr is not
required, the underlying layers can be either on-disk format v1 or v2.
If "OVERLAY_FS_V2=y" or "-o overlayfs_v2=on", all underlying layers of
overlayfs must be on-disk format v2, kernel will refuse to mount if
not. If "OVELRAY_FS_UPPER_V2=y" or "-o overlayfs=upper", relax the
requirement for declaring lower layers feature set, only the upper
layer have feature set is enough (this is useful for the case of lower
layer is read-only).
Note that the feature set xattr create by old kernel before this patch
or if "OVERLAY_FS_V2=n" may inconsistent, fsck.overlay is required
before mount with "OVERLAY_FS_V2=y" or "OVERLAY_FS_UPPER_V2=y" next
time.
Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
Suggested-by: Amir Goldstein <amir73il@gmail.com>
---
fs/overlayfs/Kconfig | 40 ++++++++++++++++++++++++++++++++++++++++
fs/overlayfs/feature.c | 20 +++++++++++++++++++-
fs/overlayfs/ovl_entry.h | 7 +++++++
fs/overlayfs/super.c | 43 +++++++++++++++++++++++++++++++++++++++++++
4 files changed, 109 insertions(+), 1 deletion(-)
diff --git a/fs/overlayfs/Kconfig b/fs/overlayfs/Kconfig
index 9384164253ac..1d7f7bfa5165 100644
--- a/fs/overlayfs/Kconfig
+++ b/fs/overlayfs/Kconfig
@@ -103,3 +103,43 @@ config OVERLAY_FS_XINO_AUTO
For more information, see Documentation/filesystems/overlayfs.txt
If unsure, say N.
+
+config OVERLAY_FS_V2
+ bool "Overlayfs: overlayfs v2 (force all layers have feature sets)"
+ default n
+ depends on OVERLAY_FS
+ help
+ If this config option is enabled then overlay feature sets are
+ necessary for each underlying layer, which is created by mkfs.overlay
+ or fsck.overlay. In this case it is still possible to turn off with
+ the "overlayfs_v2=off" module option or on a filesystem instance
+ basis with the "overlayfs_v2=off" mount option.
+
+ Note, kernel will refuse to mount overlayfs without feature set in
+ any one of the underlying layers, so must run mkfs.overlay before
+ the first mount or run fsck.overlay to tune overlayfs image form v1
+ to v2 (init feature sets).
+
+ For more information, see Documentation/filesystems/overlayfs.txt
+
+ If unsure or don't have the mkfs.overlay and mkfs.overlay tools,
+ say N.
+
+config OVERLAY_FS_UPPER_V2
+ bool "Overlayfs: upper v2 only (release requirement of the lower layers)"
+ default n
+ depends on OVERLAY_FS_V2
+ help
+ If this config option is enabled then overlay feature sets are
+ necessary for the upper layer only, feature sets are optional for
+ each lower layer. This option is useful for read-only lower layer
+ which cannot init feature set by mkfs.overlay and fsck.overlay.
+
+ Note, the lower layers may contain unmarked incompatible features,
+ mounting an overlay with these lower layers on a kernel that doesn't
+ support these features will have unexpected results.
+
+ For more information, see Documentation/filesystems/overlayfs.txt
+
+ If unsure or don't have the mkfs.overlay and mkfs.overlay tools,
+ say N.
diff --git a/fs/overlayfs/feature.c b/fs/overlayfs/feature.c
index 7c66e71bdd98..fffa79ee9b15 100644
--- a/fs/overlayfs/feature.c
+++ b/fs/overlayfs/feature.c
@@ -117,6 +117,10 @@ int ovl_set_upper_feature(struct ovl_fs *ofs,
if (!upper_layer)
return -EINVAL;
+ if (WARN_ON_ONCE((ofs->config.format != OVL_FS_V1) &&
+ !(upper_layer->feature)))
+ return -ESTALE;
+
switch (type) {
case OVL_FEATURE_COMPAT:
features = &upper_layer->compat;
@@ -148,7 +152,9 @@ int ovl_set_upper_feature(struct ovl_fs *ofs,
}
/*
- * Get features from each underlying root dir.
+ * Get features from each underlying root dir. Feature set is not
+ * necessary for v1 underlying layers, but is necessary for v2
+ * underlying layers.
*
* @ofs: overlay filesystem information
* @oe: overlay lower stack
@@ -175,6 +181,12 @@ int ovl_init_feature_set(struct ovl_fs *ofs, struct ovl_entry *oe)
upper_layer->feature = true;
kfree(odf);
} else {
+ /* Force upper on-disk format v2 */
+ if (ofs->config.format != OVL_FS_V1) {
+ pr_warn("overlayfs: upper layer feature set missing, "
+ "running fsck.overlay is recommended\n");
+ return -ESTALE;
+ }
upper_layer->feature = false;
}
}
@@ -189,6 +201,12 @@ int ovl_init_feature_set(struct ovl_fs *ofs, struct ovl_entry *oe)
return PTR_ERR(odf);
if (!odf) {
+ /* Force lower on-disk format v2 */
+ if (ofs->config.format == OVL_FS_V2) {
+ pr_warn("overlayfs: lower layer %i feature set missing, "
+ "running fsck.overlay is recommended\n", i);
+ return -ESTALE;
+ }
lower_layer->feature = false;
continue;
}
diff --git a/fs/overlayfs/ovl_entry.h b/fs/overlayfs/ovl_entry.h
index 8a28e24dd149..19885be7705c 100644
--- a/fs/overlayfs/ovl_entry.h
+++ b/fs/overlayfs/ovl_entry.h
@@ -8,6 +8,12 @@
* the Free Software Foundation.
*/
+enum ovl_format {
+ OVL_FS_V1,
+ OVL_FS_UPPER_V2,
+ OVL_FS_V2,
+};
+
struct ovl_config {
char *lowerdir;
char *upperdir;
@@ -19,6 +25,7 @@ struct ovl_config {
bool index;
bool nfs_export;
int xino;
+ enum ovl_format format;
};
struct ovl_sb {
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index 860a533ae5a9..1e81d2c6766a 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -56,6 +56,16 @@ module_param_named(xino_auto, ovl_xino_auto_def, bool, 0644);
MODULE_PARM_DESC(ovl_xino_auto_def,
"Auto enable xino feature");
+static bool ovl_ovlfs_v2_def = IS_ENABLED(CONFIG_OVERLAY_FS_V2);
+module_param_named(ovl_ovlfs_v2, ovl_ovlfs_v2_def, bool, 0644);
+MODULE_PARM_DESC(ovl_ovlfs_v2_def,
+ "Default to on or off to force mount overlay v2 layers");
+
+static bool ovl_ovlfs_upper_v2_def = IS_ENABLED(CONFIG_OVERLAY_FS_UPPER_V2);
+module_param_named(ovl_ovlfs_upper_v2, ovl_ovlfs_upper_v2_def, bool, 0644);
+MODULE_PARM_DESC(ovl_ovlfs_upper_v2_def,
+ "Force mount overlay v2 upper layer only");
+
static void ovl_entry_stack_free(struct ovl_entry *oe)
{
unsigned int i;
@@ -351,6 +361,18 @@ static inline int ovl_xino_def(void)
return ovl_xino_auto_def ? OVL_XINO_AUTO : OVL_XINO_OFF;
}
+static const char * const ovl_format_str[] = {
+ "off", /* OVL_FS_V1 */
+ "upper", /* OVL_FS_UPPER_V2 */
+ "on", /* OVL_FS_V2 */
+};
+
+static inline int ovl_format_def(void)
+{
+ return !ovl_ovlfs_v2_def ? OVL_FS_V1 :
+ (ovl_ovlfs_upper_v2_def ? OVL_FS_UPPER_V2 : OVL_FS_V2);
+}
+
/**
* ovl_show_options
*
@@ -378,6 +400,8 @@ static int ovl_show_options(struct seq_file *m, struct dentry *dentry)
"on" : "off");
if (ofs->config.xino != ovl_xino_def())
seq_printf(m, ",xino=%s", ovl_xino_str[ofs->config.xino]);
+ if (ofs->config.format != ovl_format_def())
+ seq_printf(m, ",overlayfs_v2=%s", ovl_format_str[ofs->config.format]);
return 0;
}
@@ -416,6 +440,9 @@ enum {
OPT_XINO_ON,
OPT_XINO_OFF,
OPT_XINO_AUTO,
+ OPT_OVERLAYFS_V2_ON,
+ OPT_OVERLAYFS_V2_UPPER,
+ OPT_OVERLAYFS_V2_OFF,
OPT_ERR,
};
@@ -432,6 +459,9 @@ static const match_table_t ovl_tokens = {
{OPT_XINO_ON, "xino=on"},
{OPT_XINO_OFF, "xino=off"},
{OPT_XINO_AUTO, "xino=auto"},
+ {OPT_OVERLAYFS_V2_ON, "overlayfs_v2=on"},
+ {OPT_OVERLAYFS_V2_UPPER, "overlayfs_v2=upper"},
+ {OPT_OVERLAYFS_V2_OFF, "overlayfs_v2=off"},
{OPT_ERR, NULL}
};
@@ -558,6 +588,18 @@ static int ovl_parse_opt(char *opt, struct ovl_config *config)
config->xino = OVL_XINO_AUTO;
break;
+ case OPT_OVERLAYFS_V2_ON:
+ config->format = OVL_FS_V2;
+ break;
+
+ case OPT_OVERLAYFS_V2_UPPER:
+ config->format = OVL_FS_UPPER_V2;
+ break;
+
+ case OPT_OVERLAYFS_V2_OFF:
+ config->format = OVL_FS_V1;
+ break;
+
default:
pr_err("overlayfs: unrecognized mount option \"%s\" or missing value\n", p);
return -EINVAL;
@@ -1386,6 +1428,7 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
ofs->config.index = ovl_index_def;
ofs->config.nfs_export = ovl_nfs_export_def;
ofs->config.xino = ovl_xino_def();
+ ofs->config.format = ovl_format_def();
err = ovl_parse_opt((char *) data, &ofs->config);
if (err)
goto out_err;
--
2.13.6
^ permalink raw reply related [flat|nested] 26+ messages in thread* [RFC PATCH v2 9/9] ovl: check redirect_dir feature when detect redirect xattr on dir
2018-07-31 9:37 [RFC PATCH v2 0/9] ovl: add feature set support zhangyi (F)
` (7 preceding siblings ...)
2018-07-31 9:37 ` [RFC PATCH v2 8/9] ovl: force mount underlying layers which have feature sets zhangyi (F)
@ 2018-07-31 9:37 ` zhangyi (F)
2018-08-01 11:30 ` Amir Goldstein
8 siblings, 1 reply; 26+ messages in thread
From: zhangyi (F) @ 2018-07-31 9:37 UTC (permalink / raw)
To: linux-unionfs; +Cc: miklos, amir73il, yi.zhang, miaoxie, yangerkun
After we introduce the fsck.overlay program, we could check and fix
the inconsistency of feature set xattr, so it's a good opportunity
check the redirect dir feature when we detect redirect xattr on
directory in the underlying layer. If the feature is necessary
(OVL_FS_V2 for all layers and OVL_FS_UPPER_V2 for upper layer) but
the redirect dir feature is missing, the feature set is inconsistent
and fsck.overlay is required.
Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
---
fs/overlayfs/feature.c | 11 +++++++++++
fs/overlayfs/namei.c | 33 ++++++++++++++++++++++++++++++++-
fs/overlayfs/overlayfs.h | 1 +
3 files changed, 44 insertions(+), 1 deletion(-)
diff --git a/fs/overlayfs/feature.c b/fs/overlayfs/feature.c
index fffa79ee9b15..ed9830ab7ae1 100644
--- a/fs/overlayfs/feature.c
+++ b/fs/overlayfs/feature.c
@@ -285,3 +285,14 @@ bool ovl_check_feature_ok(struct ovl_fs *ofs, bool readonly)
return true;
}
+
+/*
+ * Feature set on upper layer is necessary when OVL_FS_V2 and
+ * OVL_FS_UPPER_V2, feautre sets on lower layers are necessary
+ * when OVL_FS_V2.
+ */
+bool ovl_feature_necessary(struct ovl_fs *ofs, struct ovl_layer *layer)
+{
+ return (ofs->config.format == OVL_FS_UPPER_V2) ?
+ (layer->idx == 0) : (ofs->config.format == OVL_FS_V2);
+}
diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
index bb2d347c0a7e..160c87734092 100644
--- a/fs/overlayfs/namei.c
+++ b/fs/overlayfs/namei.c
@@ -24,6 +24,7 @@ struct ovl_lookup_data {
bool stop;
bool last;
char *redirect;
+ bool redirect_xattr;
};
static int ovl_check_redirect(struct dentry *dentry, struct ovl_lookup_data *d,
@@ -35,7 +36,7 @@ static int ovl_check_redirect(struct dentry *dentry, struct ovl_lookup_data *d,
res = vfs_getxattr(dentry, OVL_XATTR_REDIRECT, NULL, 0);
if (res < 0) {
if (res == -ENODATA || res == -EOPNOTSUPP)
- return 0;
+ goto out;
goto fail;
}
buf = kzalloc(prelen + res + strlen(post) + 1, GFP_KERNEL);
@@ -75,6 +76,7 @@ static int ovl_check_redirect(struct dentry *dentry, struct ovl_lookup_data *d,
strcat(buf, post);
kfree(d->redirect);
+ d->redirect_xattr = true;
d->redirect = buf;
d->name.name = d->redirect;
d->name.len = strlen(d->redirect);
@@ -83,6 +85,8 @@ static int ovl_check_redirect(struct dentry *dentry, struct ovl_lookup_data *d,
err_free:
kfree(buf);
+out:
+ d->redirect_xattr = false;
return 0;
fail:
pr_warn_ratelimited("overlayfs: failed to get redirect (%i)\n", res);
@@ -841,6 +845,7 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
.stop = false,
.last = ofs->config.redirect_follow ? false : !poe->numlower,
.redirect = NULL,
+ .redirect_xattr = false,
};
if (dentry->d_name.len > ofs->namelen)
@@ -875,6 +880,19 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
goto out_put_upper;
}
+ /*
+ * Check redirect_dir feature when redirect xattr is detected on
+ * directory and feature set is required.
+ */
+ err = -ESTALE;
+ if (d.redirect_xattr && d.is_dir &&
+ ovl_feature_necessary(ofs, ofs->upper_layer) &&
+ !ovl_has_feature_redirect_dir(ofs->upper_layer)) {
+ pr_warn_ratelimited("overlayfs: redirect_dir feature missing, "
+ "running fsck.overlay is recommended\n");
+ goto out_put;
+ }
+
if (d.redirect) {
err = -ENOMEM;
upperredirect = kstrdup(d.redirect, GFP_KERNEL);
@@ -942,6 +960,19 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
ctr++;
/*
+ * Check redirect_dir feature when redirect xattr is detected on
+ * directory and feature set is required.
+ */
+ err = -ESTALE;
+ if (d.redirect_xattr && d.is_dir &&
+ ovl_feature_necessary(ofs, lower.layer) &&
+ !ovl_has_feature_redirect_dir(lower.layer)) {
+ pr_warn_ratelimited("overlayfs: redirect_dir feature missing, "
+ "running fsck.overlay is recommended\n");
+ goto out_put;
+ }
+
+ /*
* Following redirects can have security consequences: it's like
* a symlink into the lower layer without the permission checks.
* This is only a problem if the upper layer is untrusted (e.g
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index 0f95e60cae69..b3db106b8e22 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -477,3 +477,4 @@ extern const struct export_operations ovl_export_operations;
/* feature.c */
int ovl_init_feature_set(struct ovl_fs *ofs, struct ovl_entry *oe);
bool ovl_check_feature_ok(struct ovl_fs *ofs, bool readonly);
+bool ovl_feature_necessary(struct ovl_fs *ofs, struct ovl_layer *layer);
--
2.13.6
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [RFC PATCH v2 9/9] ovl: check redirect_dir feature when detect redirect xattr on dir
2018-07-31 9:37 ` [RFC PATCH v2 9/9] ovl: check redirect_dir feature when detect redirect xattr on dir zhangyi (F)
@ 2018-08-01 11:30 ` Amir Goldstein
2018-08-01 11:35 ` Amir Goldstein
0 siblings, 1 reply; 26+ messages in thread
From: Amir Goldstein @ 2018-08-01 11:30 UTC (permalink / raw)
To: zhangyi (F); +Cc: overlayfs, Miklos Szeredi, Miao Xie, yangerkun
On Tue, Jul 31, 2018 at 12:37 PM, zhangyi (F) <yi.zhang@huawei.com> wrote:
> After we introduce the fsck.overlay program, we could check and fix
> the inconsistency of feature set xattr, so it's a good opportunity
> check the redirect dir feature when we detect redirect xattr on
> directory in the underlying layer. If the feature is necessary
> (OVL_FS_V2 for all layers and OVL_FS_UPPER_V2 for upper layer) but
> the redirect dir feature is missing, the feature set is inconsistent
> and fsck.overlay is required.
>
> Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
> ---
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [RFC PATCH v2 9/9] ovl: check redirect_dir feature when detect redirect xattr on dir
2018-08-01 11:30 ` Amir Goldstein
@ 2018-08-01 11:35 ` Amir Goldstein
2018-08-02 12:30 ` Vivek Goyal
0 siblings, 1 reply; 26+ messages in thread
From: Amir Goldstein @ 2018-08-01 11:35 UTC (permalink / raw)
To: zhangyi (F); +Cc: overlayfs, Miklos Szeredi, Miao Xie, yangerkun, Vivek Goyal
On Wed, Aug 1, 2018 at 2:30 PM, Amir Goldstein <amir73il@gmail.com> wrote:
> On Tue, Jul 31, 2018 at 12:37 PM, zhangyi (F) <yi.zhang@huawei.com> wrote:
>> After we introduce the fsck.overlay program, we could check and fix
>> the inconsistency of feature set xattr, so it's a good opportunity
>> check the redirect dir feature when we detect redirect xattr on
>> directory in the underlying layer. If the feature is necessary
>> (OVL_FS_V2 for all layers and OVL_FS_UPPER_V2 for upper layer) but
>> the redirect dir feature is missing, the feature set is inconsistent
>> and fsck.overlay is required.
>>
>> Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
>> ---
The concept of this patch is fine, but I would really like to review
it only after rebase with metacopy, because there are pretty big changes
for lookup+redirecet in metacopy patches.
And I would love to get Vivek's feedback on this patch.
Also, it might be enough just to check the feature bit inside
ovl_check_redirect() and warn - I am not sure that failing the lookup
is called for.
Thanks,
Amir.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [RFC PATCH v2 9/9] ovl: check redirect_dir feature when detect redirect xattr on dir
2018-08-01 11:35 ` Amir Goldstein
@ 2018-08-02 12:30 ` Vivek Goyal
0 siblings, 0 replies; 26+ messages in thread
From: Vivek Goyal @ 2018-08-02 12:30 UTC (permalink / raw)
To: Amir Goldstein
Cc: zhangyi (F), overlayfs, Miklos Szeredi, Miao Xie, yangerkun
On Wed, Aug 01, 2018 at 02:35:27PM +0300, Amir Goldstein wrote:
> On Wed, Aug 1, 2018 at 2:30 PM, Amir Goldstein <amir73il@gmail.com> wrote:
> > On Tue, Jul 31, 2018 at 12:37 PM, zhangyi (F) <yi.zhang@huawei.com> wrote:
> >> After we introduce the fsck.overlay program, we could check and fix
> >> the inconsistency of feature set xattr, so it's a good opportunity
> >> check the redirect dir feature when we detect redirect xattr on
> >> directory in the underlying layer. If the feature is necessary
> >> (OVL_FS_V2 for all layers and OVL_FS_UPPER_V2 for upper layer) but
> >> the redirect dir feature is missing, the feature set is inconsistent
> >> and fsck.overlay is required.
> >>
> >> Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
> >> ---
>
> The concept of this patch is fine, but I would really like to review
> it only after rebase with metacopy, because there are pretty big changes
> for lookup+redirecet in metacopy patches.
> And I would love to get Vivek's feedback on this patch.
I am just trying to understand what's going on on this patch series.
Zhangyi, There are lots of details in first email but high level picture
is missing. It would help if there was a README or 2-3 paragraphs
explaining what's the problem and how you are solving it. I feel
description directly jumps to details of "how some problem is being
solved". While it works for the people who know about it, I am
scratching my head. Will be nice, if in next posting if you could
also describe, what's the problem you are solving.
>
> Also, it might be enough just to check the feature bit inside
> ovl_check_redirect() and warn - I am not sure that failing the lookup
> is called for.
Without knowing much, I agreed that failing lookup feels little
drastic. A warning that running fsck.overlay is recommended, sounds
more appropriate.
Also agreed that its easier to review this after metadata only copy
up patches are merged. ovl_lookup() and friends have changed significantly
and once this patch is rebased on top of that, its easier to review.
Thanks
Vivek
^ permalink raw reply [flat|nested] 26+ messages in thread