From: "Theodore Y. Ts'o" <tytso@mit.edu>
To: Guiyao <guiyao@huawei.com>
Cc: "linux-ext4@vger.kernel.org" <linux-ext4@vger.kernel.org>,
Mingfangsen <mingfangsen@huawei.com>,
"ebiggers@google.com" <ebiggers@google.com>,
"aceballos@gmail.com" <aceballos@gmail.com>,
"vertaling@coevern.nl" <vertaling@coevern.nl>
Subject: Re: 答复: [PATCH v2] e2fsprogs: Check device id in advance to skip fake device name
Date: Mon, 11 Nov 2019 12:20:28 -0500 [thread overview]
Message-ID: <20191111172028.GE7017@mit.edu> (raw)
In-Reply-To: <005F77DB9A260B4E91664DDF22573C66E9D380C7@dggemm512-mbs.china.huawei.com>
On Mon, Nov 11, 2019 at 02:43:46PM +0000, Guiyao wrote:
>
> Actually, we found some insane system administrators, they not only do something like "mount -t tmpfs /dev/sdb /tmp ", but also they do " ln -s /dev/sdb abc ", then "resize2fs abc xxx". :(
So I don't consider ourselves necessarily obligated to twist ourselves
into knots for insane system administrators. :-)
Did you test the patch that I sent out? It handles that case already:
% grep /dev/loop /proc/mounts
/dev/loop0 /mnt2 tmpfs rw,relatime 0 0
/dev/loop0 /mnt ext4 rw,relatime 0 0
% ln -s /dev/loop0 abc
% ./tst_ismounted abc
Device abc reports flags 11
abc is apparently in use.
abc is mounted.
abc is mounted on /mnt2.
> So we have to add the fixing code in both sides of "name matched" and "name not matched".
>
> For the compiling issue, it's my fault in previous patch, and added the macro in a wrong line.
>
> So, I rewrote it again, and please give more advise. Thank you in advance.
Given that I have a patch which I've already tested, and which is a
substantial clean up in terms of removing #ifdef cases and number of
lines of code:
lib/ext2fs/ismounted.c | 39 ++++++++++++---------------------------
1 file changed, 12 insertions(+), 27 deletions(-)
I'm inclined to stick with mine.
But here's the quick review.
> {
> struct mntent *mnt;
> +#ifndef __GNU__
> + struct stat dir_st_buf;
> +#endif /* __GNU__ */
Lots of extra #ifdef/#ifndef is undesirable. As it turns out, it
isn't necessary to have a separate dir_st_buf at all.
> @@ -128,13 +131,32 @@ static errcode_t check_mntent_file(const char *mtab_file, const char *file,
> while ((mnt = getmntent (f)) != NULL) {
> if (mnt->mnt_fsname[0] != '/')
> continue;
> - if (strcmp(file, mnt->mnt_fsname) == 0)
> +#ifndef __GNU__
> + if (stat(mnt->mnt_dir, &dir_st_buf) != 0)
> + continue;
> +#endif
> + if (strcmp(file, mnt->mnt_fsname) == 0) {
> +#ifndef __GNU__
> + if (file_rdev && (file_rdev != dir_st_buf.st_dev)) {
This doesn't need to be under #ifndef __GNU__. In the GNU hurd case,
file_rdev will be zero, so the compiler will remove the if statement
for us, without needing an additional #ifndef __GNU__ test.
> if (stat(mnt->mnt_fsname, &st_buf) == 0) {
> if (ext2fsP_is_disk_device(st_buf.st_mode)) {
> #ifndef __GNU__
> - if (file_rdev && (file_rdev == st_buf.st_rdev))
> - break;
> + if (file_rdev && (file_rdev == st_buf.st_rdev)) {
> + if (file_rdev == dir_st_buf.st_dev)
> + break;
> + }
> +
The reason why this isn't necessary is because we're using stat, and
stat follows symlinks. So when you do "ln -s /dev/sdb abc", and then
we stat abc, st_buf.st_rdev contains the device node of /dev/sbc, not
the symbolic link of abc. So adding a check for dir_st_buf.st_dev is
not needed.
Cheers,
- Ted
prev parent reply other threads:[~2019-11-11 17:20 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-11-11 14:43 答复: [PATCH v2] e2fsprogs: Check device id in advance to skip fake device name Guiyao
2019-11-11 17:20 ` Theodore Y. Ts'o [this message]
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=20191111172028.GE7017@mit.edu \
--to=tytso@mit.edu \
--cc=aceballos@gmail.com \
--cc=ebiggers@google.com \
--cc=guiyao@huawei.com \
--cc=linux-ext4@vger.kernel.org \
--cc=mingfangsen@huawei.com \
--cc=vertaling@coevern.nl \
/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;
as well as URLs for NNTP newsgroup(s).