Re: [BUG] propagation graph breakage by MOVE_MOUNT_SET_GROUP move_mount(2)

[Al Viro <viro@zeniv.linux.org.uk>]

On Fri, May 09, 2025 at 09:26:28AM +0100, Al Viro wrote:
> AFAICS, 9ffb14ef61ba "move_mount: allow to add a mount into an existing
> group" breaks assertions on ->mnt_share/->mnt_slave.  For once, the data
> structures in question are actually documented.
> 
> Documentation/filesystem/sharedsubtree.rst:
>         All vfsmounts in a peer group have the same ->mnt_master.  If it is
> 	non-NULL, they form a contiguous (ordered) segment of slave list.
> 
> fs/pnode.c:
>  * Note that peer groups form contiguous segments of slave lists.
> 
> fs/namespace.c:do_set_group():
>         if (IS_MNT_SLAVE(from)) {
>                 struct mount *m = from->mnt_master;
> 
>                 list_add(&to->mnt_slave, &m->mnt_slave_list);
>                 to->mnt_master = m;
>         }
> 
>         if (IS_MNT_SHARED(from)) {
>                 to->mnt_group_id = from->mnt_group_id;
>                 list_add(&to->mnt_share, &from->mnt_share);
>                 lock_mount_hash();
>                 set_mnt_shared(to);
>                 unlock_mount_hash();
>         }
> 
> Note that 'to' goes right after 'from' in ->mnt_share (i.e. peer group
> list) and into the beginning of the slave list 'from' belongs to.  IOW,
> contiguity gets broken if 'from' is both IS_MNT_SLAVE and IS_MNT_SHARED.
> Which is what happens when the peer group 'from' is in gets propagation
> from somewhere.
> 
> It's not hard to fix - something like
> 
>         if (IS_MNT_SHARED(from)) {
> 		to->mnt_group_id = from->mnt_group_id;
>                 list_add(&to->mnt_share, &from->mnt_share);
> 		if (IS_MNT_SLAVE(from))
> 			list_add(&to->mnt_slave, &from->mnt_slave);
> 		to->mnt_master = from->mnt_master;
>                 lock_mount_hash();
>                 set_mnt_shared(to);
>                 unlock_mount_hash();
>         } else if (IS_MNT_SLAVE(from)) {
> 		to->mnt_master = from->mnt_master;
> 		list_add(&to->mnt_slave, &from->mnt_master->mnt_slave_list);
> 	}
> 
> ought to do it.  I'm nowhere near sufficiently awake right now to put
> together a regression test, but unless I'm missing something subtle, it
> should be possible to get a fairly obvious breakage of propagate_mnt()
> out of that...

Not sufficiently awake is right - wrong address on Cc...  Anyway, bedtime
for me...