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