diff mbox series

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

Message ID 20240527075854.1260981-4-lihongbo22@huawei.com (mailing list archive)
State New
Headers show
Series enhance the path resolution capability in fs_parser | expand

Commit Message

Hongbo Li May 27, 2024, 7:58 a.m. UTC
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.

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(-)

Comments

Christian Brauner May 27, 2024, 3:39 p.m. UTC | #1
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.
diff mbox series

Patch

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;
 		}
-
-		error = fs_lookup_param(fc, param, 1, LOOKUP_FOLLOW, &path);
-		if (error) {
-			ext4_msg(NULL, KERN_ERR, "error: could not find "
-				 "journal device path");
-			return -EINVAL;
-		}
-
-		journal_inode = d_inode(path.dentry);
-		ctx->journal_devnum = new_encode_dev(journal_inode->i_rdev);
+		ctx->journal_devnum = result.uint_32;
 		ctx->spec |= EXT4_SPEC_JOURNAL_DEV;
-		path_put(&path);
 		return 0;
-	}
 	case Opt_journal_ioprio:
 		if (result.uint_32 > 7) {
 			ext4_msg(NULL, KERN_ERR, "Invalid journal IO priority"