Re: [PATCH v2 3/4] fs: ext4: support relative path for `journal_path` in mount option.

[Christian Brauner <brauner@kernel.org>]

On Mon, May 27, 2024 at 03:58:53PM +0800, Hongbo Li wrote:
> After `fs_param_is_blockdev` is implemented(in fs: add blockdev parser
> for filesystem mount option.), `journal_devnum` can be obtained from
> `result.uint_32` directly.
> 
> Additionally, the `fs_lookup_param` did not consider the relative path
> for block device. When we mount ext4 with `journal_path` option using
> relative path, `param->dirfd` was not set which will cause mounting
> error.

That's a failure in userspace though. If a relative pathname is provided
then AT_FDCWD or a valid file descriptor must be provided:

fsconfig(fd_fs, FSCONFIG_SET_PATH, "journal_path", "relative/path", AT_FDCWD);

But if you're seeing this from mount(8) then util-linux very likely
does (cf. [1]):

fsconfig(fd_fs, FSCONFIG_SET_STRING, "journal_path", "relative/path", 0);

So mount(8) is passing that as a string as it has no way to figure out
that this should be passed as FSCONFIG_SET_PATH. So to allow relative
paths in string options we'd need something (untested) like:

diff --git a/fs/fs_parser.c b/fs/fs_parser.c
index a4d6ca0b8971..18ca40408e91 100644
--- a/fs/fs_parser.c
+++ b/fs/fs_parser.c
@@ -156,6 +156,9 @@ int fs_lookup_param(struct fs_context *fc,
                f = getname_kernel(param->string);
                if (IS_ERR(f))
                        return PTR_ERR(f);
+               /* relative path */
+               if (*f->name[0] != '/')
+                       param->dirfd = AT_FDCWD;
                put_f = true;
                break;
        case fs_value_is_filename:

https://github.com/util-linux/util-linux/blob/55ca447a6a95226fd031a126fb48b01b3efd6284/libmount/src/hook_mount.c#L178

> 
> This can be reproduced easily like this:
> 
> mke2fs -F -O journal_dev $JOURNAL_DEV -b 4096 100M
> mkfs.ext4 -F -J device=$JOURNAL_DEV -b 4096 $FS_DEV
> cd /dev; mount -t ext4 -o journal_path=`basename $JOURNAL_DEV` $FS_DEV $MNT
> 
> Fixes: 461c3af045d3 ("ext4: Change handle_mount_opt() to use fs_parameter")
> Signed-off-by: Hongbo Li <lihongbo22@huawei.com>
> ---
>  fs/ext4/super.c | 26 +-------------------------
>  1 file changed, 1 insertion(+), 25 deletions(-)
> 
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index c682fb927b64..94b39bcae99d 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -2290,39 +2290,15 @@ static int ext4_parse_param(struct fs_context *fc, struct fs_parameter *param)
>  		ctx->spec |= EXT4_SPEC_s_resgid;
>  		return 0;
>  	case Opt_journal_dev:
> -		if (is_remount) {
> -			ext4_msg(NULL, KERN_ERR,
> -				 "Cannot specify journal on remount");
> -			return -EINVAL;
> -		}
> -		ctx->journal_devnum = result.uint_32;
> -		ctx->spec |= EXT4_SPEC_JOURNAL_DEV;
> -		return 0;
>  	case Opt_journal_path:
> -	{
> -		struct inode *journal_inode;
> -		struct path path;
> -		int error;
> -
>  		if (is_remount) {
>  			ext4_msg(NULL, KERN_ERR,
>  				 "Cannot specify journal on remount");
>  			return -EINVAL;
>  		}

This would cause a path lookup in the VFS layer only to immediately
reject it on remount.