[PATCH] ovl: reserve ability to reconfigure mount options with new mount api

[PATCH] ovl: reserve ability to reconfigure mount options with new mount api

[Christian Brauner]

Using the old mount api to remount an overlayfs superblock via
mount(MS_REMOUNT) all mount options will be silently ignored. For
example, if you create an overlayfs mount:

        mount -t overlay overlay -o lowerdir=/mnt/a:/mnt/b,upperdir=/mnt/upper,workdir=/mnt/work /mnt/merged

and then issue a remount via:

        # force mount(8) to use mount(2)
        export LIBMOUNT_FORCE_MOUNT2=always
        mount -t overlay overlay -o remount,WOOTWOOT,lowerdir=/DOESNT-EXIST /mnt/merged

with completely nonsensical mount options whatsoever it will succeed
nonetheless. This prevents us from every changing any mount options we
might introduce in the future that could reasonably be changed during a
remount.

We don't need to carry this issue into the new mount api port. Similar
to FUSE we can use the fs_context::oldapi member to figure out that this
is a request coming through the legacy mount api. If we detect it we
continue silently ignoring all mount options.

But for the new mount api we simply report that mount options cannot
currently be changed. This will allow us to potentially alter mount
properties for new or even old properties. It any case, silently
ignoring everything is not something new apis should do.

Signed-off-by: Christian Brauner <brauner@kernel.org>
---

---
 fs/overlayfs/super.c | 25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index ed4b35c9d647..c14c52560fd6 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -499,13 +499,24 @@ static int ovl_parse_param(struct fs_context *fc, struct fs_parameter *param)
 	struct ovl_fs_context *ctx = fc->fs_private;
 	int opt;
 
-	/*
-	 * On remount overlayfs has always ignored all mount options no
-	 * matter if malformed or not so for backwards compatibility we
-	 * do the same here.
-	 */
-	if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE)
-		return 0;
+	if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE) {
+		/*
+		 * On remount overlayfs has always ignored all mount
+		 * options no matter if malformed or not so for
+		 * backwards compatibility we do the same here.
+		 */
+		if (fc->oldapi)
+			return 0;
+
+		/*
+		 * Give us the freedom to allow changing mount options
+		 * with the new mount api in the future. So instead of
+		 * silently ignoring everything we report a proper
+		 * error. This is only visible for users of the new
+		 * mount api.
+		 */
+		return invalfc(fc, "No changes allowed in reconfigure");
+	}
 
 	opt = fs_parse(fc, ovl_parameter_spec, param, &result);
 	if (opt < 0)

---
base-commit: cc7e4d7ce5ea183b9ca735f7466b4491a1ee440e
change-id: 20230620-fs-overlayfs-mount-api-remount-93b210e6ccd8

Re: [PATCH] ovl: reserve ability to reconfigure mount options with new mount api

[Amir Goldstein]

On Tue, Jun 20, 2023 at 2:42 PM Christian Brauner <brauner@kernel.org> wrote:
>
> Using the old mount api to remount an overlayfs superblock via
> mount(MS_REMOUNT) all mount options will be silently ignored. For
> example, if you create an overlayfs mount:
>
>         mount -t overlay overlay -o lowerdir=/mnt/a:/mnt/b,upperdir=/mnt/upper,workdir=/mnt/work /mnt/merged
>
> and then issue a remount via:
>
>         # force mount(8) to use mount(2)
>         export LIBMOUNT_FORCE_MOUNT2=always
>         mount -t overlay overlay -o remount,WOOTWOOT,lowerdir=/DOESNT-EXIST /mnt/merged
>
> with completely nonsensical mount options whatsoever it will succeed
> nonetheless. This prevents us from every changing any mount options we
> might introduce in the future that could reasonably be changed during a
> remount.
>
> We don't need to carry this issue into the new mount api port. Similar
> to FUSE we can use the fs_context::oldapi member to figure out that this
> is a request coming through the legacy mount api. If we detect it we
> continue silently ignoring all mount options.
>
> But for the new mount api we simply report that mount options cannot
> currently be changed. This will allow us to potentially alter mount
> properties for new or even old properties. It any case, silently
> ignoring everything is not something new apis should do.
>
> Signed-off-by: Christian Brauner <brauner@kernel.org>
> ---
>

Looks good and tests passed.
I Applied this to overlayfs-next on my github.

Miklos,

Please update your branch.

Thanks,
Amir.

> ---
>  fs/overlayfs/super.c | 25 ++++++++++++++++++-------
>  1 file changed, 18 insertions(+), 7 deletions(-)
>
> diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> index ed4b35c9d647..c14c52560fd6 100644
> --- a/fs/overlayfs/super.c
> +++ b/fs/overlayfs/super.c
> @@ -499,13 +499,24 @@ static int ovl_parse_param(struct fs_context *fc, struct fs_parameter *param)
>         struct ovl_fs_context *ctx = fc->fs_private;
>         int opt;
>
> -       /*
> -        * On remount overlayfs has always ignored all mount options no
> -        * matter if malformed or not so for backwards compatibility we
> -        * do the same here.
> -        */
> -       if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE)
> -               return 0;
> +       if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE) {
> +               /*
> +                * On remount overlayfs has always ignored all mount
> +                * options no matter if malformed or not so for
> +                * backwards compatibility we do the same here.
> +                */
> +               if (fc->oldapi)
> +                       return 0;
> +
> +               /*
> +                * Give us the freedom to allow changing mount options
> +                * with the new mount api in the future. So instead of
> +                * silently ignoring everything we report a proper
> +                * error. This is only visible for users of the new
> +                * mount api.
> +                */
> +               return invalfc(fc, "No changes allowed in reconfigure");
> +       }
>
>         opt = fs_parse(fc, ovl_parameter_spec, param, &result);
>         if (opt < 0)
>
> ---
> base-commit: cc7e4d7ce5ea183b9ca735f7466b4491a1ee440e
> change-id: 20230620-fs-overlayfs-mount-api-remount-93b210e6ccd8
>