| Message ID | 20231116000746.7359-1-raven@themaw.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | autofs: add: new_inode check in autofs_fill_super() | expand |
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/202311161857.bZMQmWsW-lkp@intel.com/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/202311161857.bZMQmWsW-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/202311161857.bZMQmWsW-lkp@intel.com/ 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
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/202311161909.KHau6jEj-lkp@intel.com/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/202311161909.KHau6jEj-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/202311161909.KHau6jEj-lkp@intel.com/
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
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/202311161909.KHau6jEj-lkp@intel.com/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/202311161909.KHau6jEj-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/202311161909.KHau6jEj-lkp@intel.com/ > > 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 >
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; } /*
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 <raven@themaw.net> Cc: Al Viro <viro@zeniv.linux.org.uk> Cc: Christian Brauner <brauner@kernel.org> Cc: Bill O'Donnell <billodo@redhat.com> Reported-by: syzbot+662f87a8ef490f45fa64@syzkaller.appspotmail.com --- fs/autofs/inode.c | 50 ++++++++++++++++++----------------------------- 1 file changed, 19 insertions(+), 31 deletions(-)