[LTP] [RFC PATCH v1 0/2] Fix NTFS-related failures in statmount02 and

[LTP] [RFC PATCH v1 0/2] Fix NTFS-related failures in statmount02 and

[Jan Polensky]

This patch series addresses two issues in the LTP statmount tests related to
incorrect assumptions about NTFS mounts via FUSE. Both issues stem from
FUSE-based mounting behavior, where the save_mount logic does not respect the
MS_RDONLY flag and reports the filesystem type as fuseblk instead of ntfs.

Jan Polensky (2):
  safe_macros.c: Fix missing ro flag for FUSE NTFS mounts
  statmount06.c: Fix incorrect fs_type assumption NTFS->FUSEBLK

 lib/safe_macros.c                                 | 9 +++++++--
 testcases/kernel/syscalls/statmount/statmount06.c | 8 +++++++-
 2 files changed, 14 insertions(+), 3 deletions(-)

--
2.49.0


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

[LTP] [RFC PATCH v1 1/2] safe_macros.c: Fix missing ro flag for FUSE NTFS mounts

[Jan Polensky]

The test incorrectly assumes that NTFS mounts are read-only, but the mount
command does not explicitly set the read-only flag. As a result, the test fails
when checking `sb_flags` against `MS_RDONLY`.

Old behavior:

	sudo LTP_SINGLE_FS_TYPE=ntfs strace -e trace=mount,statmount -o log.log -s 128 -f ./statmount02
	...
	statmount02.c:47: TFAIL: st_mount->sb_flags (0) != MS_RDONLY (1)
	...

Relevant log excerpt:

	3890601 mount("/dev/zero", "/tmp/mountBDSEqk", "ntfs", 0, NULL) = -1 ENOTBLK (Block device required)
	3890608 mount("/dev/loop0", "/tmp/LTP_staTPRruR/mntpoint", "fuseblk", 0, "allow_other,blksize=4096,fd=4,rootmode=40000,user_id=0,group_id=0") = 0
	3890607 statmount({size=24, mnt_id=0x80010957, param=STATMOUNT_SB_BASIC}, {size=512, mask=STATMOUNT_SB_BASIC, sb_dev_major=7, sb_dev_minor=0, sb_magic=FUSE_SUPER_MAGIC, sb_flags=0}, 512, 0) = 0

Signed-off-by: Jan Polensky <japo@linux.ibm.com>
---
 lib/safe_macros.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/lib/safe_macros.c b/lib/safe_macros.c
index 6946cc5bcb94..1270b17af8f4 100644
--- a/lib/safe_macros.c
+++ b/lib/safe_macros.c
@@ -942,10 +942,15 @@ int safe_mount(const char *file, const int lineno, void (*cleanup_fn)(void),
 	 */
 	if (possibly_fuse(filesystemtype)) {
 		char buf[1024];
+		const char* mount_fmt;

 		tst_resm_(file, lineno, TINFO, "Trying FUSE...");
-		snprintf(buf, sizeof(buf), "mount.%s '%s' '%s'",
-			filesystemtype, source, target);
+		if (!strcmp(filesystemtype, "ntfs") && mountflags & MS_RDONLY)
+			mount_fmt = "mount.%s -o ro '%s' '%s'";
+		else
+			mount_fmt = "mount.%s '%s' '%s'";
+		snprintf(buf, sizeof(buf), mount_fmt, filesystemtype,
+				source, target);

 		rval = tst_system(buf);
 		if (WIFEXITED(rval) && WEXITSTATUS(rval) == 0)
--
2.49.0


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

Re: [LTP] [RFC PATCH v1 1/2] safe_macros.c: Fix missing ro flag for FUSE NTFS mounts

[Petr Vorel]

Hi Jan,

> The test incorrectly assumes that NTFS mounts are read-only, but the mount
> command does not explicitly set the read-only flag. As a result, the test fails
> when checking `sb_flags` against `MS_RDONLY`.

> Old behavior:

Reviewed-by: Petr Vorel <pvorel@suse.cz>
IMHO candidate for a release. Li, Cyril WDYT?

> 	sudo LTP_SINGLE_FS_TYPE=ntfs strace -e trace=mount,statmount -o log.log -s 128 -f ./statmount02
FYI the problem is currently LTP_SINGLE_FS_TYPE is still meant only for testing
one of the filesystems among .all_filesystems => these in fs_type_whitelist[]
(see lib/tst_supported_fs_types.c).

BTW that's similar to use case we haven't even formulated in issue:
https://lore.kernel.org/ltp/Z066Fj9VQVlTOMp_@rei.lan/
https://lore.kernel.org/ltp/Z1mA2wzjW0hpQxUH@yuki.lan/
(The only difference is that this requires to use filesystem not in
fs_type_whitelist[], therefore minor bug for it was found).

Kind regards,
Petr

> 	...
> 	statmount02.c:47: TFAIL: st_mount->sb_flags (0) != MS_RDONLY (1)
> 	...

> Relevant log excerpt:

> 	3890601 mount("/dev/zero", "/tmp/mountBDSEqk", "ntfs", 0, NULL) = -1 ENOTBLK (Block device required)
> 	3890608 mount("/dev/loop0", "/tmp/LTP_staTPRruR/mntpoint", "fuseblk", 0, "allow_other,blksize=4096,fd=4,rootmode=40000,user_id=0,group_id=0") = 0
> 	3890607 statmount({size=24, mnt_id=0x80010957, param=STATMOUNT_SB_BASIC}, {size=512, mask=STATMOUNT_SB_BASIC, sb_dev_major=7, sb_dev_minor=0, sb_magic=FUSE_SUPER_MAGIC, sb_flags=0}, 512, 0) = 0

> Signed-off-by: Jan Polensky <japo@linux.ibm.com>
> ---
>  lib/safe_macros.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)

> diff --git a/lib/safe_macros.c b/lib/safe_macros.c
> index 6946cc5bcb94..1270b17af8f4 100644
> --- a/lib/safe_macros.c
> +++ b/lib/safe_macros.c
> @@ -942,10 +942,15 @@ int safe_mount(const char *file, const int lineno, void (*cleanup_fn)(void),
>  	 */
>  	if (possibly_fuse(filesystemtype)) {
>  		char buf[1024];
> +		const char* mount_fmt;

>  		tst_resm_(file, lineno, TINFO, "Trying FUSE...");
> -		snprintf(buf, sizeof(buf), "mount.%s '%s' '%s'",
> -			filesystemtype, source, target);
> +		if (!strcmp(filesystemtype, "ntfs") && mountflags & MS_RDONLY)
> +			mount_fmt = "mount.%s -o ro '%s' '%s'";
> +		else
> +			mount_fmt = "mount.%s '%s' '%s'";
> +		snprintf(buf, sizeof(buf), mount_fmt, filesystemtype,
> +				source, target);

>  		rval = tst_system(buf);
>  		if (WIFEXITED(rval) && WEXITSTATUS(rval) == 0)

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

Re: [LTP] [RFC PATCH v1 1/2] safe_macros.c: Fix missing ro flag for FUSE NTFS mounts

[Li Wang via ltp]

Petr Vorel <pvorel@suse.cz> wrote:

> Hi Jan,
>
> > The test incorrectly assumes that NTFS mounts are read-only, but the mount
> > command does not explicitly set the read-only flag. As a result, the test fails
> > when checking `sb_flags` against `MS_RDONLY`.
>
> > Old behavior:
>
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
> IMHO candidate for a release. Li, Cyril WDYT?

I agree.

-- 
Regards,
Li Wang


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

Re: [LTP] [RFC PATCH v1 1/2] safe_macros.c: Fix missing ro flag for FUSE NTFS mounts

[Cyril Hrubis]

Hi!
> diff --git a/lib/safe_macros.c b/lib/safe_macros.c
> index 6946cc5bcb94..1270b17af8f4 100644
> --- a/lib/safe_macros.c
> +++ b/lib/safe_macros.c
> @@ -942,10 +942,15 @@ int safe_mount(const char *file, const int lineno, void (*cleanup_fn)(void),
>  	 */
>  	if (possibly_fuse(filesystemtype)) {
>  		char buf[1024];
> +		const char* mount_fmt;
> 
>  		tst_resm_(file, lineno, TINFO, "Trying FUSE...");
> -		snprintf(buf, sizeof(buf), "mount.%s '%s' '%s'",
> -			filesystemtype, source, target);
> +		if (!strcmp(filesystemtype, "ntfs") && mountflags & MS_RDONLY)

Do we need to limit this to "ntfs"? I suppose that for other FUSE
filesystems would have the same problem, e.g. exfat.

Other than this the patch looks good.

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [RFC PATCH v1 1/2] safe_macros.c: Fix missing ro flag for FUSE NTFS mounts

[Cyril Hrubis]

Hi!
> > diff --git a/lib/safe_macros.c b/lib/safe_macros.c
> > index 6946cc5bcb94..1270b17af8f4 100644
> > --- a/lib/safe_macros.c
> > +++ b/lib/safe_macros.c
> > @@ -942,10 +942,15 @@ int safe_mount(const char *file, const int lineno, void (*cleanup_fn)(void),
> >  	 */
> >  	if (possibly_fuse(filesystemtype)) {
> >  		char buf[1024];
> > +		const char* mount_fmt;
> > 
> >  		tst_resm_(file, lineno, TINFO, "Trying FUSE...");
> > -		snprintf(buf, sizeof(buf), "mount.%s '%s' '%s'",
> > -			filesystemtype, source, target);
> > +		if (!strcmp(filesystemtype, "ntfs") && mountflags & MS_RDONLY)
> 
> Do we need to limit this to "ntfs"? I suppose that for other FUSE
> filesystems would have the same problem, e.g. exfat.
> 
> Other than this the patch looks good.

Also if my patch that fixes the fuse blacklist gets applied we need to
remove the fuse from the blacklist from statmount02 to get the test
enabled after this fix.

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [RFC PATCH v1 1/2] safe_macros.c: Fix missing ro flag for FUSE NTFS mounts

[Cyril Hrubis]

Hi!
> > > --- a/lib/safe_macros.c
> > > +++ b/lib/safe_macros.c
> > > @@ -942,10 +942,15 @@ int safe_mount(const char *file, const int lineno, void (*cleanup_fn)(void),
> > >  	 */
> > >  	if (possibly_fuse(filesystemtype)) {
> > >  		char buf[1024];
> > > +		const char* mount_fmt;
> > > 
> > >  		tst_resm_(file, lineno, TINFO, "Trying FUSE...");
> > > -		snprintf(buf, sizeof(buf), "mount.%s '%s' '%s'",
> > > -			filesystemtype, source, target);
> > > +		if (!strcmp(filesystemtype, "ntfs") && mountflags & MS_RDONLY)
> > 
> > Do we need to limit this to "ntfs"? I suppose that for other FUSE
> > filesystems would have the same problem, e.g. exfat.
> > 
> > Other than this the patch looks good.
> 
> Also if my patch that fixes the fuse blacklist gets applied we need to
> remove the fuse from the blacklist from statmount02 to get the test
> enabled after this fix.

Ping. Are you going to work on this? I think that it's a good idea to
fix the fuse mount to apply the flags correctly and then enable the
tests that were previously disabled by this.

-- 
Cyril Hrubis
chrubis@suse.cz

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

[LTP] [RFC PATCH v1 2/2] statmount06.c: Fix incorrect fs_type assumption NTFS->FUSEBLK

[Jan Polensky]

The test incorrectly assumed that a volume mounted with "fuseblk" would report
its fs_type as "ntfs" via statmount, which contradicts the actual mount options
used and the resulting fs_type "fuseblk".

Old behavior:

	sudo LTP_SINGLE_FS_TYPE=ntfs strace -e trace=mount,statmount -o log.log -s 128 -f ./statmount06
	...
	statmount06.c:42: TFAIL: st_mount->str + st_mount->fs_type (fuseblk) != tst_device->fs_type (ntfs)
	...

Relevant log excerpt:

	111740 mount("/dev/zero", "/tmp/mountbWjYuf", "ntfs", 0, NULL) = -1 ENODEV (No such device)
	111749 mount("/dev/loop9", "/tmp/LTP_stai6UG2H/mntpoint", "fuseblk", 0, "allow_other,blksize=4096,fd=4,rootmode=40000,user_id=0,group_id=0") = 0
	111752 statmount({size=24, mnt_id=0x80003ab9, param=STATMOUNT_FS_TYPE}, {size=521, mask=STATMOUNT_FS_TYPE, fs_type="fuseblk"}, 1024, 0) = 0

Signed-off-by: Jan Polensky <japo@linux.ibm.com>
---
 testcases/kernel/syscalls/statmount/statmount06.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/testcases/kernel/syscalls/statmount/statmount06.c b/testcases/kernel/syscalls/statmount/statmount06.c
index fe41d5b87e7f..dedf9bba14f9 100644
--- a/testcases/kernel/syscalls/statmount/statmount06.c
+++ b/testcases/kernel/syscalls/statmount/statmount06.c
@@ -28,6 +28,8 @@ static struct statmount *st_mount;
 
 static void run(void)
 {
+	const char *expected_fs_type;
+
 	memset(st_mount, 0, SM_SIZE);
 
 	TST_EXP_PASS(statmount(root_id,	STATMOUNT_FS_TYPE, st_mount,
@@ -37,7 +39,11 @@ static void run(void)
 		return;
 
 	TST_EXP_EQ_LI(st_mount->mask, STATMOUNT_FS_TYPE);
-	TST_EXP_EQ_STR(st_mount->str + st_mount->fs_type, tst_device->fs_type);
+	if (!strcmp(tst_device->fs_type, "ntfs"))
+		expected_fs_type = "fuseblk";
+	else
+		expected_fs_type = tst_device->fs_type;
+	TST_EXP_EQ_STR(st_mount->str + st_mount->fs_type, expected_fs_type);
 }
 
 static void setup(void)
-- 
2.49.0


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

Re: [LTP] [RFC PATCH v1 2/2] statmount06.c: Fix incorrect fs_type assumption NTFS->FUSEBLK

[Cyril Hrubis]

Hi!
> The test incorrectly assumed that a volume mounted with "fuseblk" would report
> its fs_type as "ntfs" via statmount, which contradicts the actual mount options
> used and the resulting fs_type "fuseblk".
> 
> Old behavior:
> 
> 	sudo LTP_SINGLE_FS_TYPE=ntfs strace -e trace=mount,statmount -o log.log -s 128 -f ./statmount06
> 	...
> 	statmount06.c:42: TFAIL: st_mount->str + st_mount->fs_type (fuseblk) != tst_device->fs_type (ntfs)
> 	...

This may be a bug in the LTP_SINGLE_FS_TYPE, because the test has
.skip_filesystems = {"fuse", NULL} in the tst_test structure, which
means that the test is not supposed to run on fuse.

> Signed-off-by: Jan Polensky <japo@linux.ibm.com>
> ---
>  testcases/kernel/syscalls/statmount/statmount06.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/testcases/kernel/syscalls/statmount/statmount06.c b/testcases/kernel/syscalls/statmount/statmount06.c
> index fe41d5b87e7f..dedf9bba14f9 100644
> --- a/testcases/kernel/syscalls/statmount/statmount06.c
> +++ b/testcases/kernel/syscalls/statmount/statmount06.c
> @@ -28,6 +28,8 @@ static struct statmount *st_mount;
>  
>  static void run(void)
>  {
> +	const char *expected_fs_type;
> +
>  	memset(st_mount, 0, SM_SIZE);
>  
>  	TST_EXP_PASS(statmount(root_id,	STATMOUNT_FS_TYPE, st_mount,
> @@ -37,7 +39,11 @@ static void run(void)
>  		return;
>  
>  	TST_EXP_EQ_LI(st_mount->mask, STATMOUNT_FS_TYPE);
> -	TST_EXP_EQ_STR(st_mount->str + st_mount->fs_type, tst_device->fs_type);
> +	if (!strcmp(tst_device->fs_type, "ntfs"))
> +		expected_fs_type = "fuseblk";
> +	else
> +		expected_fs_type = tst_device->fs_type;
> +	TST_EXP_EQ_STR(st_mount->str + st_mount->fs_type, expected_fs_type);

And this does not really work, since there is in-kernel ntfs driver as
well and in newer kernels it has supposedly read-write support too.

I guess that if we wanted to enable the test on fuse, we would have to
base this on check if the fs has been mounted by fuse. Maybe we need a
tst_is_fuse() function that would possibly parse /proc/mount or
something along that way.

-- 
Cyril Hrubis
chrubis@suse.cz

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