Re: [f2fs-dev] [External Mail]Re: [PATCH v3] f2fs-tools: fix to check loop device for non-root users

["黄佳男 via Linux-f2fs-devel" <linux-f2fs-devel@lists.sourceforge.net>]

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.


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