diff mbox

[1/3] ovl: check fs features

Message ID 1477380887-21333-2-git-send-email-mszeredi@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Miklos Szeredi Oct. 25, 2016, 7:34 a.m. UTC
To allow adding new, backward incompatible features to overlayfs, we need a
way to store the list of features in the overlay.  This is done via
"trusted.overlay.features" xattr on the root of the upper layer (or one of
the lower layers, that previously acted as an upper layer).  It's a comma
separated list of case sensitive strings.

If an overlay has an unknown feature, mount shall return an error.  So
mechanism should only be used for backward incompatible features.

This patch doesn't add any features.  If the "trusted.overlay.features"
xattr contains a non-empty list, then return EINVAL error for the mount.

Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
Cc: <stable@vger.kernel.org>
---
 Documentation/filesystems/overlayfs.txt | 12 ++++++++++
 fs/overlayfs/overlayfs.h                |  1 +
 fs/overlayfs/super.c                    | 41 +++++++++++++++++++++++++++++++++
 3 files changed, 54 insertions(+)

Comments

Amir Goldstein Oct. 25, 2016, 11:24 a.m. UTC | #1
On Tue, Oct 25, 2016 at 10:34 AM, Miklos Szeredi <mszeredi@redhat.com> wrote:
> To allow adding new, backward incompatible features to overlayfs, we need a
> way to store the list of features in the overlay.  This is done via
> "trusted.overlay.features" xattr on the root of the upper layer (or one of
> the lower layers, that previously acted as an upper layer).  It's a comma
> separated list of case sensitive strings.
>
> If an overlay has an unknown feature, mount shall return an error.  So
> mechanism should only be used for backward incompatible features.

So maybe be explicit and call the attribute trusted.overlay.incompat_features,
to allow future addition of compat and rocompat feature sets?

>
> This patch doesn't add any features.  If the "trusted.overlay.features"
> xattr contains a non-empty list, then return EINVAL error for the mount.
>
> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
> Cc: <stable@vger.kernel.org>
> ---
>  Documentation/filesystems/overlayfs.txt | 12 ++++++++++
>  fs/overlayfs/overlayfs.h                |  1 +
>  fs/overlayfs/super.c                    | 41 +++++++++++++++++++++++++++++++++
>  3 files changed, 54 insertions(+)
>
> diff --git a/Documentation/filesystems/overlayfs.txt b/Documentation/filesystems/overlayfs.txt
> index 7aeb8e8d80cf..5108425157ac 100644
> --- a/Documentation/filesystems/overlayfs.txt
> +++ b/Documentation/filesystems/overlayfs.txt
> @@ -175,6 +175,18 @@ The specified lower directories will be stacked beginning from the
>  rightmost one and going left.  In the above example lower1 will be the
>  top, lower2 the middle and lower3 the bottom layer.
>
> +Filesystem features
> +-------------------
> +
> +Features are enabled via "trusted.overlay.features" xattr on the root of the
> +upper layer.  E.g. the following command can be used to enable features "foo"
> +and "bar" on the overlay:
> +
> +  setfattr -n "trusted.overlay.features" -v "foo,bar" /upper
> +  mount -t overlay overlay -olowerdir=/lower,upperdir=/upper,\
> +workdir=/work /merged
> +
> +If an overlay has an unknown feature, mount shall return an error.
>
>  Non-standard behavior
>  ---------------------
> diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
> index f6e4d3539a25..d61d5b9d0d91 100644
> --- a/fs/overlayfs/overlayfs.h
> +++ b/fs/overlayfs/overlayfs.h
> @@ -19,6 +19,7 @@ enum ovl_path_type {
>
>  #define OVL_XATTR_PREFIX XATTR_TRUSTED_PREFIX "overlay."
>  #define OVL_XATTR_OPAQUE OVL_XATTR_PREFIX "opaque"
> +#define OVL_XATTR_FEATURES OVL_XATTR_PREFIX "features"
>
>  #define OVL_ISUPPER_MASK 1UL
>
> diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> index 30263a541fd5..d6dc8d905d00 100644
> --- a/fs/overlayfs/super.c
> +++ b/fs/overlayfs/super.c
> @@ -397,6 +397,39 @@ static struct dentry *ovl_workdir_create(struct vfsmount *mnt,
>         goto out_unlock;
>  }
>
> +static int ovl_check_features(struct dentry *root)
> +{
> +       int res;
> +       char *buf, *tmp, *p;
> +
> +       res = vfs_getxattr(root, OVL_XATTR_FEATURES, NULL, 0);
> +       if (res <= 0) {
> +               if (res == -EOPNOTSUPP || res == -ENODATA)
> +                       res = 0;
> +               return res;
> +       }
> +
> +       buf = kmalloc(res + 1, GFP_TEMPORARY);
> +       if (!buf)
> +               return -ENOMEM;
> +
> +       res = vfs_getxattr(root, OVL_XATTR_FEATURES, buf, res);
> +       if (res <= 0)
> +               goto out_free;
> +
> +       buf[res] = '\0';
> +       res = 0;
> +       tmp = buf;
> +       while ((p = strsep(&tmp, ",")) != NULL) {
> +               res = -EINVAL;
> +               pr_err("overlayfs: feature '%s' not supported\n", p);
> +       }
> +out_free:
> +       kfree(buf);
> +
> +       return res;
> +}
> +
>  static void ovl_unescape(char *s)
>  {
>         char *d = s;
> @@ -471,6 +504,10 @@ static int ovl_lower_dir(const char *name, struct path *path, long *namelen,
>         if (err)
>                 goto out;
>
> +       err = ovl_check_features(path->dentry);
> +       if (err)
> +               goto out_put;
> +
>         err = vfs_statfs(path, &statfs);
>         if (err) {
>                 pr_err("overlayfs: statfs failed on '%s'\n", name);
> @@ -693,6 +730,10 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
>                         goto out_put_upperpath;
>                 }
>
> +               err = ovl_check_features(upperpath.dentry);
> +               if (err)
> +                       goto out_put_upperpath;
> +
>                 err = ovl_mount_dir(ufs->config.workdir, &workpath);
>                 if (err)
>                         goto out_put_upperpath;
> --
> 2.5.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-unionfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Amir Goldstein Nov. 5, 2016, 8:40 p.m. UTC | #2
On Tue, Oct 25, 2016 at 2:24 PM, Amir Goldstein <amir73il@gmail.com> wrote:
> On Tue, Oct 25, 2016 at 10:34 AM, Miklos Szeredi <mszeredi@redhat.com> wrote:
>> To allow adding new, backward incompatible features to overlayfs, we need a
>> way to store the list of features in the overlay.  This is done via
>> "trusted.overlay.features" xattr on the root of the upper layer (or one of
>> the lower layers, that previously acted as an upper layer).  It's a comma
>> separated list of case sensitive strings.
>>
>> If an overlay has an unknown feature, mount shall return an error.  So
>> mechanism should only be used for backward incompatible features.
>
> So maybe be explicit and call the attribute trusted.overlay.incompat_features,
> to allow future addition of compat and rocompat feature sets?
>

On top of the proposed features xattr,
for the sake of being backward compatible with old kernels,
how about creating a file 3 levels down from work dir (e.g. /work/work/a/b/c)
This would cause old kernels to mount overlay read-only,
which is sufficient to keep them from corrupting the redirect structure.

And once again, I suggest learning from the elders fs, who have
successfully gone through many on-disk format upgrades,
and copy the design of the feature trio (compat/incompat/rocompat)

Amir.
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/filesystems/overlayfs.txt b/Documentation/filesystems/overlayfs.txt
index 7aeb8e8d80cf..5108425157ac 100644
--- a/Documentation/filesystems/overlayfs.txt
+++ b/Documentation/filesystems/overlayfs.txt
@@ -175,6 +175,18 @@  The specified lower directories will be stacked beginning from the
 rightmost one and going left.  In the above example lower1 will be the
 top, lower2 the middle and lower3 the bottom layer.
 
+Filesystem features
+-------------------
+
+Features are enabled via "trusted.overlay.features" xattr on the root of the
+upper layer.  E.g. the following command can be used to enable features "foo"
+and "bar" on the overlay:
+
+  setfattr -n "trusted.overlay.features" -v "foo,bar" /upper
+  mount -t overlay overlay -olowerdir=/lower,upperdir=/upper,\
+workdir=/work /merged
+
+If an overlay has an unknown feature, mount shall return an error.
 
 Non-standard behavior
 ---------------------
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index f6e4d3539a25..d61d5b9d0d91 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -19,6 +19,7 @@  enum ovl_path_type {
 
 #define OVL_XATTR_PREFIX XATTR_TRUSTED_PREFIX "overlay."
 #define OVL_XATTR_OPAQUE OVL_XATTR_PREFIX "opaque"
+#define OVL_XATTR_FEATURES OVL_XATTR_PREFIX "features"
 
 #define OVL_ISUPPER_MASK 1UL
 
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index 30263a541fd5..d6dc8d905d00 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -397,6 +397,39 @@  static struct dentry *ovl_workdir_create(struct vfsmount *mnt,
 	goto out_unlock;
 }
 
+static int ovl_check_features(struct dentry *root)
+{
+	int res;
+	char *buf, *tmp, *p;
+
+	res = vfs_getxattr(root, OVL_XATTR_FEATURES, NULL, 0);
+	if (res <= 0) {
+		if (res == -EOPNOTSUPP || res == -ENODATA)
+			res = 0;
+		return res;
+	}
+
+	buf = kmalloc(res + 1, GFP_TEMPORARY);
+	if (!buf)
+		return -ENOMEM;
+
+	res = vfs_getxattr(root, OVL_XATTR_FEATURES, buf, res);
+	if (res <= 0)
+		goto out_free;
+
+	buf[res] = '\0';
+	res = 0;
+	tmp = buf;
+	while ((p = strsep(&tmp, ",")) != NULL) {
+		res = -EINVAL;
+		pr_err("overlayfs: feature '%s' not supported\n", p);
+	}
+out_free:
+	kfree(buf);
+
+	return res;
+}
+
 static void ovl_unescape(char *s)
 {
 	char *d = s;
@@ -471,6 +504,10 @@  static int ovl_lower_dir(const char *name, struct path *path, long *namelen,
 	if (err)
 		goto out;
 
+	err = ovl_check_features(path->dentry);
+	if (err)
+		goto out_put;
+
 	err = vfs_statfs(path, &statfs);
 	if (err) {
 		pr_err("overlayfs: statfs failed on '%s'\n", name);
@@ -693,6 +730,10 @@  static int ovl_fill_super(struct super_block *sb, void *data, int silent)
 			goto out_put_upperpath;
 		}
 
+		err = ovl_check_features(upperpath.dentry);
+		if (err)
+			goto out_put_upperpath;
+
 		err = ovl_mount_dir(ufs->config.workdir, &workpath);
 		if (err)
 			goto out_put_upperpath;