On 2026-06-18 18:56:42+08:00, Song Liu wrote: > On Wed, Jun 17, 2026 at 9:53 PM Christian Brauner <[email protected]> wrote: > > > On Thu, May 28, 2026 at 11:26:06AM -0700, Song Liu wrote: > > [...] > > > > > > > > This again is racy as it is called outside of the namespace semaphore: > > > > err = security_mount_bind(&old_path, path, recurse); > > if (err) > > return err; > > > > if (mnt_ns_loop(old_path.dentry)) > > return -EINVAL; > > > > LOCK_MOUNT(mp, path); > > if (IS_ERR(mp.parent)) > > return PTR_ERR(mp.parent); > > > > After LOCK_MOUNT @path might point to a completely different mount then > > the one you performed your security checks on. > > I thought we agreed at LSF/MM/BPF 2026 to add the LSM hooks > before taking namespace semaphore, so that it is possible for LSMs > to defend against DoS attacks on namespace semaphore? Did I > miss/misunderstand something?
I think there was a misunderstanding. What I pointed out was that it's a trade-off. If we do call security hooks under the namespace semaphore or mount lock than anything that's called under there must take care to not cause deadlocks - which is especially easy to do with mount lock and even with the namespace semaphore it may get hairy (automounts etc). The dos thing is another worry but if an LSM does stupid things we tell it to not do stupid things and to go away. But as the hooks are done right now they are meaningless from a security perspective. You might have a policy that allows mounting on dentry_a and deny mounting on dentry_b: before LOCK_MOUNT*() you may see dentry_a and allow the mount but after LOCK_MOUNT*() someone raced you and shoved a dentry_b mount onto dentry_b and now you allow overmounting dentry_b which your policy didn't allow -> hosed. > > Placement of this hook suffers from the same issue as the bind mount > > hook. Here it's worse because the security layer isn't even informed > > about MOVE_MOUNT_BENEATH which completely alters the mount relationship. > > Current hook security_move_mount doesn't handle > MOVE_MOUNT_BENEATH. But we can add mflags to security_mount_move(). > Do we need anything other than mflags? I think you either need to pass three mounts (source, target, top_mnt) where for non-mount beneath target == top_mnt or you need two separate hooks. Because for MOVE_MOUNT_BENEATH you may want to have a tri-part policy: source, target, top_mnt.
