From: Jaegeuk Kim <jaegeuk@kernel.org>
To: 黄佳男 <huangjianan@xiaomi.com>
Cc: 王辉 <wanghui33@xiaomi.com>,
"jnhuang95@gmail.com" <jnhuang95@gmail.com>,
"linux-f2fs-devel@lists.sourceforge.net"
<linux-f2fs-devel@lists.sourceforge.net>
Subject: Re: [f2fs-dev] [External Mail]Re: [PATCH v3] f2fs-tools: fix to check loop device for non-root users
Date: Tue, 5 Mar 2024 12:51:53 -0800 [thread overview]
Message-ID: <ZeeF6QrxxdJ3qnng@google.com> (raw)
In-Reply-To: <9ee3e01b-72c2-4eed-aed7-b005b3173203@xiaomi.com>
On 03/01, 黄佳男 wrote:
> On 2024/3/1 16:39, Juhyung Park wrote:
> > [外部邮件] 此邮件来源于小米公司外部，请谨慎处理。若对邮件安全性存疑，请将邮件转发给misec@xiaomi.com进行反馈
> >
> > Hi Huang and Chao.
> >
> > I feel like this special loopback handling alongside Chao's
> > 14197d546b93 on f2fs-tools is just unnecessarily complicating the code
> > flow.
> > We're now doing what, lookup to /sys, parse original backing file,
> > remove trailing newline char, stat()'ing it to make sure it exists?
>
> Indeed this is not a good approach.
>
> > What if the stat()'ed file is a new file after the original backing
> > file has been deleted?
> >
> > Being able to overwrite an active loopback backing file is a semantic
> > that Linux provides willingly.
> > O_EXCL only works on block devices and it's a POSIX guarantee that
> > multiple writers can work on a regular file.
> >
> > IMHO we should honor that, but if we really want to prevent this akin
> > to e2fsprogs, we should be using mntent like e2fsprogs.
>
> e2fsprogs will continue to check if opening the loop device fails,
> rather than
>
> exiting. This way non-root users can use mkfs/fsck normally, although there
>
> may be overwrite issues.
I think we need to fix this first and try to find a better way.
https://lore.kernel.org/linux-f2fs-devel/20240305204834.101697-1-jaegeuk@kernel.org/T/#u
>
>
> Thanks,
>
> Jianan
>
> > On Fri, Mar 1, 2024 at 4:15 PM Huang Jianan via Linux-f2fs-devel
> > <linux-f2fs-devel@lists.sourceforge.net> wrote:
> >> Currently mkfs/fsck gets the following error when executed by
> >> non-root users:
> >>
> >> Info: open /dev/loop0 failed errno:13
> >> Error: Not available on mounted device!
> >>
> >> Let's fix it by reading the backing file from sysfs.
> >>
> >> Fixes: 14197d546b93 ("f2fs-tools: fix to check loop device")
> >> Signed-off-by: Huang Jianan <huangjianan@xiaomi.com>
> >> ---
> >> v3:
> >> - Skip deleted backing file.
> >> lib/libf2fs.c | 40 +++++++++++++++++++++++++++-------------
> >> 1 file changed, 27 insertions(+), 13 deletions(-)
> >>
> >> diff --git a/lib/libf2fs.c b/lib/libf2fs.c
> >> index d51e485..fad3fd4 100644
> >> --- a/lib/libf2fs.c
> >> +++ b/lib/libf2fs.c
> >> @@ -832,7 +832,7 @@ int f2fs_dev_is_umounted(char *path)
> >> }
> >> } else if (S_ISREG(st_buf.st_mode)) {
> >> /* check whether regular is backfile of loop device */
> >> -#if defined(HAVE_LINUX_LOOP_H) && defined(HAVE_LINUX_MAJOR_H)
> >> +#if defined(HAVE_LINUX_MAJOR_H)
> >> struct mntent *mnt;
> >> struct stat st_loop;
> >> FILE *f;
> >> @@ -840,8 +840,9 @@ int f2fs_dev_is_umounted(char *path)
> >> f = setmntent("/proc/mounts", "r");
> >>
> >> while ((mnt = getmntent(f)) != NULL) {
> >> - struct loop_info64 loopinfo = {0, };
> >> - int loop_fd, err;
> >> + struct stat st_back;
> >> + int sysfs_fd, rc;
> >> + char buf[PATH_MAX + 1];
> >>
> >> if (mnt->mnt_fsname[0] != '/')
> >> continue;
> >> @@ -852,23 +853,36 @@ int f2fs_dev_is_umounted(char *path)
> >> if (major(st_loop.st_rdev) != LOOP_MAJOR)
> >> continue;
> >>
> >> - loop_fd = open(mnt->mnt_fsname, O_RDONLY);
> >> - if (loop_fd < 0) {
> >> + snprintf(buf, PATH_MAX,
> >> + "/sys/dev/block/%d:%d/loop/backing_file",
> >> + major(st_loop.st_rdev), minor(st_loop.st_rdev));
> >> +
> >> + sysfs_fd = open(buf, O_RDONLY);
> >> + if (sysfs_fd < 0) {
> >> MSG(0, "Info: open %s failed errno:%d\n",
> >> - mnt->mnt_fsname, errno);
> >> + buf, errno);
> >> return -1;
> >> }
> >>
> >> - err = ioctl(loop_fd, LOOP_GET_STATUS64, &loopinfo);
> >> - close(loop_fd);
> >> - if (err < 0) {
> >> - MSG(0, "\tError: ioctl LOOP_GET_STATUS64 failed errno:%d!\n",
> >> - errno);
> >> + memset(buf, 0, PATH_MAX + 1);
> >> + rc = read(sysfs_fd, buf, PATH_MAX);
> >> + if (rc < 0) {
> >> + MSG(0, "Info: read %s failed errno:%d\n",
> >> + buf, errno);
> >> return -1;
> >> }
> >>
> >> - if (st_buf.st_dev == loopinfo.lo_device &&
> >> - st_buf.st_ino == loopinfo.lo_inode) {
> >> + /* Remove trailing newline */
> >> + if (rc > 0 && *(buf + rc - 1) == '\n')
> >> + --rc;
> >> + buf[rc] = '\0';
> >> +
> >> + /* Skip deleted files like "xxx (deleted)" */
> >> + if (stat(buf, &st_back) != 0)
> >> + continue;
> >> +
> >> + if (st_buf.st_dev == st_back.st_dev &&
> >> + st_buf.st_ino == st_back.st_ino) {
> >> MSG(0, "\tError: In use by loop device!\n");
> >> return -EBUSY;
> >> }
> >> --
> >> 2.34.1
> >>
> >>
> >>
> >> _______________________________________________
> >> Linux-f2fs-devel mailing list
> >> Linux-f2fs-devel@lists.sourceforge.net
> >> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
>
>
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
prev parent reply other threads:[~2024-03-05 20:52 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-29 7:18 [f2fs-dev] [PATCH] f2fs-tools: fix to check loop device for non-root users Huang Jianan via Linux-f2fs-devel
2024-02-29 7:41 ` [f2fs-dev] [PATCH v2] " Huang Jianan via Linux-f2fs-devel
2024-03-01 7:14 ` [f2fs-dev] [PATCH v3] " Huang Jianan via Linux-f2fs-devel
2024-03-01 8:39 ` Juhyung Park
2024-03-01 10:32 ` [f2fs-dev] [External Mail]Re: " 黄佳男 via Linux-f2fs-devel
2024-03-05 20:51 ` Jaegeuk Kim [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=ZeeF6QrxxdJ3qnng@google.com \
--to=jaegeuk@kernel.org \
--cc=huangjianan@xiaomi.com \
--cc=jnhuang95@gmail.com \
--cc=linux-f2fs-devel@lists.sourceforge.net \
--cc=wanghui33@xiaomi.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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).