Add missing NULL check of root_inode in autofs_fill_super().
While we are at it simplify the logic by taking advantage of the VFS
cleanup procedures and get rid of the goto error handling, as suggested
by Al Viro.
Signed-off-by: Ian Kent <[email protected]>
Cc: Al Viro <[email protected]>
Cc: Christian Brauner <[email protected]>
Cc: Bill O'Donnell <[email protected]>
Reported-by: [email protected]
---
fs/autofs/inode.c | 50 ++++++++++++++++++-----------------------------
1 file changed, 19 insertions(+), 31 deletions(-)
diff --git a/fs/autofs/inode.c b/fs/autofs/inode.c
index a5083d447a62..d5dd4223b461 100644
--- a/fs/autofs/inode.c
+++ b/fs/autofs/inode.c
@@ -311,7 +311,6 @@ static int autofs_fill_super(struct super_block *s, struct fs_context *fc)
struct inode *root_inode;
struct dentry *root;
struct autofs_info *ino;
- int ret = -ENOMEM;
pr_debug("starting up, sbi = %p\n", sbi);
@@ -328,56 +327,45 @@ static int autofs_fill_super(struct super_block *s, struct fs_context *fc)
*/
ino = autofs_new_ino(sbi);
if (!ino)
- goto fail;
+ goto -ENOMEM;
root_inode = autofs_get_inode(s, S_IFDIR | 0755);
- root_inode->i_uid = ctx->uid;
- root_inode->i_gid = ctx->gid;
-
- root = d_make_root(root_inode);
- if (!root)
- goto fail_ino;
-
- root->d_fsdata = ino;
+ if (root_inode) {
+ root_inode->i_uid = ctx->uid;
+ root_inode->i_gid = ctx->gid;
+ root_inode->i_fop = &autofs_root_operations;
+ root_inode->i_op = &autofs_dir_inode_operations;
+ }
+ s->s_root = d_make_root(root_inode);
+ if (unlikely(!s->s_root)) {
+ autofs_free_ino(ino);
+ return -ENOMEM;
+ }
+ s->s_root->d_fsdata = ino;
if (ctx->pgrp_set) {
sbi->oz_pgrp = find_get_pid(ctx->pgrp);
if (!sbi->oz_pgrp) {
ret = invalf(fc, "Could not find process group %d",
ctx->pgrp);
- goto fail_dput;
+ return ret;
}
} else {
sbi->oz_pgrp = get_task_pid(current, PIDTYPE_PGID);
}
if (autofs_type_trigger(sbi->type))
- __managed_dentry_set_managed(root);
-
- root_inode->i_fop = &autofs_root_operations;
- root_inode->i_op = &autofs_dir_inode_operations;
+ /* s->s_root won't be contended so there's little to
+ * be gained by not taking the d_lock when setting
+ * d_flags, even when a lot mounts are being done.
+ */
+ managed_dentry_set_managed(s->s_root);
pr_debug("pipe fd = %d, pgrp = %u\n",
sbi->pipefd, pid_nr(sbi->oz_pgrp));
sbi->flags &= ~AUTOFS_SBI_CATATONIC;
-
- /*
- * Success! Install the root dentry now to indicate completion.
- */
- s->s_root = root;
return 0;
-
- /*
- * Failure ... clean up.
- */
-fail_dput:
- dput(root);
- goto fail;
-fail_ino:
- autofs_free_ino(ino);
-fail:
- return ret;
}
/*
--
2.41.0
Hi Ian,
kernel test robot noticed the following build errors:
[auto build test ERROR on brauner-vfs/vfs.all]
[also build test ERROR on linus/master v6.7-rc1 next-20231116]
[cannot apply to vfs-idmapping/for-next]
[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/Ian-Kent/autofs-add-new_inode-check-in-autofs_fill_super/20231116-081017
base: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git vfs.all
patch link: https://lore.kernel.org/r/20231116000746.7359-1-raven%40themaw.net
patch subject: [PATCH] autofs: add: new_inode check in autofs_fill_super()
config: i386-buildonly-randconfig-001-20231116 (https://download.01.org/0day-ci/archive/20231116/[email protected]/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231116/[email protected]/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 <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All error/warnings (new ones prefixed by >>):
fs/autofs/inode.c: In function 'autofs_fill_super':
>> fs/autofs/inode.c:330:22: error: expected identifier or '*' before '-' token
330 | goto -ENOMEM;
| ^
>> fs/autofs/inode.c:349:25: error: 'ret' undeclared (first use in this function); did you mean 'net'?
349 | ret = invalf(fc, "Could not find process group %d",
| ^~~
| net
fs/autofs/inode.c:349:25: note: each undeclared identifier is reported only once for each function it appears in
>> fs/autofs/inode.c:312:24: warning: unused variable 'root' [-Wunused-variable]
312 | struct dentry *root;
| ^~~~
vim +330 fs/autofs/inode.c
306
307 static int autofs_fill_super(struct super_block *s, struct fs_context *fc)
308 {
309 struct autofs_fs_context *ctx = fc->fs_private;
310 struct autofs_sb_info *sbi = s->s_fs_info;
311 struct inode *root_inode;
> 312 struct dentry *root;
313 struct autofs_info *ino;
314
315 pr_debug("starting up, sbi = %p\n", sbi);
316
317 sbi->sb = s;
318 s->s_blocksize = 1024;
319 s->s_blocksize_bits = 10;
320 s->s_magic = AUTOFS_SUPER_MAGIC;
321 s->s_op = &autofs_sops;
322 s->s_d_op = &autofs_dentry_operations;
323 s->s_time_gran = 1;
324
325 /*
326 * Get the root inode and dentry, but defer checking for errors.
327 */
328 ino = autofs_new_ino(sbi);
329 if (!ino)
> 330 goto -ENOMEM;
331
332 root_inode = autofs_get_inode(s, S_IFDIR | 0755);
333 if (root_inode) {
334 root_inode->i_uid = ctx->uid;
335 root_inode->i_gid = ctx->gid;
336 root_inode->i_fop = &autofs_root_operations;
337 root_inode->i_op = &autofs_dir_inode_operations;
338 }
339 s->s_root = d_make_root(root_inode);
340 if (unlikely(!s->s_root)) {
341 autofs_free_ino(ino);
342 return -ENOMEM;
343 }
344 s->s_root->d_fsdata = ino;
345
346 if (ctx->pgrp_set) {
347 sbi->oz_pgrp = find_get_pid(ctx->pgrp);
348 if (!sbi->oz_pgrp) {
> 349 ret = invalf(fc, "Could not find process group %d",
350 ctx->pgrp);
351 return ret;
352 }
353 } else {
354 sbi->oz_pgrp = get_task_pid(current, PIDTYPE_PGID);
355 }
356
357 if (autofs_type_trigger(sbi->type))
358 /* s->s_root won't be contended so there's little to
359 * be gained by not taking the d_lock when setting
360 * d_flags, even when a lot mounts are being done.
361 */
362 managed_dentry_set_managed(s->s_root);
363
364 pr_debug("pipe fd = %d, pgrp = %u\n",
365 sbi->pipefd, pid_nr(sbi->oz_pgrp));
366
367 sbi->flags &= ~AUTOFS_SBI_CATATONIC;
368 return 0;
369 }
370
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Hi Ian,
kernel test robot noticed the following build warnings:
[auto build test WARNING on brauner-vfs/vfs.all]
[also build test WARNING on linus/master v6.7-rc1 next-20231116]
[cannot apply to vfs-idmapping/for-next]
[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/Ian-Kent/autofs-add-new_inode-check-in-autofs_fill_super/20231116-081017
base: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git vfs.all
patch link: https://lore.kernel.org/r/20231116000746.7359-1-raven%40themaw.net
patch subject: [PATCH] autofs: add: new_inode check in autofs_fill_super()
config: x86_64-rhel-8.3-rust (https://download.01.org/0day-ci/archive/20231116/[email protected]/config)
compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git ae42196bc493ffe877a7e3dff8be32035dea4d07)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231116/[email protected]/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 <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All warnings (new ones prefixed by >>):
fs/autofs/inode.c:330:8: error: expected identifier
goto -ENOMEM;
^
>> fs/autofs/inode.c:330:8: warning: misleading indentation; statement is not part of the previous 'if' [-Wmisleading-indentation]
fs/autofs/inode.c:329:2: note: previous statement is here
if (!ino)
^
fs/autofs/inode.c:349:4: error: use of undeclared identifier 'ret'
ret = invalf(fc, "Could not find process group %d",
^
fs/autofs/inode.c:351:11: error: use of undeclared identifier 'ret'
return ret;
^
>> fs/autofs/inode.c:330:8: warning: expression result unused [-Wunused-value]
goto -ENOMEM;
^~~~~~~
2 warnings and 3 errors generated.
vim +/if +330 fs/autofs/inode.c
306
307 static int autofs_fill_super(struct super_block *s, struct fs_context *fc)
308 {
309 struct autofs_fs_context *ctx = fc->fs_private;
310 struct autofs_sb_info *sbi = s->s_fs_info;
311 struct inode *root_inode;
312 struct dentry *root;
313 struct autofs_info *ino;
314
315 pr_debug("starting up, sbi = %p\n", sbi);
316
317 sbi->sb = s;
318 s->s_blocksize = 1024;
319 s->s_blocksize_bits = 10;
320 s->s_magic = AUTOFS_SUPER_MAGIC;
321 s->s_op = &autofs_sops;
322 s->s_d_op = &autofs_dentry_operations;
323 s->s_time_gran = 1;
324
325 /*
326 * Get the root inode and dentry, but defer checking for errors.
327 */
328 ino = autofs_new_ino(sbi);
329 if (!ino)
> 330 goto -ENOMEM;
331
332 root_inode = autofs_get_inode(s, S_IFDIR | 0755);
333 if (root_inode) {
334 root_inode->i_uid = ctx->uid;
335 root_inode->i_gid = ctx->gid;
336 root_inode->i_fop = &autofs_root_operations;
337 root_inode->i_op = &autofs_dir_inode_operations;
338 }
339 s->s_root = d_make_root(root_inode);
340 if (unlikely(!s->s_root)) {
341 autofs_free_ino(ino);
342 return -ENOMEM;
343 }
344 s->s_root->d_fsdata = ino;
345
346 if (ctx->pgrp_set) {
347 sbi->oz_pgrp = find_get_pid(ctx->pgrp);
348 if (!sbi->oz_pgrp) {
349 ret = invalf(fc, "Could not find process group %d",
350 ctx->pgrp);
351 return ret;
352 }
353 } else {
354 sbi->oz_pgrp = get_task_pid(current, PIDTYPE_PGID);
355 }
356
357 if (autofs_type_trigger(sbi->type))
358 /* s->s_root won't be contended so there's little to
359 * be gained by not taking the d_lock when setting
360 * d_flags, even when a lot mounts are being done.
361 */
362 managed_dentry_set_managed(s->s_root);
363
364 pr_debug("pipe fd = %d, pgrp = %u\n",
365 sbi->pipefd, pid_nr(sbi->oz_pgrp));
366
367 sbi->flags &= ~AUTOFS_SBI_CATATONIC;
368 return 0;
369 }
370
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
On 16/11/23 19:23, kernel test robot wrote:
> Hi Ian,
>
> kernel test robot noticed the following build warnings:
Crikey, how did this compile ... I think I need to just send a replacement
patch.
Ian
> [auto build test WARNING on brauner-vfs/vfs.all]
> [also build test WARNING on linus/master v6.7-rc1 next-20231116]
> [cannot apply to vfs-idmapping/for-next]
> [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/Ian-Kent/autofs-add-new_inode-check-in-autofs_fill_super/20231116-081017
> base:https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git vfs.all
> patch link:https://lore.kernel.org/r/20231116000746.7359-1-raven%40themaw.net
> patch subject: [PATCH] autofs: add: new_inode check in autofs_fill_super()
> config: x86_64-rhel-8.3-rust (https://download.01.org/0day-ci/archive/20231116/[email protected]/config)
> compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git ae42196bc493ffe877a7e3dff8be32035dea4d07)
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231116/[email protected]/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<[email protected]>
> | Closes:https://lore.kernel.org/oe-kbuild-all/[email protected]/
>
> All warnings (new ones prefixed by >>):
>
> fs/autofs/inode.c:330:8: error: expected identifier
> goto -ENOMEM;
> ^
>>> fs/autofs/inode.c:330:8: warning: misleading indentation; statement is not part of the previous 'if' [-Wmisleading-indentation]
> fs/autofs/inode.c:329:2: note: previous statement is here
> if (!ino)
> ^
> fs/autofs/inode.c:349:4: error: use of undeclared identifier 'ret'
> ret = invalf(fc, "Could not find process group %d",
> ^
> fs/autofs/inode.c:351:11: error: use of undeclared identifier 'ret'
> return ret;
> ^
>>> fs/autofs/inode.c:330:8: warning: expression result unused [-Wunused-value]
> goto -ENOMEM;
> ^~~~~~~
> 2 warnings and 3 errors generated.
>
>
> vim +/if +330 fs/autofs/inode.c
>
> 306
> 307 static int autofs_fill_super(struct super_block *s, struct fs_context *fc)
> 308 {
> 309 struct autofs_fs_context *ctx = fc->fs_private;
> 310 struct autofs_sb_info *sbi = s->s_fs_info;
> 311 struct inode *root_inode;
> 312 struct dentry *root;
> 313 struct autofs_info *ino;
> 314
> 315 pr_debug("starting up, sbi = %p\n", sbi);
> 316
> 317 sbi->sb = s;
> 318 s->s_blocksize = 1024;
> 319 s->s_blocksize_bits = 10;
> 320 s->s_magic = AUTOFS_SUPER_MAGIC;
> 321 s->s_op = &autofs_sops;
> 322 s->s_d_op = &autofs_dentry_operations;
> 323 s->s_time_gran = 1;
> 324
> 325 /*
> 326 * Get the root inode and dentry, but defer checking for errors.
> 327 */
> 328 ino = autofs_new_ino(sbi);
> 329 if (!ino)
> > 330 goto -ENOMEM;
> 331
> 332 root_inode = autofs_get_inode(s, S_IFDIR | 0755);
> 333 if (root_inode) {
> 334 root_inode->i_uid = ctx->uid;
> 335 root_inode->i_gid = ctx->gid;
> 336 root_inode->i_fop = &autofs_root_operations;
> 337 root_inode->i_op = &autofs_dir_inode_operations;
> 338 }
> 339 s->s_root = d_make_root(root_inode);
> 340 if (unlikely(!s->s_root)) {
> 341 autofs_free_ino(ino);
> 342 return -ENOMEM;
> 343 }
> 344 s->s_root->d_fsdata = ino;
> 345
> 346 if (ctx->pgrp_set) {
> 347 sbi->oz_pgrp = find_get_pid(ctx->pgrp);
> 348 if (!sbi->oz_pgrp) {
> 349 ret = invalf(fc, "Could not find process group %d",
> 350 ctx->pgrp);
> 351 return ret;
> 352 }
> 353 } else {
> 354 sbi->oz_pgrp = get_task_pid(current, PIDTYPE_PGID);
> 355 }
> 356
> 357 if (autofs_type_trigger(sbi->type))
> 358 /* s->s_root won't be contended so there's little to
> 359 * be gained by not taking the d_lock when setting
> 360 * d_flags, even when a lot mounts are being done.
> 361 */
> 362 managed_dentry_set_managed(s->s_root);
> 363
> 364 pr_debug("pipe fd = %d, pgrp = %u\n",
> 365 sbi->pipefd, pid_nr(sbi->oz_pgrp));
> 366
> 367 sbi->flags &= ~AUTOFS_SBI_CATATONIC;
> 368 return 0;
> 369 }
> 370
>