On 30/10/23 13:37, Ian Kent wrote:

On 28/10/23 09:36, Ian Kent wrote:

quoted

On 27/10/23 16:17, Miklos Szeredi wrote:

quoted

On Fri, Oct 27, 2023 at 5:12 AM Ian Kent [off-list ref] wrote:

quoted

On 25/10/23 22:02, Miklos Szeredi wrote:

quoted

The mnt.mnt_list is still used to set up the mount tree and for
propagation, but not after the mount has been added to a 
namespace.  Hence
mnt_list can live in union with rb_node.  Use MNT_ONRB mount flag to
validate that the mount is on the correct list.

Is that accurate, propagation occurs at mount and also at umount.

When propagating a mount, the new mount's mnt_list is used as a head
for the new propagated mounts.  These are then moved to the rb tree by
commit_tree().

When umounting there's a "to umount" list called tmp_list in
umount_tree(), this list is used to collect direct umounts and then
propagated umounts.  The direct umounts are added in umount_tree(),
the propagated ones umount_one().

Note: umount_tree() can be called on a not yet finished mount, in that
case the mounts are still on mnt_list, so umount_tree() needs to deal
with both.

quoted

IDG how the change to umount_one() works, it looks like umount_list()

uses mnt_list. It looks like propagate_umount() is also using 
mnt_list.


Am I missing something obvious?

So when a mount is part of a namespace (either anonymous or not) it is
on the rb tree, when not then it can temporarily be on mnt_list.
MNT_ONRB flag is used to validate that the mount is on the list that
we expect it to be on, but also to detect the case of the mount setup
being aborted.

We could handle the second case differently, since we should be able
to tell when we are removing the mount from a namespace and when we
are aborting a mount, but this was the least invasive way to do this.

Thanks for the explanation, what you've said is essentially what I

understood reading the series.


But I still haven't quite got this so I'll need to spend more time

on this part of the patch series.


That's not a problem, ;).

After cloning your git tree and looking in there I don't see what

I was concerned about so I think I was confused by obscurity by

diff rather than seeing a real problem, ;)


Still that union worries me a little bit so I'll keep looking at

the code for a while.

Is fs/namespace.c:iterate_mounts() a problem?

It's called from:

1) ./kernel/audit_tree.c:709: if (iterate_mounts(compare_root,
2) ./kernel/audit_tree.c:839:    err = iterate_mounts(tag_mount, tree, mnt);
3) ./kernel/audit_tree.c:917:        failed = iterate_mounts(tag_mount, 
tree, tagged);


 From functions 1) audit_trim_trees(), 2) audit_add_tree_rule() and

3) audit_tag_tree().

quoted

Ian

quoted

Thanks,
Miklos