| Message ID | 20230328144031.791212-1-pvorel@suse.cz |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [1/1] fsconfig03: Skip on FUSE | expand |
On Tue, Mar 28, 2023 at 04:40:31PM +0200, Petr Vorel wrote: > fsconfig03 is broken not only on vfat and ntfs, but also over FUSE: > > tst_supported_fs_types.c:120: TINFO: FUSE does support exfat I have a question on function has_kernel_support. If has_kernel_support start check exfat file system, if exfat not exist then start check fuse, i have no idea why we still need check fuse, i suppose directly return "exfat not support" instead of "FUSE does support exfat". static enum tst_fs_impl has_kernel_support(const char *fs_type) { ... snprintf(template, sizeof(template), "%s/mountXXXXXX", tmpdir); if (!mkdtemp(template)) tst_brk(TBROK | TERRNO, "mkdtemp(%s) failed", template); ret = mount("/dev/zero", template, fs_type, 0, NULL); if ((ret && errno != ENODEV) || !ret) { if (!ret) tst_umount(template); tst_res(TINFO, "Kernel supports %s", fs_type); SAFE_RMDIR(template); return TST_FS_KERNEL; } SAFE_RMDIR(template); /* Is FUSE supported by kernel? */ /////////start check fuse??? if (fuse_supported == -1) { ret = open("/dev/fuse", O_RDWR); if (ret < 0) { fuse_supported = 0; } else { fuse_supported = 1; SAFE_CLOSE(ret); > tst_supported_fs_types.c:55: TINFO: mkfs.exfat does exist > ... > tst_test.c:1634: TINFO: === Testing on exfat === > tst_test.c:1093: TINFO: Formatting /dev/loop0 with exfat opts='' extra opts='' > fsconfig03.c:38: TBROK: fsopen() failed: ENODEV (19) > > NOTE: it actually works on vfat which is not over FUSE: > tst_supported_fs_types.c:90: TINFO: Kernel supports vfat > tst_supported_fs_types.c:55: TINFO: mkfs.vfat does exist > ... > tst_test.c:1634: TINFO: === Testing on vfat === > tst_test.c:1093: TINFO: Formatting /dev/loop0 with vfat opts='' extra opts='' > fsconfig03.c:72: TPASS: kernel seems to be fine on vfat > > Reported-by: Wei Gao <wegao@suse.com> > Signed-off-by: Petr Vorel <pvorel@suse.cz> > --- > Verifying NTFS as kernel module (I'd be surprised if it was not > working). The setup is the same as for fsconfig01.c (fsconfig02.c is for > expected errors, thus has less strict requirements). > > Kind regards, > Petr > > testcases/kernel/syscalls/fsconfig/fsconfig03.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/testcases/kernel/syscalls/fsconfig/fsconfig03.c b/testcases/kernel/syscalls/fsconfig/fsconfig03.c > index e891c9f98..0ba5355d3 100644 > --- a/testcases/kernel/syscalls/fsconfig/fsconfig03.c > +++ b/testcases/kernel/syscalls/fsconfig/fsconfig03.c > @@ -88,7 +88,7 @@ static struct tst_test test = { > .mntpoint = MNTPOINT, > .all_filesystems = 1, > .taint_check = TST_TAINT_W | TST_TAINT_D, > - .skip_filesystems = (const char *const []){"ntfs", "vfat", NULL}, > + .skip_filesystems = (const char *const []){"fuse", NULL}, I feel you can not skip fuse system since i found following list in LTP: static const char *const fs_type_whitelist[] = { "ext2", "ext3", "ext4", "xfs", "btrfs", "vfat", "exfat", "ntfs", "tmpfs", NULL }; > .tags = (const struct tst_tag[]) { > {"linux-git", "722d94847de29"}, > {"CVE", "2022-0185"}, > -- > 2.40.0 >
> On Tue, Mar 28, 2023 at 04:40:31PM +0200, Petr Vorel wrote: > > fsconfig03 is broken not only on vfat and ntfs, but also over FUSE: > > tst_supported_fs_types.c:120: TINFO: FUSE does support exfat > I have a question on function has_kernel_support. > If has_kernel_support start check exfat file system, if exfat not exist then start > check fuse, i have no idea why we still need check fuse, i suppose directly > return "exfat not support" instead of "FUSE does support exfat". Because some filesystems can be supported by both Linux kernel or by FUSE (userspace). IMHO only NTFS and exfat are supported by both. We first check kernel implementation, but if it's not supported (e.g. kernel configured to support particular filesystem, but kernel module not being installed), we try to check if FUSE does support that filesystem. > static enum tst_fs_impl has_kernel_support(const char *fs_type) > { > ... > snprintf(template, sizeof(template), "%s/mountXXXXXX", tmpdir); > if (!mkdtemp(template)) > tst_brk(TBROK | TERRNO, "mkdtemp(%s) failed", template); > ret = mount("/dev/zero", template, fs_type, 0, NULL); > if ((ret && errno != ENODEV) || !ret) { > if (!ret) > tst_umount(template); > tst_res(TINFO, "Kernel supports %s", fs_type); > SAFE_RMDIR(template); > return TST_FS_KERNEL; > } > SAFE_RMDIR(template); > /* Is FUSE supported by kernel? */ /////////start check fuse??? > if (fuse_supported == -1) { > ret = open("/dev/fuse", O_RDWR); > if (ret < 0) { > fuse_supported = 0; > } else { > fuse_supported = 1; > SAFE_CLOSE(ret); ... > > +++ b/testcases/kernel/syscalls/fsconfig/fsconfig03.c > > @@ -88,7 +88,7 @@ static struct tst_test test = { > > .mntpoint = MNTPOINT, > > .all_filesystems = 1, > > .taint_check = TST_TAINT_W | TST_TAINT_D, > > - .skip_filesystems = (const char *const []){"ntfs", "vfat", NULL}, > > + .skip_filesystems = (const char *const []){"fuse", NULL}, > I feel you can not skip fuse system since i found following list in LTP: > static const char *const fs_type_whitelist[] = { > "ext2", > "ext3", > "ext4", > "xfs", > "btrfs", > "vfat", > "exfat", > "ntfs", > "tmpfs", > NULL > }; This array name is quite confusing, that I even once proposed to rename it :) [1]. It's used for .all_filesystems = 1 (if you don't define .skip_filesystems, all filesystems defined in fs_type_whitelist will be running. Therefore only filesystems defined in it makes sense to whitelist. But on tests without .all_filesystems = 1, any filesystem can be defined in .skip_filesystems (see testcases/kernel/syscalls/fcntl/fcntl33.c, it has "ramfs" and "nfs"). In this case filesystem in $TMPDIR is checked and if the same as any member in .skip_filesystems, test is being skipped (see the beginning of do_test_setup()). I put some effort to document it, but mainly due "ext2/ext3/ext4" (when .all_filesystems = 1, is *not defined*) vs. using these separately (e.g..skip_filesystems = (const char *const []){"ext2", "ext3", NULL} ). Back to your question, fuse is somehow special, see functions in lib/safe_macros.c Also, note, we don't even use kernel's NTFS driver, see lib/safe_macros.c: int safe_mount(const char *file, const int lineno, void (*cleanup_fn)(void), const char *source, const char *target, const char *filesystemtype, unsigned long mountflags, const void *data) { int rval = -1; /* * Don't try using the kernel's NTFS driver when mounting NTFS, since * the kernel's NTFS driver doesn't have proper write support. */ if (!filesystemtype || strcmp(filesystemtype, "ntfs")) { rval = mount(source, target, filesystemtype, mountflags, data); if (!rval) return 0; } /* * The FUSE filesystem executes mount.fuse helper, which tries to * execute corresponding binary name which is encoded at the start of * the source string and separated by # from the device name. * * The mount helpers are called mount.$fs_type. */ if (possibly_fuse(filesystemtype)) { char buf[1024]; tst_resm_(file, lineno, TINFO, "Trying FUSE..."); snprintf(buf, sizeof(buf), "mount.%s '%s' '%s'", filesystemtype, source, target); Kind regards, Petr [1] https://lore.kernel.org/ltp/20220126141152.6428-1-pvorel@suse.cz/ [2] https://github.com/linux-test-project/ltp/wiki/C-Test-API#113-filesystem-type-detection-and-skiplist
On Wed, Mar 29, 2023 at 03:38:18PM +0200, Petr Vorel wrote: > > On Tue, Mar 28, 2023 at 04:40:31PM +0200, Petr Vorel wrote: > > > fsconfig03 is broken not only on vfat and ntfs, but also over FUSE: > > > > tst_supported_fs_types.c:120: TINFO: FUSE does support exfat > > > I have a question on function has_kernel_support. > > > If has_kernel_support start check exfat file system, if exfat not exist then start > > check fuse, i have no idea why we still need check fuse, i suppose directly > > return "exfat not support" instead of "FUSE does support exfat". > > Because some filesystems can be supported by both Linux kernel or by FUSE > (userspace). IMHO only NTFS and exfat are supported by both. We first check > kernel implementation, but if it's not supported (e.g. kernel configured to > support particular filesystem, but kernel module not being installed), > we try to check if FUSE does support that filesystem. > My opinion is has_kernel_support need ONLY check exfat implementation in kernel, this can better match the name of function. Use for example has_fuse_support return exfat-fuse or ntfs-3g support. > > > +++ b/testcases/kernel/syscalls/fsconfig/fsconfig03.c > > > @@ -88,7 +88,7 @@ static struct tst_test test = { > > > .mntpoint = MNTPOINT, > > > .all_filesystems = 1, > > > .taint_check = TST_TAINT_W | TST_TAINT_D, > > > - .skip_filesystems = (const char *const []){"ntfs", "vfat", NULL}, > > > + .skip_filesystems = (const char *const []){"fuse", NULL}, > > > I feel you can not skip fuse system since i found following list in LTP: > > static const char *const fs_type_whitelist[] = { > > "ext2", > > "ext3", > > "ext4", > > "xfs", > > "btrfs", > > "vfat", > > "exfat", > > "ntfs", > > "tmpfs", > > NULL > > }; > > This array name is quite confusing, that I even once proposed to rename it :) [1]. > It's used for .all_filesystems = 1 (if you don't define .skip_filesystems, all > filesystems defined in fs_type_whitelist will be running. Therefore only > filesystems defined in it makes sense to whitelist. > I prefer replace .all_filesystems to .def_filesystems_check if we keep current logic. > But on tests without .all_filesystems = 1, any filesystem can be defined in > .skip_filesystems (see testcases/kernel/syscalls/fcntl/fcntl33.c, it has "ramfs" > and "nfs"). In this case filesystem in $TMPDIR is checked and if the same as any > member in .skip_filesystems, test is being skipped (see the beginning of > do_test_setup()). I put some effort to document it, but mainly due > "ext2/ext3/ext4" (when .all_filesystems = 1, is *not defined*) vs. using these > separately (e.g..skip_filesystems = (const char *const []){"ext2", "ext3", NULL} ). I have some difficult to understand above logic. > > Back to your question, fuse is somehow special, see functions in lib/safe_macros.c > Also, note, we don't even use kernel's NTFS driver, see lib/safe_macros.c: > I prefer using more clear word describe fuse or non-fuse filesystem for white list/skip_filesystems such as: *exfat // means we check exfat kernel version *exfat-fuse //fuse version, we can add this into current white list *ntfs // check ntfs kernel version *ntfs-3g //fuse userspace implemen, we can add this into current white list > int safe_mount(const char *file, const int lineno, void (*cleanup_fn)(void), > const char *source, const char *target, > const char *filesystemtype, unsigned long mountflags, > const void *data) > { > int rval = -1; > > /* > * Don't try using the kernel's NTFS driver when mounting NTFS, since > * the kernel's NTFS driver doesn't have proper write support. > */ > if (!filesystemtype || strcmp(filesystemtype, "ntfs")) { > rval = mount(source, target, filesystemtype, mountflags, data); > if (!rval) > return 0; > } > > /* > * The FUSE filesystem executes mount.fuse helper, which tries to > * execute corresponding binary name which is encoded at the start of > * the source string and separated by # from the device name. > * > * The mount helpers are called mount.$fs_type. > */ > if (possibly_fuse(filesystemtype)) { > char buf[1024]; > > tst_resm_(file, lineno, TINFO, "Trying FUSE..."); > snprintf(buf, sizeof(buf), "mount.%s '%s' '%s'", > filesystemtype, source, target); > > Kind regards, > Petr > > [1] https://lore.kernel.org/ltp/20220126141152.6428-1-pvorel@suse.cz/ > [2] https://github.com/linux-test-project/ltp/wiki/C-Test-API#113-filesystem-type-detection-and-skiplist Thanks again for support so much detail backgroud information!
Hi Petr, Reviewed-by: Avinesh Kumar <akumar@suse.de> Tested-by: Avinesh Kumar <akumar@suse.de> On Tuesday, March 28, 2023 8:10:31 PM IST Petr Vorel wrote: > fsconfig03 is broken not only on vfat and ntfs, but also over FUSE: > > tst_supported_fs_types.c:120: TINFO: FUSE does support exfat > tst_supported_fs_types.c:55: TINFO: mkfs.exfat does exist > ... > tst_test.c:1634: TINFO: === Testing on exfat === > tst_test.c:1093: TINFO: Formatting /dev/loop0 with exfat opts='' extra opts='' > fsconfig03.c:38: TBROK: fsopen() failed: ENODEV (19) I could also reproduce this failure when running FUSE implementation of exfat. And tested the patch which takes care of it. > > NOTE: it actually works on vfat which is not over FUSE: Yes, test works fine on vfat. > tst_supported_fs_types.c:90: TINFO: Kernel supports vfat > tst_supported_fs_types.c:55: TINFO: mkfs.vfat does exist > ... > tst_test.c:1634: TINFO: === Testing on vfat === > tst_test.c:1093: TINFO: Formatting /dev/loop0 with vfat opts='' extra opts='' > fsconfig03.c:72: TPASS: kernel seems to be fine on vfat > > Reported-by: Wei Gao <wegao@suse.com> > Signed-off-by: Petr Vorel <pvorel@suse.cz> > --- > Verifying NTFS as kernel module (I'd be surprised if it was not > working). The setup is the same as for fsconfig01.c (fsconfig02.c is for > expected errors, thus has less strict requirements). > > Kind regards, > Petr > > testcases/kernel/syscalls/fsconfig/fsconfig03.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/testcases/kernel/syscalls/fsconfig/fsconfig03.c b/testcases/kernel/syscalls/fsconfig/fsconfig03.c > index e891c9f98..0ba5355d3 100644 > --- a/testcases/kernel/syscalls/fsconfig/fsconfig03.c > +++ b/testcases/kernel/syscalls/fsconfig/fsconfig03.c > @@ -88,7 +88,7 @@ static struct tst_test test = { > .mntpoint = MNTPOINT, > .all_filesystems = 1, > .taint_check = TST_TAINT_W | TST_TAINT_D, > - .skip_filesystems = (const char *const []){"ntfs", "vfat", NULL}, > + .skip_filesystems = (const char *const []){"fuse", NULL}, > .tags = (const struct tst_tag[]) { > {"linux-git", "722d94847de29"}, > {"CVE", "2022-0185"}, > Regards, Avinesh
Hi Avinesh, thanks for your review and testing. Merged. Kind regards, Petr
diff --git a/testcases/kernel/syscalls/fsconfig/fsconfig03.c b/testcases/kernel/syscalls/fsconfig/fsconfig03.c index e891c9f98..0ba5355d3 100644 --- a/testcases/kernel/syscalls/fsconfig/fsconfig03.c +++ b/testcases/kernel/syscalls/fsconfig/fsconfig03.c @@ -88,7 +88,7 @@ static struct tst_test test = { .mntpoint = MNTPOINT, .all_filesystems = 1, .taint_check = TST_TAINT_W | TST_TAINT_D, - .skip_filesystems = (const char *const []){"ntfs", "vfat", NULL}, + .skip_filesystems = (const char *const []){"fuse", NULL}, .tags = (const struct tst_tag[]) { {"linux-git", "722d94847de29"}, {"CVE", "2022-0185"},
fsconfig03 is broken not only on vfat and ntfs, but also over FUSE: tst_supported_fs_types.c:120: TINFO: FUSE does support exfat tst_supported_fs_types.c:55: TINFO: mkfs.exfat does exist ... tst_test.c:1634: TINFO: === Testing on exfat === tst_test.c:1093: TINFO: Formatting /dev/loop0 with exfat opts='' extra opts='' fsconfig03.c:38: TBROK: fsopen() failed: ENODEV (19) NOTE: it actually works on vfat which is not over FUSE: tst_supported_fs_types.c:90: TINFO: Kernel supports vfat tst_supported_fs_types.c:55: TINFO: mkfs.vfat does exist ... tst_test.c:1634: TINFO: === Testing on vfat === tst_test.c:1093: TINFO: Formatting /dev/loop0 with vfat opts='' extra opts='' fsconfig03.c:72: TPASS: kernel seems to be fine on vfat Reported-by: Wei Gao <wegao@suse.com> Signed-off-by: Petr Vorel <pvorel@suse.cz> --- Verifying NTFS as kernel module (I'd be surprised if it was not working). The setup is the same as for fsconfig01.c (fsconfig02.c is for expected errors, thus has less strict requirements). Kind regards, Petr testcases/kernel/syscalls/fsconfig/fsconfig03.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)