All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [linux-next:master] [fs/mnt_idmapping.c]  b4291c7fd9: xfstests.generic.645.fail
@ 2024-02-19  6:55 kernel test robot
  2024-02-20  8:57 ` Christian Brauner
  0 siblings, 1 reply; 15+ messages in thread
From: kernel test robot @ 2024-02-19  6:55 UTC (permalink / raw
  To: Taylor Jackson
  Cc: oe-lkp, lkp, Linux Memory Management List, Christian Brauner,
	linux-fsdevel, oliver.sang



Hello,

kernel test robot noticed "xfstests.generic.645.fail" on:

commit: b4291c7fd9e550b91b10c3d7787b9bf5be38de67 ("fs/mnt_idmapping.c: Return -EINVAL when no map is written")
https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git master

[test failed on linux-next/master d37e1e4c52bc60578969f391fb81f947c3e83118]

in testcase: xfstests
version: xfstests-x86_64-c46ca4d1-1_20240205
with following parameters:

	disk: 4HDD
	fs: f2fs
	test: generic-645



compiler: gcc-12
test machine: 4 threads Intel(R) Core(TM) i5-6500 CPU @ 3.20GHz (Skylake) with 32G memory

(please refer to attached dmesg/kmsg for entire log/backtrace)




If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <oliver.sang@intel.com>
| Closes: https://lore.kernel.org/oe-lkp/202402191416.17ec9160-oliver.sang@intel.com

2024-02-17 15:04:19 export TEST_DIR=/fs/sda1
2024-02-17 15:04:19 export TEST_DEV=/dev/sda1
2024-02-17 15:04:19 export FSTYP=f2fs
2024-02-17 15:04:19 export SCRATCH_MNT=/fs/scratch
2024-02-17 15:04:19 mkdir /fs/scratch -p
2024-02-17 15:04:19 export SCRATCH_DEV=/dev/sda4
2024-02-17 15:04:19 export MKFS_OPTIONS=-f
2024-02-17 15:04:19 echo generic/645
2024-02-17 15:04:19 ./check generic/645
FSTYP         -- f2fs
PLATFORM      -- Linux/x86_64 lkp-skl-d03 6.8.0-rc1-00033-gb4291c7fd9e5 #1 SMP PREEMPT_DYNAMIC Sat Feb 17 22:11:35 CST 2024
MKFS_OPTIONS  -- -f /dev/sda4
MOUNT_OPTIONS -- -o acl,user_xattr /dev/sda4 /fs/scratch

generic/645       [failed, exit status 1]- output mismatch (see /lkp/benchmarks/xfstests/results//generic/645.out.bad)
    --- tests/generic/645.out	2024-02-05 17:37:40.000000000 +0000
    +++ /lkp/benchmarks/xfstests/results//generic/645.out.bad	2024-02-17 15:07:42.613312168 +0000
    @@ -1,2 +1,4 @@
     QA output created by 645
     Silence is golden
    +idmapped-mounts.c: 6671: nested_userns - Invalid argument - failure: sys_mount_setattr
    +vfstest.c: 2418: run_test - Invalid argument - failure: test that nested user namespaces behave correctly when attached to idmapped mounts
    ...
    (Run 'diff -u /lkp/benchmarks/xfstests/tests/generic/645.out /lkp/benchmarks/xfstests/results//generic/645.out.bad'  to see the entire diff)
Ran: generic/645
Failures: generic/645
Failed 1 of 1 tests




The kernel config and materials to reproduce are available at:
https://download.01.org/0day-ci/archive/20240219/202402191416.17ec9160-oliver.sang@intel.com



-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki



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

* Re: [linux-next:master] [fs/mnt_idmapping.c]  b4291c7fd9: xfstests.generic.645.fail
  2024-02-19  6:55 [linux-next:master] [fs/mnt_idmapping.c] b4291c7fd9: xfstests.generic.645.fail kernel test robot
@ 2024-02-20  8:57 ` Christian Brauner
  2024-03-25 16:58   ` Darrick J. Wong
  0 siblings, 1 reply; 15+ messages in thread
From: Christian Brauner @ 2024-02-20  8:57 UTC (permalink / raw
  To: kernel test robot
  Cc: Taylor Jackson, oe-lkp, lkp, Linux Memory Management List,
	linux-fsdevel

On Mon, Feb 19, 2024 at 02:55:42PM +0800, kernel test robot wrote:
> 
> 
> Hello,
> 
> kernel test robot noticed "xfstests.generic.645.fail" on:
> 
> commit: b4291c7fd9e550b91b10c3d7787b9bf5be38de67 ("fs/mnt_idmapping.c: Return -EINVAL when no map is written")
> https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git master

The test needs to be updated. We now explicitly fail when no map is
written.

> 
> [test failed on linux-next/master d37e1e4c52bc60578969f391fb81f947c3e83118]
> 
> in testcase: xfstests
> version: xfstests-x86_64-c46ca4d1-1_20240205
> with following parameters:
> 
> 	disk: 4HDD
> 	fs: f2fs
> 	test: generic-645
> 
> 
> 
> compiler: gcc-12
> test machine: 4 threads Intel(R) Core(TM) i5-6500 CPU @ 3.20GHz (Skylake) with 32G memory
> 
> (please refer to attached dmesg/kmsg for entire log/backtrace)
> 
> 
> 
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <oliver.sang@intel.com>
> | Closes: https://lore.kernel.org/oe-lkp/202402191416.17ec9160-oliver.sang@intel.com
> 
> 2024-02-17 15:04:19 export TEST_DIR=/fs/sda1
> 2024-02-17 15:04:19 export TEST_DEV=/dev/sda1
> 2024-02-17 15:04:19 export FSTYP=f2fs
> 2024-02-17 15:04:19 export SCRATCH_MNT=/fs/scratch
> 2024-02-17 15:04:19 mkdir /fs/scratch -p
> 2024-02-17 15:04:19 export SCRATCH_DEV=/dev/sda4
> 2024-02-17 15:04:19 export MKFS_OPTIONS=-f
> 2024-02-17 15:04:19 echo generic/645
> 2024-02-17 15:04:19 ./check generic/645
> FSTYP         -- f2fs
> PLATFORM      -- Linux/x86_64 lkp-skl-d03 6.8.0-rc1-00033-gb4291c7fd9e5 #1 SMP PREEMPT_DYNAMIC Sat Feb 17 22:11:35 CST 2024
> MKFS_OPTIONS  -- -f /dev/sda4
> MOUNT_OPTIONS -- -o acl,user_xattr /dev/sda4 /fs/scratch
> 
> generic/645       [failed, exit status 1]- output mismatch (see /lkp/benchmarks/xfstests/results//generic/645.out.bad)
>     --- tests/generic/645.out	2024-02-05 17:37:40.000000000 +0000
>     +++ /lkp/benchmarks/xfstests/results//generic/645.out.bad	2024-02-17 15:07:42.613312168 +0000
>     @@ -1,2 +1,4 @@
>      QA output created by 645
>      Silence is golden
>     +idmapped-mounts.c: 6671: nested_userns - Invalid argument - failure: sys_mount_setattr
>     +vfstest.c: 2418: run_test - Invalid argument - failure: test that nested user namespaces behave correctly when attached to idmapped mounts
>     ...
>     (Run 'diff -u /lkp/benchmarks/xfstests/tests/generic/645.out /lkp/benchmarks/xfstests/results//generic/645.out.bad'  to see the entire diff)
> Ran: generic/645
> Failures: generic/645
> Failed 1 of 1 tests
> 
> 
> 
> 
> The kernel config and materials to reproduce are available at:
> https://download.01.org/0day-ci/archive/20240219/202402191416.17ec9160-oliver.sang@intel.com
> 
> 
> 
> -- 
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki
> 

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

* Re: [linux-next:master] [fs/mnt_idmapping.c]  b4291c7fd9: xfstests.generic.645.fail
  2024-02-20  8:57 ` Christian Brauner
@ 2024-03-25 16:58   ` Darrick J. Wong
  2024-03-26 11:43     ` Christian Brauner
  0 siblings, 1 reply; 15+ messages in thread
From: Darrick J. Wong @ 2024-03-25 16:58 UTC (permalink / raw
  To: Christian Brauner, Dave Chinner
  Cc: kernel test robot, Taylor Jackson, oe-lkp, lkp,
	Linux Memory Management List, linux-fsdevel, xfs, fstests

On Tue, Feb 20, 2024 at 09:57:30AM +0100, Christian Brauner wrote:
> On Mon, Feb 19, 2024 at 02:55:42PM +0800, kernel test robot wrote:
> > 
> > 
> > Hello,
> > 
> > kernel test robot noticed "xfstests.generic.645.fail" on:
> > 
> > commit: b4291c7fd9e550b91b10c3d7787b9bf5be38de67 ("fs/mnt_idmapping.c: Return -EINVAL when no map is written")
> > https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git master
> 
> The test needs to be updated. We now explicitly fail when no map is
> written.

Has there been any progress on updating generic/645?  6.9-rc1 is out,
and Dave and I have both noticed this regressing.

--D

> > 
> > [test failed on linux-next/master d37e1e4c52bc60578969f391fb81f947c3e83118]
> > 
> > in testcase: xfstests
> > version: xfstests-x86_64-c46ca4d1-1_20240205
> > with following parameters:
> > 
> > 	disk: 4HDD
> > 	fs: f2fs
> > 	test: generic-645
> > 
> > 
> > 
> > compiler: gcc-12
> > test machine: 4 threads Intel(R) Core(TM) i5-6500 CPU @ 3.20GHz (Skylake) with 32G memory
> > 
> > (please refer to attached dmesg/kmsg for entire log/backtrace)
> > 
> > 
> > 
> > 
> > If you fix the issue in a separate patch/commit (i.e. not just a new version of
> > the same patch/commit), kindly add following tags
> > | Reported-by: kernel test robot <oliver.sang@intel.com>
> > | Closes: https://lore.kernel.org/oe-lkp/202402191416.17ec9160-oliver.sang@intel.com
> > 
> > 2024-02-17 15:04:19 export TEST_DIR=/fs/sda1
> > 2024-02-17 15:04:19 export TEST_DEV=/dev/sda1
> > 2024-02-17 15:04:19 export FSTYP=f2fs
> > 2024-02-17 15:04:19 export SCRATCH_MNT=/fs/scratch
> > 2024-02-17 15:04:19 mkdir /fs/scratch -p
> > 2024-02-17 15:04:19 export SCRATCH_DEV=/dev/sda4
> > 2024-02-17 15:04:19 export MKFS_OPTIONS=-f
> > 2024-02-17 15:04:19 echo generic/645
> > 2024-02-17 15:04:19 ./check generic/645
> > FSTYP         -- f2fs
> > PLATFORM      -- Linux/x86_64 lkp-skl-d03 6.8.0-rc1-00033-gb4291c7fd9e5 #1 SMP PREEMPT_DYNAMIC Sat Feb 17 22:11:35 CST 2024
> > MKFS_OPTIONS  -- -f /dev/sda4
> > MOUNT_OPTIONS -- -o acl,user_xattr /dev/sda4 /fs/scratch
> > 
> > generic/645       [failed, exit status 1]- output mismatch (see /lkp/benchmarks/xfstests/results//generic/645.out.bad)
> >     --- tests/generic/645.out	2024-02-05 17:37:40.000000000 +0000
> >     +++ /lkp/benchmarks/xfstests/results//generic/645.out.bad	2024-02-17 15:07:42.613312168 +0000
> >     @@ -1,2 +1,4 @@
> >      QA output created by 645
> >      Silence is golden
> >     +idmapped-mounts.c: 6671: nested_userns - Invalid argument - failure: sys_mount_setattr
> >     +vfstest.c: 2418: run_test - Invalid argument - failure: test that nested user namespaces behave correctly when attached to idmapped mounts
> >     ...
> >     (Run 'diff -u /lkp/benchmarks/xfstests/tests/generic/645.out /lkp/benchmarks/xfstests/results//generic/645.out.bad'  to see the entire diff)
> > Ran: generic/645
> > Failures: generic/645
> > Failed 1 of 1 tests
> > 
> > 
> > 
> > 
> > The kernel config and materials to reproduce are available at:
> > https://download.01.org/0day-ci/archive/20240219/202402191416.17ec9160-oliver.sang@intel.com
> > 
> > 
> > 
> > -- 
> > 0-DAY CI Kernel Test Service
> > https://github.com/intel/lkp-tests/wiki
> > 
> 

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

* Re: [linux-next:master] [fs/mnt_idmapping.c]  b4291c7fd9: xfstests.generic.645.fail
  2024-03-25 16:58   ` Darrick J. Wong
@ 2024-03-26 11:43     ` Christian Brauner
  2024-03-26 15:22       ` Darrick J. Wong
  0 siblings, 1 reply; 15+ messages in thread
From: Christian Brauner @ 2024-03-26 11:43 UTC (permalink / raw
  To: Darrick J. Wong
  Cc: Dave Chinner, kernel test robot, Taylor Jackson, oe-lkp, lkp,
	Linux Memory Management List, linux-fsdevel, xfs, fstests

On Mon, Mar 25, 2024 at 09:58:09AM -0700, Darrick J. Wong wrote:
> On Tue, Feb 20, 2024 at 09:57:30AM +0100, Christian Brauner wrote:
> > On Mon, Feb 19, 2024 at 02:55:42PM +0800, kernel test robot wrote:
> > > 
> > > 
> > > Hello,
> > > 
> > > kernel test robot noticed "xfstests.generic.645.fail" on:
> > > 
> > > commit: b4291c7fd9e550b91b10c3d7787b9bf5be38de67 ("fs/mnt_idmapping.c: Return -EINVAL when no map is written")
> > > https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git master
> > 
> > The test needs to be updated. We now explicitly fail when no map is
> > written.
> 
> Has there been any progress on updating generic/645?  6.9-rc1 is out,
> and Dave and I have both noticed this regressing.

Iirc, Taylor wanted to fix this but it seems that hasn't happened yet.
I'll ping again and if nothing's happened until tomorrow I'll send a
patch.

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

* Re: [linux-next:master] [fs/mnt_idmapping.c]  b4291c7fd9: xfstests.generic.645.fail
  2024-03-26 11:43     ` Christian Brauner
@ 2024-03-26 15:22       ` Darrick J. Wong
  2024-03-27 10:33         ` [PATCH 2/2] vfs/idmapped_mounts.c: Change mount_setattr expected output Christian Brauner
  0 siblings, 1 reply; 15+ messages in thread
From: Darrick J. Wong @ 2024-03-26 15:22 UTC (permalink / raw
  To: Christian Brauner
  Cc: Dave Chinner, kernel test robot, Taylor Jackson, oe-lkp, lkp,
	Linux Memory Management List, linux-fsdevel, xfs, fstests

On Tue, Mar 26, 2024 at 12:43:27PM +0100, Christian Brauner wrote:
> On Mon, Mar 25, 2024 at 09:58:09AM -0700, Darrick J. Wong wrote:
> > On Tue, Feb 20, 2024 at 09:57:30AM +0100, Christian Brauner wrote:
> > > On Mon, Feb 19, 2024 at 02:55:42PM +0800, kernel test robot wrote:
> > > > 
> > > > 
> > > > Hello,
> > > > 
> > > > kernel test robot noticed "xfstests.generic.645.fail" on:
> > > > 
> > > > commit: b4291c7fd9e550b91b10c3d7787b9bf5be38de67 ("fs/mnt_idmapping.c: Return -EINVAL when no map is written")
> > > > https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git master
> > > 
> > > The test needs to be updated. We now explicitly fail when no map is
> > > written.
> > 
> > Has there been any progress on updating generic/645?  6.9-rc1 is out,
> > and Dave and I have both noticed this regressing.
> 
> Iirc, Taylor wanted to fix this but it seems that hasn't happened yet.
> I'll ping again and if nothing's happened until tomorrow I'll send a
> patch.

Ok, glad to hear that this is still on your radar.  Thank you for
following up!

--D

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

* [PATCH 0/2] Patches to update xfs test "generic/645" for mount_setattr
@ 2024-03-26 20:33 ` Taylor Jackson
  0 siblings, 0 replies; 15+ messages in thread
From: Taylor Jackson via B4 Relay @ 2024-03-26 20:33 UTC (permalink / raw
  To: fstests; +Cc: Christian Brauner, Seth Forshee, Taylor Jackson,
	kernel test robot

The following patches fix the "generic/645" xfs test which was 
affected by the mount_setattr change in kernel commit dacfd001eaf2 
(“fs/mnt_idmapping.c: Return -EINVAL when no map is written”). 

Reported-by: kernel test robot <oliver.sang@intel.com>
Closes: https://lore.kernel.org/oe-lkp/202402191416.17ec9160-oliver.sang@intel.com
Signed-off-by: Taylor Jackson <tjackson9431@gmail.com>
---
Taylor Jackson (2):
      vfs/idmapped_mounts.c: Incorrect array index for nested user ns
      vfs/idmapped_mounts.c: Change mount_setattr expected output

 src/vfs/idmapped-mounts.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)
---
base-commit: 9b6df9a01ac8ee3f28a2a24d71e45792e21b6d48
change-id: 20240326-mount-setattr-test-90942470c820

Best regards,
-- 
Taylor Jackson <tjackson9431@gmail.com>



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

* [PATCH 0/2] Patches to update xfs test "generic/645" for mount_setattr
@ 2024-03-26 20:33 ` Taylor Jackson
  0 siblings, 0 replies; 15+ messages in thread
From: Taylor Jackson @ 2024-03-26 20:33 UTC (permalink / raw
  To: fstests; +Cc: Christian Brauner, Seth Forshee, Taylor Jackson,
	kernel test robot

The following patches fix the "generic/645" xfs test which was 
affected by the mount_setattr change in kernel commit dacfd001eaf2 
(“fs/mnt_idmapping.c: Return -EINVAL when no map is written”). 

Reported-by: kernel test robot <oliver.sang@intel.com>
Closes: https://lore.kernel.org/oe-lkp/202402191416.17ec9160-oliver.sang@intel.com
Signed-off-by: Taylor Jackson <tjackson9431@gmail.com>
---
Taylor Jackson (2):
      vfs/idmapped_mounts.c: Incorrect array index for nested user ns
      vfs/idmapped_mounts.c: Change mount_setattr expected output

 src/vfs/idmapped-mounts.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)
---
base-commit: 9b6df9a01ac8ee3f28a2a24d71e45792e21b6d48
change-id: 20240326-mount-setattr-test-90942470c820

Best regards,
-- 
Taylor Jackson <tjackson9431@gmail.com>


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

* [PATCH 1/2] vfs/idmapped_mounts.c: Incorrect array index for nested user ns
  2024-03-26 20:33 ` Taylor Jackson
@ 2024-03-26 20:33   ` Taylor Jackson
  -1 siblings, 0 replies; 15+ messages in thread
From: Taylor Jackson via B4 Relay @ 2024-03-26 20:33 UTC (permalink / raw
  To: fstests; +Cc: Christian Brauner, Seth Forshee, Taylor Jackson,
	kernel test robot

From: Taylor Jackson <tjackson9431@gmail.com>

Within the vfs test for idmapped mounts, the function nested_userns()
is using an incorrect array index when attempting to set up the mapping
for the 4th nested user ns within hierarchy[4]. The correct index that
belongs to the 4th nested user ns is actually hierarchy[3].
And hierarchy[4] is reserved for the dummy entry that marks the end
of the array.

Signed-off-by: Taylor Jackson <tjackson9431@gmail.com>
---
 src/vfs/idmapped-mounts.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/vfs/idmapped-mounts.c b/src/vfs/idmapped-mounts.c
index 547182fe..34052ca3 100644
--- a/src/vfs/idmapped-mounts.c
+++ b/src/vfs/idmapped-mounts.c
@@ -6556,7 +6556,7 @@ static int nested_userns(const struct vfstest_info *info)
 	}
 
 	/* Don't write a mapping in the 4th userns. */
-	list_empty(&hierarchy[4].id_map);
+	list_empty(&hierarchy[3].id_map);
 
 	/* Create the actual userns hierarchy. */
 	ret = create_userns_hierarchy(hierarchy);

-- 
2.34.1



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

* [PATCH 1/2] vfs/idmapped_mounts.c: Incorrect array index for nested user ns
@ 2024-03-26 20:33   ` Taylor Jackson
  0 siblings, 0 replies; 15+ messages in thread
From: Taylor Jackson @ 2024-03-26 20:33 UTC (permalink / raw
  To: fstests; +Cc: Christian Brauner, Seth Forshee, Taylor Jackson,
	kernel test robot

Within the vfs test for idmapped mounts, the function nested_userns()
is using an incorrect array index when attempting to set up the mapping
for the 4th nested user ns within hierarchy[4]. The correct index that
belongs to the 4th nested user ns is actually hierarchy[3].
And hierarchy[4] is reserved for the dummy entry that marks the end
of the array.

Signed-off-by: Taylor Jackson <tjackson9431@gmail.com>
---
 src/vfs/idmapped-mounts.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/vfs/idmapped-mounts.c b/src/vfs/idmapped-mounts.c
index 547182fe..34052ca3 100644
--- a/src/vfs/idmapped-mounts.c
+++ b/src/vfs/idmapped-mounts.c
@@ -6556,7 +6556,7 @@ static int nested_userns(const struct vfstest_info *info)
 	}
 
 	/* Don't write a mapping in the 4th userns. */
-	list_empty(&hierarchy[4].id_map);
+	list_empty(&hierarchy[3].id_map);
 
 	/* Create the actual userns hierarchy. */
 	ret = create_userns_hierarchy(hierarchy);

-- 
2.34.1


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

* [PATCH 2/2] vfs/idmapped_mounts.c: Change mount_setattr expected output
  2024-03-26 20:33 ` Taylor Jackson
@ 2024-03-26 20:33   ` Taylor Jackson
  -1 siblings, 0 replies; 15+ messages in thread
From: Taylor Jackson via B4 Relay @ 2024-03-26 20:33 UTC (permalink / raw
  To: fstests; +Cc: Christian Brauner, Seth Forshee, Taylor Jackson,
	kernel test robot

From: Taylor Jackson <tjackson9431@gmail.com>

In kernel commit dacfd001eaf2 (“fs/mnt_idmapping.c: Return -EINVAL
when no map is written”), the behavior of mount_setattr changed to
return EINVAL when attempting to create an idmapped mount when using
a user namespace with no mappings. The following commit updates the test
to expect no mount to be created in that case. And since no mount is created,
this commit also removes the check for overflow IDs because it does not make
sense to check for overflow IDs for a mount that was not created.

Signed-off-by: Taylor Jackson <tjackson9431@gmail.com>
---
 src/vfs/idmapped-mounts.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/src/vfs/idmapped-mounts.c b/src/vfs/idmapped-mounts.c
index 34052ca3..f4dfc3f3 100644
--- a/src/vfs/idmapped-mounts.c
+++ b/src/vfs/idmapped-mounts.c
@@ -6667,7 +6667,7 @@ static int nested_userns(const struct vfstest_info *info)
 	}
 
 	if (sys_mount_setattr(fd_open_tree_level4, "", AT_EMPTY_PATH,
-			      &attr_level4, sizeof(attr_level4))) {
+			      &attr_level4, sizeof(attr_level4)) != -1 || errno != EINVAL) {
 		log_stderr("failure: sys_mount_setattr");
 		goto out;
 	}
@@ -6706,11 +6706,6 @@ static int nested_userns(const struct vfstest_info *info)
 			log_stderr("failure: check ownership %s", file);
 			goto out;
 		}
-
-		if (!expected_uid_gid(fd_open_tree_level4, file, 0, info->t_overflowuid, info->t_overflowgid)) {
-			log_stderr("failure: check ownership %s", file);
-			goto out;
-		}
 	}
 
 	/* Verify that ownership looks correct for callers in the first userns. */

-- 
2.34.1



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

* [PATCH 2/2] vfs/idmapped_mounts.c: Change mount_setattr expected output
@ 2024-03-26 20:33   ` Taylor Jackson
  0 siblings, 0 replies; 15+ messages in thread
From: Taylor Jackson @ 2024-03-26 20:33 UTC (permalink / raw
  To: fstests; +Cc: Christian Brauner, Seth Forshee, Taylor Jackson,
	kernel test robot

In kernel commit dacfd001eaf2 (“fs/mnt_idmapping.c: Return -EINVAL
when no map is written”), the behavior of mount_setattr changed to
return EINVAL when attempting to create an idmapped mount when using
a user namespace with no mappings. The following commit updates the test
to expect no mount to be created in that case. And since no mount is created,
this commit also removes the check for overflow IDs because it does not make
sense to check for overflow IDs for a mount that was not created.

Signed-off-by: Taylor Jackson <tjackson9431@gmail.com>
---
 src/vfs/idmapped-mounts.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/src/vfs/idmapped-mounts.c b/src/vfs/idmapped-mounts.c
index 34052ca3..f4dfc3f3 100644
--- a/src/vfs/idmapped-mounts.c
+++ b/src/vfs/idmapped-mounts.c
@@ -6667,7 +6667,7 @@ static int nested_userns(const struct vfstest_info *info)
 	}
 
 	if (sys_mount_setattr(fd_open_tree_level4, "", AT_EMPTY_PATH,
-			      &attr_level4, sizeof(attr_level4))) {
+			      &attr_level4, sizeof(attr_level4)) != -1 || errno != EINVAL) {
 		log_stderr("failure: sys_mount_setattr");
 		goto out;
 	}
@@ -6706,11 +6706,6 @@ static int nested_userns(const struct vfstest_info *info)
 			log_stderr("failure: check ownership %s", file);
 			goto out;
 		}
-
-		if (!expected_uid_gid(fd_open_tree_level4, file, 0, info->t_overflowuid, info->t_overflowgid)) {
-			log_stderr("failure: check ownership %s", file);
-			goto out;
-		}
 	}
 
 	/* Verify that ownership looks correct for callers in the first userns. */

-- 
2.34.1


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

* Re: [PATCH 1/2] vfs/idmapped_mounts.c: Incorrect array index for nested user ns
  2024-03-26 20:33   ` Taylor Jackson
  (?)
@ 2024-03-27 10:31   ` Christian Brauner
  -1 siblings, 0 replies; 15+ messages in thread
From: Christian Brauner @ 2024-03-27 10:31 UTC (permalink / raw
  To: tjackson9431; +Cc: fstests, Seth Forshee, kernel test robot

On Tue, Mar 26, 2024 at 08:33:51PM +0000, Taylor Jackson via B4 Relay wrote:
> From: Taylor Jackson <tjackson9431@gmail.com>
> 
> Within the vfs test for idmapped mounts, the function nested_userns()
> is using an incorrect array index when attempting to set up the mapping
> for the 4th nested user ns within hierarchy[4]. The correct index that
> belongs to the 4th nested user ns is actually hierarchy[3].
> And hierarchy[4] is reserved for the dummy entry that marks the end
> of the array.
> 
> Signed-off-by: Taylor Jackson <tjackson9431@gmail.com>
> ---

Ah, thanks for fixing this!
Reviewed-by: Christian Brauner <brauner@kernel.org>

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

* Re: [PATCH 2/2] vfs/idmapped_mounts.c: Change mount_setattr expected output
  2024-03-26 15:22       ` Darrick J. Wong
@ 2024-03-27 10:33         ` Christian Brauner
  0 siblings, 0 replies; 15+ messages in thread
From: Christian Brauner @ 2024-03-27 10:33 UTC (permalink / raw
  To: tjackson9431, Darrick J. Wong
  Cc: fstests, Seth Forshee, kernel test robot, Dave Chinner,
	Taylor Jackson, oe-lkp, lkp, Linux Memory Management List,
	linux-fsdevel, xfs

On Tue, Mar 26, 2024 at 08:33:52PM +0000, Taylor Jackson via B4 Relay wrote:
> From: Taylor Jackson <tjackson9431@gmail.com>
> 
> In kernel commit dacfd001eaf2 (“fs/mnt_idmapping.c: Return -EINVAL
> when no map is written”), the behavior of mount_setattr changed to
> return EINVAL when attempting to create an idmapped mount when using
> a user namespace with no mappings. The following commit updates the test
> to expect no mount to be created in that case. And since no mount is created,
> this commit also removes the check for overflow IDs because it does not make
> sense to check for overflow IDs for a mount that was not created.
> 
> Signed-off-by: Taylor Jackson <tjackson9431@gmail.com>
> ---

Thanks for fixing this!
Reviewed-by: Christian Brauner <brauner@kernel.org>

>  src/vfs/idmapped-mounts.c | 7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)
> 
> diff --git a/src/vfs/idmapped-mounts.c b/src/vfs/idmapped-mounts.c
> index 34052ca3..f4dfc3f3 100644
> --- a/src/vfs/idmapped-mounts.c
> +++ b/src/vfs/idmapped-mounts.c
> @@ -6667,7 +6667,7 @@ static int nested_userns(const struct vfstest_info *info)
>  	}
>  
>  	if (sys_mount_setattr(fd_open_tree_level4, "", AT_EMPTY_PATH,
> -			      &attr_level4, sizeof(attr_level4))) {
> +			      &attr_level4, sizeof(attr_level4)) != -1 || errno != EINVAL) {
>  		log_stderr("failure: sys_mount_setattr");
>  		goto out;
>  	}
> @@ -6706,11 +6706,6 @@ static int nested_userns(const struct vfstest_info *info)
>  			log_stderr("failure: check ownership %s", file);
>  			goto out;
>  		}
> -
> -		if (!expected_uid_gid(fd_open_tree_level4, file, 0, info->t_overflowuid, info->t_overflowgid)) {
> -			log_stderr("failure: check ownership %s", file);
> -			goto out;
> -		}
>  	}
>  
>  	/* Verify that ownership looks correct for callers in the first userns. */
> 
> -- 
> 2.34.1
> 
> 

On Tue, Mar 26, 2024 at 08:22:28AM -0700, Darrick J. Wong wrote:
> On Tue, Mar 26, 2024 at 12:43:27PM +0100, Christian Brauner wrote:
> > On Mon, Mar 25, 2024 at 09:58:09AM -0700, Darrick J. Wong wrote:
> > > On Tue, Feb 20, 2024 at 09:57:30AM +0100, Christian Brauner wrote:
> > > > On Mon, Feb 19, 2024 at 02:55:42PM +0800, kernel test robot wrote:
> > > > > 
> > > > > 
> > > > > Hello,
> > > > > 
> > > > > kernel test robot noticed "xfstests.generic.645.fail" on:
> > > > > 
> > > > > commit: b4291c7fd9e550b91b10c3d7787b9bf5be38de67 ("fs/mnt_idmapping.c: Return -EINVAL when no map is written")
> > > > > https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git master
> > > > 
> > > > The test needs to be updated. We now explicitly fail when no map is
> > > > written.
> > > 
> > > Has there been any progress on updating generic/645?  6.9-rc1 is out,
> > > and Dave and I have both noticed this regressing.
> > 
> > Iirc, Taylor wanted to fix this but it seems that hasn't happened yet.
> > I'll ping again and if nothing's happened until tomorrow I'll send a
> > patch.
> 
> Ok, glad to hear that this is still on your radar.  Thank you for
> following up!

@Darrick, Taylor sent fixes for this now (I've took the liberty to
respond to both mails combined.).

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

* Re: [PATCH 2/2] vfs/idmapped_mounts.c: Change mount_setattr expected output
  2024-03-26 20:33   ` Taylor Jackson
  (?)
@ 2024-03-27 13:27   ` Zorro Lang
  -1 siblings, 0 replies; 15+ messages in thread
From: Zorro Lang @ 2024-03-27 13:27 UTC (permalink / raw
  To: tjackson9431; +Cc: fstests

On Tue, Mar 26, 2024 at 08:33:52PM +0000, Taylor Jackson via B4 Relay wrote:
> From: Taylor Jackson <tjackson9431@gmail.com>
> 
> In kernel commit dacfd001eaf2 (“fs/mnt_idmapping.c: Return -EINVAL
> when no map is written”), the behavior of mount_setattr changed to
> return EINVAL when attempting to create an idmapped mount when using
> a user namespace with no mappings. The following commit updates the test
> to expect no mount to be created in that case. And since no mount is created,
> this commit also removes the check for overflow IDs because it does not make
> sense to check for overflow IDs for a mount that was not created.
> 
> Signed-off-by: Taylor Jackson <tjackson9431@gmail.com>
> ---

Thanks for fixing it!

Reviewed-by: Zorro Lang <zlang@redhat.com>

>  src/vfs/idmapped-mounts.c | 7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)
> 
> diff --git a/src/vfs/idmapped-mounts.c b/src/vfs/idmapped-mounts.c
> index 34052ca3..f4dfc3f3 100644
> --- a/src/vfs/idmapped-mounts.c
> +++ b/src/vfs/idmapped-mounts.c
> @@ -6667,7 +6667,7 @@ static int nested_userns(const struct vfstest_info *info)
>  	}
>  
>  	if (sys_mount_setattr(fd_open_tree_level4, "", AT_EMPTY_PATH,
> -			      &attr_level4, sizeof(attr_level4))) {
> +			      &attr_level4, sizeof(attr_level4)) != -1 || errno != EINVAL) {
>  		log_stderr("failure: sys_mount_setattr");
>  		goto out;
>  	}
> @@ -6706,11 +6706,6 @@ static int nested_userns(const struct vfstest_info *info)
>  			log_stderr("failure: check ownership %s", file);
>  			goto out;
>  		}
> -
> -		if (!expected_uid_gid(fd_open_tree_level4, file, 0, info->t_overflowuid, info->t_overflowgid)) {
> -			log_stderr("failure: check ownership %s", file);
> -			goto out;
> -		}
>  	}
>  
>  	/* Verify that ownership looks correct for callers in the first userns. */
> 
> -- 
> 2.34.1
> 
> 
> 


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

* Re: [PATCH 2/2] vfs/idmapped_mounts.c: Change mount_setattr expected output
  2024-03-26 20:33   ` Taylor Jackson
  (?)
  (?)
@ 2024-04-08  6:18   ` Zorro Lang
  -1 siblings, 0 replies; 15+ messages in thread
From: Zorro Lang @ 2024-04-08  6:18 UTC (permalink / raw
  To: tjackson9431; +Cc: fstests, Christian Brauner

On Tue, Mar 26, 2024 at 08:33:52PM +0000, Taylor Jackson via B4 Relay wrote:
> From: Taylor Jackson <tjackson9431@gmail.com>
> 
> In kernel commit dacfd001eaf2 (“fs/mnt_idmapping.c: Return -EINVAL
> when no map is written”), the behavior of mount_setattr changed to
> return EINVAL when attempting to create an idmapped mount when using
> a user namespace with no mappings. The following commit updates the test
> to expect no mount to be created in that case. And since no mount is created,
> this commit also removes the check for overflow IDs because it does not make
> sense to check for overflow IDs for a mount that was not created.
> 
> Signed-off-by: Taylor Jackson <tjackson9431@gmail.com>
> ---
>  src/vfs/idmapped-mounts.c | 7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)
> 
> diff --git a/src/vfs/idmapped-mounts.c b/src/vfs/idmapped-mounts.c
> index 34052ca3..f4dfc3f3 100644
> --- a/src/vfs/idmapped-mounts.c
> +++ b/src/vfs/idmapped-mounts.c
> @@ -6667,7 +6667,7 @@ static int nested_userns(const struct vfstest_info *info)
>  	}
>  
>  	if (sys_mount_setattr(fd_open_tree_level4, "", AT_EMPTY_PATH,
> -			      &attr_level4, sizeof(attr_level4))) {
> +			      &attr_level4, sizeof(attr_level4)) != -1 || errno != EINVAL) {

Hi Taylor,

This patch has been merged, but I found it causes g/645 fail on old kernel[1].
Is it expected?

If it's an expected failure on kernel without dacfd001eaf2 (“fs/mnt_idmapping.c:
Return -EINVAL when no map is written”), we'd better to add this commit to
generic/645 as a hint, to avoid others feel they hit a regression.

Thanks,
Zorro

[1]
FSTYP         -- xfs (non-debug)
PLATFORM      -- Linux/x86_64 sheep-3 5.14.0-xxx.el9 #1 SMP PREEMPT_DYNAMIC Fri Apr 5 12:21:04 EDT 2024
MKFS_OPTIONS  -- -f -m crc=1,finobt=1,reflink=1,rmapbt=0,bigtime=1,inobtcount=1 /dev/vda3
MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /dev/vda3 /mnt/xfstests/scratch

generic/645       [failed, exit status 1]- output mismatch (see /var/lib/xfstests/results//generic/645.out.bad)
    --- tests/generic/645.out	2024-04-08 07:27:51.432955372 +0200
    +++ /var/lib/xfstests/results//generic/645.out.bad	2024-04-08 07:31:49.811581285 +0200
    @@ -1,2 +1,4 @@
     QA output created by 645
     Silence is golden
    +idmapped-mounts.c: 6671: nested_userns - Success - failure: sys_mount_setattr
    +vfstest.c: 2418: run_test - Success - failure: test that nested user namespaces behave correctly when attached to idmapped mounts
    ...
    (Run 'diff -u /var/lib/xfstests/tests/generic/645.out /var/lib/xfstests/results//generic/645.out.bad'  to see the entire diff)
Ran: generic/645
Failures: generic/645
Failed 1 of 1 tests

>  		log_stderr("failure: sys_mount_setattr");
>  		goto out;
>  	}
> @@ -6706,11 +6706,6 @@ static int nested_userns(const struct vfstest_info *info)
>  			log_stderr("failure: check ownership %s", file);
>  			goto out;
>  		}
> -
> -		if (!expected_uid_gid(fd_open_tree_level4, file, 0, info->t_overflowuid, info->t_overflowgid)) {
> -			log_stderr("failure: check ownership %s", file);
> -			goto out;
> -		}
>  	}
>  
>  	/* Verify that ownership looks correct for callers in the first userns. */
> 
> -- 
> 2.34.1
> 
> 
> 


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

end of thread, other threads:[~2024-04-08  6:18 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-02-19  6:55 [linux-next:master] [fs/mnt_idmapping.c] b4291c7fd9: xfstests.generic.645.fail kernel test robot
2024-02-20  8:57 ` Christian Brauner
2024-03-25 16:58   ` Darrick J. Wong
2024-03-26 11:43     ` Christian Brauner
2024-03-26 15:22       ` Darrick J. Wong
2024-03-27 10:33         ` [PATCH 2/2] vfs/idmapped_mounts.c: Change mount_setattr expected output Christian Brauner
  -- strict thread matches above, loose matches on Subject: below --
2024-03-26 20:33 [PATCH 0/2] Patches to update xfs test "generic/645" for mount_setattr Taylor Jackson via B4 Relay
2024-03-26 20:33 ` Taylor Jackson
2024-03-26 20:33 ` [PATCH 1/2] vfs/idmapped_mounts.c: Incorrect array index for nested user ns Taylor Jackson via B4 Relay
2024-03-26 20:33   ` Taylor Jackson
2024-03-27 10:31   ` Christian Brauner
2024-03-26 20:33 ` [PATCH 2/2] vfs/idmapped_mounts.c: Change mount_setattr expected output Taylor Jackson via B4 Relay
2024-03-26 20:33   ` Taylor Jackson
2024-03-27 13:27   ` Zorro Lang
2024-04-08  6:18   ` Zorro Lang

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.