From: "Sungjong Seo" <sj1557.seo@samsung.com>
To: "'Tetsuhiro Kohada'" <kohada.t2@gmail.com>
Cc: <kohada.tetsuhiro@dc.mitsubishielectric.co.jp>,
<mori.takahiro@ab.mitsubishielectric.co.jp>,
<motai.hirotaka@aj.mitsubishielectric.co.jp>,
"'Namjae Jeon'" <namjae.jeon@samsung.com>,
<linux-fsdevel@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH 2/3] exfat: remove useless check in exfat_move_file()
Date: Mon, 28 Sep 2020 16:36:28 +0900 [thread overview]
Message-ID: <8c9701d6956a$13898560$3a9c9020$@samsung.com> (raw)
In-Reply-To: <8a430d18-39ac-135f-d522-90d44276faf8@gmail.com>
> >> --- a/fs/exfat/namei.c
> >> +++ b/fs/exfat/namei.c
> >> @@ -1095,11 +1095,6 @@ static int exfat_move_file(struct inode
> >> *inode, struct exfat_chain *p_olddir,
> >> if (!epmov)
> >> return -EIO;
> >>
> >> - /* check if the source and target directory is the same */
> >> - if (exfat_get_entry_type(epmov) == TYPE_DIR &&
> >> - le32_to_cpu(epmov->dentry.stream.start_clu) == p_newdir->dir)
> >> - return -EINVAL;
> >> -
> >
> > It might check if the cluster numbers are same between source entry
> > and target directory.
>
> This checks if newdir is the move target itself.
> Example:
> mv /mnt/dir0 /mnt/dir0/foo
>
> However, this check is not enough.
> We need to check newdir and all ancestors.
> Example:
> mv /mnt/dir0 /mnt/dir0/dir1/foo
> mv /mnt/dir0 /mnt/dir0/dir1/dir2/foo
> ...
>
> This is probably a taboo for all layered filesystems.
>
>
> > Could you let me know what code you mentioned?
> > Or do you mean the codes on vfs?
>
> You can find in do_renameat2(). --- around 'fs/namei.c:4440'
> If the destination ancestors are itself, our driver will not be called.
I think, of course, vfs has been doing that.
So that code is unnecessary in normal situations.
That code comes from the old exfat implementation.
And as far as I understand, it seems to check once more "the cluster number"
even though it comes through vfs so that it tries detecting abnormal of on-disk.
Anyway, I agonized if it is really needed.
In conclusion, old code could be eliminated and your patch looks reasonable.
Thanks
Acked-by: Sungjong Seo <sj1557.seo@samsung.com>
>
>
> BTW
> Are you busy now?
I'm sorry, I'm so busy for my full time work :(
Anyway, I'm trying to review serious bug patches or bug reports first.
Other patches, such as clean-up or code refactoring, may take some time to review.
> I am waiting for your reply about "integrates dir-entry getting and
> validation" patch.
As I know, your patch is being under review by Namjae.
>
> BR
> ---
> Tetsuhiro Kohada <kohada.t2@gmail.com>
next prev parent reply other threads:[~2020-09-28 7:36 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <CGME20200911044511epcas1p4d62863352e65c534cd6080dd38d54b26@epcas1p4.samsung.com>
2020-09-11 4:45 ` [PATCH 2/3] exfat: remove useless check in exfat_move_file() Tetsuhiro Kohada
2020-09-16 2:32 ` Sungjong Seo
2020-09-16 9:30 ` Tetsuhiro Kohada
2020-09-28 7:36 ` Sungjong Seo [this message]
2020-09-28 7:49 ` Namjae Jeon
2020-09-30 10:41 ` Tetsuhiro Kohada
2020-09-30 4:01 ` Tetsuhiro Kohada
2020-09-30 9:08 ` Tetsuhiro Kohada
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='8c9701d6956a$13898560$3a9c9020$@samsung.com' \
--to=sj1557.seo@samsung.com \
--cc=kohada.t2@gmail.com \
--cc=kohada.tetsuhiro@dc.mitsubishielectric.co.jp \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mori.takahiro@ab.mitsubishielectric.co.jp \
--cc=motai.hirotaka@aj.mitsubishielectric.co.jp \
--cc=namjae.jeon@samsung.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox