From: kernel test robot <lkp@intel.com>
To: Namjae Jeon <linkinjeon@kernel.org>
Cc: llvm@lists.linux.dev, kbuild-all@lists.01.org
Subject: Re: [PATCH] ksmbd: fix racy issue from using ->d_parent and ->d_name
Date: Wed, 9 Feb 2022 02:34:43 +0800 [thread overview]
Message-ID: <202202090207.MiyIsofZ-lkp@intel.com> (raw)
In-Reply-To: <20220208010959.4050-1-linkinjeon@kernel.org>
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-from-using-d_parent-and-d_name/20220208-092438
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-lkp@intel.com/config)
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/f4cb65c1c670f5332092a7eb75d569bbd4e46a5f
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@intel.com>
All warnings (new ones prefixed by >>):
>> fs/ksmbd/vfs.c:654:6: warning: variable 'old_dentry' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
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
>> fs/ksmbd/vfs.c:654:6: warning: variable 'new_dentry' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
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
> 654 if (d_is_symlink(new_path.dentry)) {
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
WARNING: multiple messages have this Message-ID (diff)
From: kernel test robot <lkp@intel.com>
To: kbuild-all@lists.01.org
Subject: Re: [PATCH] ksmbd: fix racy issue from using ->d_parent and ->d_name
Date: Wed, 09 Feb 2022 02:34:43 +0800 [thread overview]
Message-ID: <202202090207.MiyIsofZ-lkp@intel.com> (raw)
In-Reply-To: <20220208010959.4050-1-linkinjeon@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 7451 bytes --]
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-from-using-d_parent-and-d_name/20220208-092438
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-lkp(a)intel.com/config)
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/f4cb65c1c670f5332092a7eb75d569bbd4e46a5f
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@intel.com>
All warnings (new ones prefixed by >>):
>> fs/ksmbd/vfs.c:654:6: warning: variable 'old_dentry' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
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
>> fs/ksmbd/vfs.c:654:6: warning: variable 'new_dentry' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
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
> 654 if (d_is_symlink(new_path.dentry)) {
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(a)lists.01.org
next prev parent reply other threads:[~2022-02-08 18:35 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-08 1:09 [PATCH] ksmbd: fix racy issue from using ->d_parent and ->d_name Namjae Jeon
2022-02-08 18:34 ` kernel test robot [this message]
2022-02-08 18:34 ` kernel test robot
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=202202090207.MiyIsofZ-lkp@intel.com \
--to=lkp@intel.com \
--cc=kbuild-all@lists.01.org \
--cc=linkinjeon@kernel.org \
--cc=llvm@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.