All of lore.kernel.org
 help / color / mirror / Atom feed
* [LTP] [PATCH 1/1] fsconfig03: Skip on FUSE
@ 2023-03-28 14:40 Petr Vorel
  2023-03-29 11:58 ` Wei Gao via ltp
  2023-03-31 15:34 ` Avinesh Kumar
  0 siblings, 2 replies; 6+ messages in thread
From: Petr Vorel @ 2023-03-28 14:40 UTC (permalink / raw)
  To: ltp

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(-)

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"},
-- 
2.40.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [LTP] [PATCH 1/1] fsconfig03: Skip on FUSE
  2023-03-28 14:40 [LTP] [PATCH 1/1] fsconfig03: Skip on FUSE Petr Vorel
@ 2023-03-29 11:58 ` Wei Gao via ltp
  2023-03-29 13:38   ` Petr Vorel
  2023-03-31 15:34 ` Avinesh Kumar
  1 sibling, 1 reply; 6+ messages in thread
From: Wei Gao via ltp @ 2023-03-29 11:58 UTC (permalink / raw)
  To: Petr Vorel; +Cc: ltp

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
> 

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [LTP] [PATCH 1/1] fsconfig03: Skip on FUSE
  2023-03-29 11:58 ` Wei Gao via ltp
@ 2023-03-29 13:38   ` Petr Vorel
  2023-03-30 14:14     ` Wei Gao via ltp
  0 siblings, 1 reply; 6+ messages in thread
From: Petr Vorel @ 2023-03-29 13:38 UTC (permalink / raw)
  To: Wei Gao; +Cc: ltp

> 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

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [LTP] [PATCH 1/1] fsconfig03: Skip on FUSE
  2023-03-29 13:38   ` Petr Vorel
@ 2023-03-30 14:14     ` Wei Gao via ltp
  0 siblings, 0 replies; 6+ messages in thread
From: Wei Gao via ltp @ 2023-03-30 14:14 UTC (permalink / raw)
  To: Petr Vorel; +Cc: ltp

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!

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [LTP] [PATCH 1/1] fsconfig03: Skip on FUSE
  2023-03-28 14:40 [LTP] [PATCH 1/1] fsconfig03: Skip on FUSE Petr Vorel
  2023-03-29 11:58 ` Wei Gao via ltp
@ 2023-03-31 15:34 ` Avinesh Kumar
  2023-04-03  5:55   ` Petr Vorel
  1 sibling, 1 reply; 6+ messages in thread
From: Avinesh Kumar @ 2023-03-31 15:34 UTC (permalink / raw)
  To: Petr Vorel; +Cc: ltp

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



-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [LTP] [PATCH 1/1] fsconfig03: Skip on FUSE
  2023-03-31 15:34 ` Avinesh Kumar
@ 2023-04-03  5:55   ` Petr Vorel
  0 siblings, 0 replies; 6+ messages in thread
From: Petr Vorel @ 2023-04-03  5:55 UTC (permalink / raw)
  To: Avinesh Kumar; +Cc: ltp

Hi Avinesh,

thanks for your review and testing. Merged.

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-04-03  5:55 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-28 14:40 [LTP] [PATCH 1/1] fsconfig03: Skip on FUSE Petr Vorel
2023-03-29 11:58 ` Wei Gao via ltp
2023-03-29 13:38   ` Petr Vorel
2023-03-30 14:14     ` Wei Gao via ltp
2023-03-31 15:34 ` Avinesh Kumar
2023-04-03  5:55   ` Petr Vorel

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.