[kbuild-all] Re: [PATCH] ksmbd: fix racy issue from using ->d_parent and ->d_name

Hi Namjae, I love your patch! Perhaps something to improve: [auto build test WARNING on linus/master] [also build test WARNING on v5.17-rc3 next-20220208] [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] url: https://github.com/0day-ci/linux/commits/Namjae-Jeon/ksmbd-fix-racy-issue... base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 555f3d7be91a873114c9656069f1a9fa476ec41a config: i386-allyesconfig (https://download.01.org/0day-ci/archive/20220209/202202090207.MiyIsofZ-lk...) compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project e8bff9ae54a55b4dbfeb6ba55f723abbd81bf494) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/f4cb65c1c670f5332092a7eb75d569bbd... git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Namjae-Jeon/ksmbd-fix-racy-issue-from-using-d_parent-and-d_name/20220208-092438 git checkout f4cb65c1c670f5332092a7eb75d569bbd4e46a5f # save the config file to linux build tree mkdir build_dir COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash fs/ksmbd/ If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp(a)intel.com&gt; All warnings (new ones prefixed by >>): if (d_is_symlink(new_path.dentry)) { ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ fs/ksmbd/vfs.c:719:7: note: uninitialized use occurs here dput(old_dentry); ^~~~~~~~~~ fs/ksmbd/vfs.c:654:2: note: remove the 'if' if its condition is always false if (d_is_symlink(new_path.dentry)) { ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ fs/ksmbd/vfs.c:604:27: note: initialize the variable 'old_dentry' to silence this warning struct dentry *old_dentry, *new_dentry, *trap; ^ = NULL if (d_is_symlink(new_path.dentry)) { ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ fs/ksmbd/vfs.c:717:7: note: uninitialized use occurs here dput(new_dentry); ^~~~~~~~~~ fs/ksmbd/vfs.c:654:2: note: remove the 'if' if its condition is always false if (d_is_symlink(new_path.dentry)) { ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ fs/ksmbd/vfs.c:604:40: note: initialize the variable 'new_dentry' to silence this warning struct dentry *old_dentry, *new_dentry, *trap; ^ = NULL 2 warnings generated. vim +654 fs/ksmbd/vfs.c 600 601 int ksmbd_vfs_rename(struct ksmbd_work *work, struct path *path, char *newname, 602 int flags) 603 { 604 struct dentry *old_dentry, *new_dentry, *trap; 605 struct path old_path, new_path; 606 struct qstr old_last, new_last; 607 struct renamedata rd; 608 struct filename *from, *to; 609 struct ksmbd_share_config *share_conf = work->tcon->share_conf; 610 struct ksmbd_file *parent_fp; 611 int old_type, new_type; 612 int err, lookup_flags = LOOKUP_NO_SYMLINKS; 613 char *pathname, *abs_oldname; 614 615 if (ksmbd_override_fsids(work)) 616 return -ENOMEM; 617 618 pathname = kmalloc(PATH_MAX, GFP_KERNEL); 619 if (!pathname) { 620 ksmbd_revert_fsids(work); 621 return -ENOMEM; 622 } 623 624 abs_oldname = d_path(path, pathname, PATH_MAX); 625 if (IS_ERR(abs_oldname)) { 626 err = -EINVAL; 627 goto free_pathname; 628 } 629 630 from = getname_kernel(abs_oldname); 631 if (IS_ERR(from)) { 632 err = PTR_ERR(from); 633 goto free_pathname; 634 } 635 636 to = getname_kernel(newname); 637 if (IS_ERR(to)) { 638 err = PTR_ERR(to); 639 goto putname_from; 640 } 641 642 err = filename_parentat(AT_FDCWD, from, lookup_flags, &old_path, 643 &old_last, &old_type); 644 if (err) 645 goto putnames; 646 647 err = vfs_path_parent_lookup(share_conf->vfs_path.dentry, 648 share_conf->vfs_path.mnt, to, 649 lookup_flags | LOOKUP_BENEATH, 650 &new_path, &new_last, &new_type); 651 if (err) 652 goto out1; 653 655 err = -EACCES; 656 goto out4; 657 } 658 659 trap = lock_rename(old_path.dentry, new_path.dentry); 660 old_dentry = __lookup_hash(&old_last, old_path.dentry, 0); 661 if (IS_ERR(old_dentry)) { 662 err = PTR_ERR(old_dentry); 663 goto out2; 664 } 665 if (d_is_negative(old_dentry)) { 666 err = -ENOENT; 667 goto out3; 668 } 669 670 new_dentry = __lookup_hash(&new_last, new_path.dentry, 671 LOOKUP_RENAME_TARGET); 672 if (IS_ERR(new_dentry)) { 673 err = PTR_ERR(new_dentry); 674 goto out3; 675 } 676 677 if (d_is_symlink(new_dentry)) { 678 err = -EACCES; 679 goto out4; 680 } 681 682 if ((flags & RENAME_NOREPLACE) && d_is_positive(new_dentry)) { 683 err = -EEXIST; 684 goto out4; 685 } 686 687 if (old_dentry == trap) { 688 err = -EINVAL; 689 goto out4; 690 } 691 692 if (new_dentry == trap) { 693 err = -ENOTEMPTY; 694 goto out4; 695 } 696 697 parent_fp = ksmbd_lookup_fd_inode(old_path.dentry->d_inode); 698 if (parent_fp) { 699 if (parent_fp->daccess & FILE_DELETE_LE) { 700 pr_err("parent dir is opened with delete access\n"); 701 err = -ESHARE; 702 goto out4; 703 } 704 } 705 706 rd.old_mnt_userns = mnt_user_ns(old_path.mnt), 707 rd.old_dir = old_path.dentry->d_inode, 708 rd.old_dentry = old_dentry, 709 rd.new_mnt_userns = mnt_user_ns(new_path.mnt), 710 rd.new_dir = new_path.dentry->d_inode, 711 rd.new_dentry = new_dentry, 712 rd.flags = flags, 713 err = vfs_rename(&rd); 714 if (err) 715 ksmbd_debug(VFS, "vfs_rename failed err %d\n", err); 716 out4: 717 dput(new_dentry); 718 out3: 719 dput(old_dentry); 720 out2: 721 unlock_rename(new_path.dentry, old_path.dentry); 722 path_put(&new_path); 723 out1: 724 path_put(&old_path); 725 726 putnames: 727 putname(to); 728 putname_from: 729 putname(from); 730 free_pathname: 731 kfree(pathname); 732 ksmbd_revert_fsids(work); 733 return err; 734 } 735 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org