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@xxxxxxxxxx>
> ---
> 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.
