On Sat, Jun 25, 2022 at 06:25:40PM +0200, Christian Brauner wrote: > On Sat, Jun 25, 2022 at 11:17:15PM +0800, Zorro Lang wrote: > > On Wed, Jun 15, 2022 at 11:28:26AM +0200, Christian Brauner wrote: > > > When group ownership is changed a caller whose fsuid owns the inode can > > > change the group of the inode to any group they are a member of. When > > > searching through the caller's groups we failed to use the gid mapped > > > according to the idmapped mount otherwise we fail to change ownership. > > > Add a test for this. > > > > > > Cc: Seth Forshee <sforshee@xxxxxxxxxxxxxxxx> > > > Cc: Amir Goldstein <amir73il@xxxxxxxxx> > > > Cc: Christoph Hellwig <hch@xxxxxx> > > > Cc: Aleksa Sarai <cyphar@xxxxxxxxxx> > > > Cc: <fstests@xxxxxxxxxxxxxxx> > > > Signed-off-by: Christian Brauner (Microsoft) <brauner@xxxxxxxxxx> > > > --- > > > /* v2 */ > > > - Zorro Lang <zlang@xxxxxxxxxx>: > > > - various minor fixes > > > > > > - Christian Brauner (Microsoft) <brauner@xxxxxxxxxx>: > > > - Expand test to also cover overlayfs on top of idmapped mounts. > > > > > > /* v3 */ > > > - Amir Goldstein <amir73il@xxxxxxxxx>: > > > - Switch from calls to the mount binary to _overlay_mount_dirs helper. > > > > > > - Christian Brauner (Microsoft) <brauner@xxxxxxxxxx>: > > > - Expand test to also cover non-idmapped mounts. > > > --- > > > > We really make this test case become more complicated than its v1 version, > > for covering overlayfs on top of idmapped mounts. > > > > I'm wondering if it's worth splitting this case to two cases, one similar > > as v1 to cover the original regression test, the other expand for overlayfs, > > although that will cause some duplicate testing steps use? > > Both approaches would work. I chose to combine them because it's the > same regression just under different circumstances and because it avoids > duplicated code. > > > > > If most of you prefer this v3 version, I'd like to get more review points. > > If you want to wait for additional acks then sure. But I'd like to not > put merging this off for too long. > I prefer to merge regression tests quickly so they are run as soon as > possible especially since the fix is already upstream for about 2 weeks > now. I'd like to split this test to two cases for two reasons: 1) Reduce the complexity of this case. 2) That overlayfs test part need a separated overlayfs idmapped feature which is not necessary to be a limitation of this regression test, cause this case can't be run on older kernel. And that: _supports_filetype $SCRATCH_MNT || _notrun "overlayfs test requires d_type" line should be called after _scratch_mount, due to $SCRATCH_MNT might not be mounted before _scratch_mount. Thanks, Zorro > > Christian >
