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