[PATCH v5 7/8] vfs: Replace security_sb_mount/security_move_mount with granular hooks

[PATCH v5 7/8] vfs: Replace security_sb_mount/security_move_mount with granular hooks

[Song Liu]

Replace the monolithic security_sb_mount() call in path_mount() and
security_move_mount() in vfs_move_mount() with the new granular mount
hooks:

- do_loopback(): call security_mount_bind()
- do_new_mount(): call security_mount_new()
- do_remount(): call security_mount_remount()
- do_reconfigure_mnt(): call security_mount_reconfigure()
- do_move_mount_old(): call security_mount_move()
- do_change_type(): call security_mount_change_type()
- vfs_move_mount(): replace security_move_mount() with
  security_mount_move()

The new hooks are called at the individual operation level with
appropriate context (resolved paths, fs_context), rather than at
the top of path_mount() with raw string arguments.

Code generated with the assistance of Claude, reviewed by human.

Reviewed-by: Stephen Smalley <stephen.smalley.work@gmail.com>
Tested-by: Stephen Smalley <stephen.smalley.work@gmail.com> # for selinux only
Signed-off-by: Song Liu <song@kernel.org>
---
 fs/namespace.c | 41 ++++++++++++++++++++++++++++++-----------
 1 file changed, 30 insertions(+), 11 deletions(-)

diff --git a/fs/namespace.c b/fs/namespace.c
index fe919abd2f01..43f22c5e2bf4 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -2888,6 +2888,10 @@ static int do_change_type(const struct path *path, int ms_flags)
 	if (!type)
 		return -EINVAL;
 
+	err = security_mount_change_type(path, ms_flags);
+	if (err)
+		return err;
+
 	guard(namespace_excl)();
 
 	err = may_change_propagation(mnt);
@@ -3006,6 +3010,10 @@ static int do_loopback(const struct path *path, const char *old_name,
 	if (err)
 		return err;
 
+	err = security_mount_bind(&old_path, path, recurse);
+	if (err)
+		return err;
+
 	if (mnt_ns_loop(old_path.dentry))
 		return -EINVAL;
 
@@ -3328,7 +3336,8 @@ static void mnt_warn_timestamp_expiry(const struct path *mountpoint,
  * superblock it refers to.  This is triggered by specifying MS_REMOUNT|MS_BIND
  * to mount(2).
  */
-static int do_reconfigure_mnt(const struct path *path, unsigned int mnt_flags)
+static int do_reconfigure_mnt(const struct path *path, unsigned int mnt_flags,
+			      unsigned long flags)
 {
 	struct super_block *sb = path->mnt->mnt_sb;
 	struct mount *mnt = real_mount(path->mnt);
@@ -3343,6 +3352,10 @@ static int do_reconfigure_mnt(const struct path *path, unsigned int mnt_flags)
 	if (!can_change_locked_flags(mnt, mnt_flags))
 		return -EPERM;
 
+	ret = security_mount_reconfigure(path, mnt_flags, flags);
+	if (ret)
+		return ret;
+
 	/*
 	 * We're only checking whether the superblock is read-only not
 	 * changing it, so only take down_read(&sb->s_umount).
@@ -3366,7 +3379,7 @@ static int do_reconfigure_mnt(const struct path *path, unsigned int mnt_flags)
  * on it - tough luck.
  */
 static int do_remount(const struct path *path, int sb_flags,
-		      int mnt_flags, void *data)
+		      int mnt_flags, void *data, unsigned long flags)
 {
 	int err;
 	struct super_block *sb = path->mnt->mnt_sb;
@@ -3393,6 +3406,9 @@ static int do_remount(const struct path *path, int sb_flags,
 	fc->oldapi = true;
 
 	err = parse_monolithic_mount_data(fc, data);
+	if (!err)
+		err = security_mount_remount(fc, path, mnt_flags, flags,
+					    data);
 	if (!err) {
 		down_write(&sb->s_umount);
 		err = -EPERM;
@@ -3708,6 +3724,10 @@ static int do_move_mount_old(const struct path *path, const char *old_name)
 	if (err)
 		return err;
 
+	err = security_mount_move(&old_path, path);
+	if (err)
+		return err;
+
 	return do_move_mount(&old_path, path, 0);
 }
 
@@ -3786,7 +3806,7 @@ static int do_new_mount_fc(struct fs_context *fc, const struct path *mountpoint,
  */
 static int do_new_mount(const struct path *path, const char *fstype,
 			int sb_flags, int mnt_flags,
-			const char *name, void *data)
+			const char *name, void *data, unsigned long flags)
 {
 	struct file_system_type *type;
 	struct fs_context *fc;
@@ -3830,6 +3850,9 @@ static int do_new_mount(const struct path *path, const char *fstype,
 		err = parse_monolithic_mount_data(fc, data);
 	if (!err && !mount_capable(fc))
 		err = -EPERM;
+
+	if (!err)
+		err = security_mount_new(fc, path, mnt_flags, flags, data);
 	if (!err)
 		err = do_new_mount_fc(fc, path, mnt_flags);
 
@@ -4080,7 +4103,6 @@ int path_mount(const char *dev_name, const struct path *path,
 		const char *type_page, unsigned long flags, void *data_page)
 {
 	unsigned int mnt_flags = 0, sb_flags;
-	int ret;
 
 	/* Discard magic */
 	if ((flags & MS_MGC_MSK) == MS_MGC_VAL)
@@ -4093,9 +4115,6 @@ int path_mount(const char *dev_name, const struct path *path,
 	if (flags & MS_NOUSER)
 		return -EINVAL;
 
-	ret = security_sb_mount(dev_name, path, type_page, flags, data_page);
-	if (ret)
-		return ret;
 	if (!may_mount())
 		return -EPERM;
 	if (flags & SB_MANDLOCK)
@@ -4141,9 +4160,9 @@ int path_mount(const char *dev_name, const struct path *path,
 			    SB_I_VERSION);
 
 	if ((flags & (MS_REMOUNT | MS_BIND)) == (MS_REMOUNT | MS_BIND))
-		return do_reconfigure_mnt(path, mnt_flags);
+		return do_reconfigure_mnt(path, mnt_flags, flags);
 	if (flags & MS_REMOUNT)
-		return do_remount(path, sb_flags, mnt_flags, data_page);
+		return do_remount(path, sb_flags, mnt_flags, data_page, flags);
 	if (flags & MS_BIND)
 		return do_loopback(path, dev_name, flags & MS_REC);
 	if (flags & (MS_SHARED | MS_PRIVATE | MS_SLAVE | MS_UNBINDABLE))
@@ -4152,7 +4171,7 @@ int path_mount(const char *dev_name, const struct path *path,
 		return do_move_mount_old(path, dev_name);
 
 	return do_new_mount(path, type_page, sb_flags, mnt_flags, dev_name,
-			    data_page);
+			    data_page, flags);
 }
 
 int do_mount(const char *dev_name, const char __user *dir_name,
@@ -4545,7 +4564,7 @@ static inline int vfs_move_mount(const struct path *from_path,
 {
 	int ret;
 
-	ret = security_move_mount(from_path, to_path);
+	ret = security_mount_move(from_path, to_path);
 	if (ret)
 		return ret;
 
-- 
2.53.0-Meta

Re: [PATCH v5 7/8] vfs: Replace security_sb_mount/security_move_mount with granular hooks

[Song Liu]

Hi Christian,

On Thu, May 28, 2026 at 11:26 AM Song Liu <song@kernel.org> wrote:
>
> Replace the monolithic security_sb_mount() call in path_mount() and
> security_move_mount() in vfs_move_mount() with the new granular mount
> hooks:
>
> - do_loopback(): call security_mount_bind()
> - do_new_mount(): call security_mount_new()
> - do_remount(): call security_mount_remount()
> - do_reconfigure_mnt(): call security_mount_reconfigure()
> - do_move_mount_old(): call security_mount_move()
> - do_change_type(): call security_mount_change_type()
> - vfs_move_mount(): replace security_move_mount() with
>   security_mount_move()

Does this version look good to you?

Thanks,
Song

Re: [PATCH v5 7/8] vfs: Replace security_sb_mount/security_move_mount with granular hooks

[Christian Brauner]

On Thu, May 28, 2026 at 11:26:06AM -0700, Song Liu wrote:
> Replace the monolithic security_sb_mount() call in path_mount() and
> security_move_mount() in vfs_move_mount() with the new granular mount
> hooks:
> 
> - do_loopback(): call security_mount_bind()
> - do_new_mount(): call security_mount_new()
> - do_remount(): call security_mount_remount()
> - do_reconfigure_mnt(): call security_mount_reconfigure()
> - do_move_mount_old(): call security_mount_move()
> - do_change_type(): call security_mount_change_type()
> - vfs_move_mount(): replace security_move_mount() with
>   security_mount_move()
> 
> The new hooks are called at the individual operation level with
> appropriate context (resolved paths, fs_context), rather than at
> the top of path_mount() with raw string arguments.
> 
> Code generated with the assistance of Claude, reviewed by human.
> 
> Reviewed-by: Stephen Smalley <stephen.smalley.work@gmail.com>
> Tested-by: Stephen Smalley <stephen.smalley.work@gmail.com> # for selinux only
> Signed-off-by: Song Liu <song@kernel.org>
> ---
>  fs/namespace.c | 41 ++++++++++++++++++++++++++++++-----------
>  1 file changed, 30 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/namespace.c b/fs/namespace.c
> index fe919abd2f01..43f22c5e2bf4 100644
> --- a/fs/namespace.c
> +++ b/fs/namespace.c
> @@ -2888,6 +2888,10 @@ static int do_change_type(const struct path *path, int ms_flags)
>  	if (!type)
>  		return -EINVAL;
>  
> +	err = security_mount_change_type(path, ms_flags);
> +	if (err)
> +		return err;
> +
>  	guard(namespace_excl)();
>  
>  	err = may_change_propagation(mnt);
> @@ -3006,6 +3010,10 @@ static int do_loopback(const struct path *path, const char *old_name,
>  	if (err)
>  		return err;
>  
> +	err = security_mount_bind(&old_path, path, recurse);
> +	if (err)
> +		return err;

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.

> +
>  	if (mnt_ns_loop(old_path.dentry))
>  		return -EINVAL;
>  
> @@ -3328,7 +3336,8 @@ static void mnt_warn_timestamp_expiry(const struct path *mountpoint,
>   * superblock it refers to.  This is triggered by specifying MS_REMOUNT|MS_BIND
>   * to mount(2).
>   */
> -static int do_reconfigure_mnt(const struct path *path, unsigned int mnt_flags)
> +static int do_reconfigure_mnt(const struct path *path, unsigned int mnt_flags,
> +			      unsigned long flags)
>  {
>  	struct super_block *sb = path->mnt->mnt_sb;
>  	struct mount *mnt = real_mount(path->mnt);
> @@ -3343,6 +3352,10 @@ static int do_reconfigure_mnt(const struct path *path, unsigned int mnt_flags)
>  	if (!can_change_locked_flags(mnt, mnt_flags))
>  		return -EPERM;
>  
> +	ret = security_mount_reconfigure(path, mnt_flags, flags);
> +	if (ret)
> +		return ret;
> +
>  	/*
>  	 * We're only checking whether the superblock is read-only not
>  	 * changing it, so only take down_read(&sb->s_umount).
> @@ -3366,7 +3379,7 @@ static int do_reconfigure_mnt(const struct path *path, unsigned int mnt_flags)
>   * on it - tough luck.
>   */
>  static int do_remount(const struct path *path, int sb_flags,
> -		      int mnt_flags, void *data)
> +		      int mnt_flags, void *data, unsigned long flags)
>  {
>  	int err;
>  	struct super_block *sb = path->mnt->mnt_sb;
> @@ -3393,6 +3406,9 @@ static int do_remount(const struct path *path, int sb_flags,
>  	fc->oldapi = true;
>  
>  	err = parse_monolithic_mount_data(fc, data);
> +	if (!err)
> +		err = security_mount_remount(fc, path, mnt_flags, flags,
> +					    data);
>  	if (!err) {
>  		down_write(&sb->s_umount);
>  		err = -EPERM;
> @@ -3708,6 +3724,10 @@ static int do_move_mount_old(const struct path *path, const char *old_name)
>  	if (err)
>  		return err;
>  
> +	err = security_mount_move(&old_path, path);
> +	if (err)
> +		return err;

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.

Re: [PATCH v5 7/8] vfs: Replace security_sb_mount/security_move_mount with granular hooks

[Song Liu]

On Wed, Jun 17, 2026 at 9:53 PM Christian Brauner <brauner@kernel.org> wrote:
>
> On Thu, May 28, 2026 at 11:26:06AM -0700, Song Liu wrote:
[...]
> >
> > +     err = security_mount_bind(&old_path, path, recurse);
> > +     if (err)
> > +             return err;
>
> 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?

> > +
> >       if (mnt_ns_loop(old_path.dentry))
> >               return -EINVAL;
> >
[...]
> >
> >       err = parse_monolithic_mount_data(fc, data);
> > +     if (!err)
> > +             err = security_mount_remount(fc, path, mnt_flags, flags,
> > +                                         data);
> >       if (!err) {
> >               down_write(&sb->s_umount);
> >               err = -EPERM;
> > @@ -3708,6 +3724,10 @@ static int do_move_mount_old(const struct path *path, const char *old_name)
> >       if (err)
> >               return err;
> >
> > +     err = security_mount_move(&old_path, path);
> > +     if (err)
> > +             return err;
>
> 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?

Thanks,
Song

Re: [PATCH v5 7/8] vfs: Replace security_sb_mount/security_move_mount with granular hooks

[Christian Brauner]

On 2026-06-18 18:56:42+08:00, Song Liu wrote:
> On Wed, Jun 17, 2026 at 9:53 PM Christian Brauner <brauner@kernel.org> 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.

Re: [PATCH v5 7/8] vfs: Replace security_sb_mount/security_move_mount with granular hooks

[Bryam Vargas]

Song,

> +	err = security_mount_change_type(path, ms_flags);

This gates the propagation change on the mount(2) path. The same change on
the newer mount_setattr(2)/open_tree_attr(2) path is left open:
do_mount_setattr() -> mount_setattr_commit() calls change_mnt_propagation()
for the propagation and writes the MNT_NOEXEC/NOSUID/NODEV/READONLY flags --
the same work do_change_type() and do_reconfigure_mnt() do, but with no
hook. security_sb_mount() never reached that path either, so the gap isn't
new. But once this series checks the mount(2) propagation and remount
paths, mount_setattr(2) is the one path left without a check.

It's reachable. A Landlock domain denies mount(2) for the confined task, so
mount(MS_PRIVATE) and a remount clearing noexec both return -EPERM -- but
mount_setattr(propagation=MS_PRIVATE) and
mount_setattr(attr_clr=MOUNT_ATTR_NOEXEC) succeed, and the task then runs a
binary on a mount the policy marked noexec. A SELinux/AppArmor policy that
denies the mount has the same gap. With this series applied,
do_mount_setattr() still carries no security_ call, so the divergence
stands.

Adding the propagation hook and a reconfigure hook in
mount_setattr_commit() would cover mount_setattr too. Happy to send that as
a patch if you want it folded in.

Bryam