| Message ID | 20240527014717.690140-2-lihongbo22@huawei.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | enhance the path resolution capability in fs_parser | expand |
Hi Hongbo, kernel test robot noticed the following build warnings: [auto build test WARNING on tytso-ext4/dev] [also build test WARNING on brauner-vfs/vfs.all linus/master v6.10-rc1 next-20240523] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Hongbo-Li/fs-add-blockdev-parser-for-filesystem-mount-option/20240527-094930 base: https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git dev patch link: https://lore.kernel.org/r/20240527014717.690140-2-lihongbo22%40huawei.com patch subject: [PATCH 1/4] fs: add blockdev parser for filesystem mount option. config: x86_64-allnoconfig (https://download.01.org/0day-ci/archive/20240527/202405271100.aGNNnrKK-lkp@intel.com/config) compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240527/202405271100.aGNNnrKK-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202405271100.aGNNnrKK-lkp@intel.com/ All warnings (new ones prefixed by >>): >> fs/fs_parser.c:324:8: warning: variable 'f' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized] 324 | if (p->flags & fs_param_can_be_empty) | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ fs/fs_parser.c:343:29: note: uninitialized use occurs here 343 | ret = filename_lookup(dfd, f, LOOKUP_FOLLOW, &path, NULL); | ^ fs/fs_parser.c:324:4: note: remove the 'if' if its condition is always true 324 | if (p->flags & fs_param_can_be_empty) | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 325 | return 0; fs/fs_parser.c:316:20: note: initialize the variable 'f' to silence this warning 316 | struct filename *f; | ^ | = NULL >> fs/fs_parser.c:324:8: warning: variable 'put_f' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized] 324 | if (p->flags & fs_param_can_be_empty) | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ fs/fs_parser.c:360:6: note: uninitialized use occurs here 360 | if (put_f) | ^~~~~ fs/fs_parser.c:324:4: note: remove the 'if' if its condition is always true 324 | if (p->flags & fs_param_can_be_empty) | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 325 | return 0; fs/fs_parser.c:319:12: note: initialize the variable 'put_f' to silence this warning 319 | bool put_f; | ^ | = 0 >> fs/fs_parser.c:324:8: warning: variable 'dfd' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized] 324 | if (p->flags & fs_param_can_be_empty) | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ fs/fs_parser.c:343:24: note: uninitialized use occurs here 343 | ret = filename_lookup(dfd, f, LOOKUP_FOLLOW, &path, NULL); | ^~~ fs/fs_parser.c:324:4: note: remove the 'if' if its condition is always true 324 | if (p->flags & fs_param_can_be_empty) | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 325 | return 0; fs/fs_parser.c:315:9: note: initialize the variable 'dfd' to silence this warning 315 | int dfd; | ^ | = 0 3 warnings generated. vim +324 fs/fs_parser.c 310 311 int fs_param_is_blockdev(struct p_log *log, const struct fs_parameter_spec *p, 312 struct fs_parameter *param, struct fs_parse_result *result) 313 { 314 int ret; 315 int dfd; 316 struct filename *f; 317 struct inode *dev_inode; 318 struct path path; 319 bool put_f; 320 321 switch (param->type) { 322 case fs_value_is_string: 323 if (!*param->string) { > 324 if (p->flags & fs_param_can_be_empty) 325 return 0; 326 break; 327 } 328 f = getname_kernel(param->string); 329 if (IS_ERR(f)) 330 return fs_param_bad_value(log, param); 331 dfd = AT_FDCWD; 332 put_f = true; 333 break; 334 case fs_value_is_filename: 335 f = param->name; 336 dfd = param->dirfd; 337 put_f = false; 338 break; 339 default: 340 return fs_param_bad_value(log, param); 341 } 342 343 ret = filename_lookup(dfd, f, LOOKUP_FOLLOW, &path, NULL); 344 if (ret < 0) { 345 error_plog(log, "%s: Lookup failure for '%s'", param->key, f->name); 346 goto out_putname; 347 } 348 349 dev_inode = d_backing_inode(path.dentry); 350 if (!S_ISBLK(dev_inode->i_mode)) { 351 error_plog(log, "%s: Non-blockdev passed as '%s'", param->key, f->name); 352 ret = -1; 353 goto out_putpath; 354 } 355 result->uint_32 = new_encode_dev(dev_inode->i_rdev); 356 357 out_putpath: 358 path_put(&path); 359 out_putname: 360 if (put_f) 361 putname(f); 362 363 return (ret < 0) ? fs_param_bad_value(log, param) : 0; 364 } 365 EXPORT_SYMBOL(fs_param_is_blockdev); 366
diff --git a/fs/fs_parser.c b/fs/fs_parser.c index a4d6ca0b8971..48f60ecfcca0 100644 --- a/fs/fs_parser.c +++ b/fs/fs_parser.c @@ -311,7 +311,56 @@ EXPORT_SYMBOL(fs_param_is_fd); int fs_param_is_blockdev(struct p_log *log, const struct fs_parameter_spec *p, struct fs_parameter *param, struct fs_parse_result *result) { - return 0; + int ret; + int dfd; + struct filename *f; + struct inode *dev_inode; + struct path path; + bool put_f; + + switch (param->type) { + case fs_value_is_string: + if (!*param->string) { + if (p->flags & fs_param_can_be_empty) + return 0; + break; + } + f = getname_kernel(param->string); + if (IS_ERR(f)) + return fs_param_bad_value(log, param); + dfd = AT_FDCWD; + put_f = true; + break; + case fs_value_is_filename: + f = param->name; + dfd = param->dirfd; + put_f = false; + break; + default: + return fs_param_bad_value(log, param); + } + + ret = filename_lookup(dfd, f, LOOKUP_FOLLOW, &path, NULL); + if (ret < 0) { + error_plog(log, "%s: Lookup failure for '%s'", param->key, f->name); + goto out_putname; + } + + dev_inode = d_backing_inode(path.dentry); + if (!S_ISBLK(dev_inode->i_mode)) { + error_plog(log, "%s: Non-blockdev passed as '%s'", param->key, f->name); + ret = -1; + goto out_putpath; + } + result->uint_32 = new_encode_dev(dev_inode->i_rdev); + +out_putpath: + path_put(&path); +out_putname: + if (put_f) + putname(f); + + return (ret < 0) ? fs_param_bad_value(log, param) : 0; } EXPORT_SYMBOL(fs_param_is_blockdev);
`fsparam_bdev` uses `fs_param_is_blockdev` to parse the option, but it is currently empty. Filesystems like ext4 use the `fsparam_bdev` to parse `journal_path` into the block device. This general logic should be moved to the vfs layer, not the specific filesystem. Therefore, we implement block device parser in `fs_param_is_blockdev`. And the logic is similar with `fs_lookup_param`. Signed-off-by: Hongbo Li <lihongbo22@huawei.com> --- fs/fs_parser.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 50 insertions(+), 1 deletion(-)