On Fri, 6 Oct 2023 at 04:56, Paul Moore <paul@xxxxxxxxxxxxxx> wrote: > > Also I cannot see the point in hiding some mount ID's from the list. > > It seems to me that the list is just an array of numbers that in > > itself doesn't carry any information. > > I think it really comes down to the significance of the mount ID, and > I can't say I know enough of the details here to be entirely > comfortable taking a hard stance on this. Can you help me understand > the mount ID concept a bit better? Mount ID is a descriptor that allows referring to a specific struct mount from userspace. The old 32 bit mount id is allocated with IDA from a global pool. Because it's non-referencing it doesn't allow uniquely identifying a mount. That was a design mistake that I made back in 2008, thinking that the same sort of dense descriptor space as used for file descriptors would work. Originally it was used to identify the mount and the parent mount in /proc/PID/mountinfo. Later it was also added to the following interfaces: - name_to_handle_at(2) returns 32 bit value - /proc/PID/FD/fdinfo - statx(2) returns 64 bit value It was never used on the kernel interfaces as an input argument. statmount(2) and listmount(2) require the mount to be identified by userspace, so having a unique ID is important. So the "[1/4] add unique mount ID" adds a new 64 bit ID (still global) that is allocated sequentially and only reused after reboot. It is used as an input to these syscalls. It is returned by statx(2) if requested by STATX_MNT_ID_UNIQUE and as an array of ID's by listmount(2). I can see mild security problems with the global allocation, since a task can observe mounts being done in other namespaces. This doesn't sound too serious, and the old ID has similar issues. But I think making the new ID be local to the mount namespace is also feasible. > While I'm reasonably confident that we want a security_sb_statfs() > control point in statmount(), it may turn out that we don't want/need > a call in the listmount() case. Perhaps your original patch was > correct in the sense that we only want a single security_sb_statfs() > call for the root (implying that the child mount IDs are attributes of > the root/parent mount)? Maybe it's something else entirely? Mounts are arranged in a tree (I think it obvious how) and listmount(2) just lists the IDs of the immediate children of a mount. I don't see ID being an attribute of a mount, it's a descriptor. Thanks, Miklos
