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. Christian
