Re: [PATCH 2/2] autofs: dont trigger mount if it cant succeed

[Ian Kent]

On 11/11/25 18:55, Christian Brauner wrote:
> On Tue, Nov 11, 2025 at 10:24:35AM +0000, Al Viro wrote:
> > On Tue, Nov 11, 2025 at 11:19:59AM +0100, Christian Brauner wrote:
> >
> > > > +	sbi->owner = current->nsproxy->mnt_ns;
> > > ns_ref_get()
> > > Can be called directly on the mount namespace.
> > ... and would leak all mounts in the mount tree, unless I'm missing
> > something subtle.
> Right, I thought you actually wanted to pin it.
> Anyway, you could take a passive reference but I think that's nonsense
> as well. The following should do it:

Right, I'll need to think about this for a little while, I did think

of using an id for the comparison but I diverged down the wrong path so

this is a very welcome suggestion. There's still the handling of where

the daemon goes away (crash or SIGKILL, yes people deliberately do this

at times, think simulated disaster recovery) which I've missed in this

revision.


Al, thoughts please?


> UNTESTED, UNCOMPILED

I'll give it a whirl, ;)


> ---
>   fs/autofs/autofs_i.h |  4 ++++
>   fs/autofs/inode.c    |  3 +++
>   fs/autofs/root.c     | 10 ++++++++++
>   fs/namespace.c       |  6 ++++++
>   include/linux/fs.h   |  1 +
>   5 files changed, 24 insertions(+)
>
> diff --git a/fs/autofs/autofs_i.h b/fs/autofs/autofs_i.h
> index 23cea74f9933..2b9d2300d351 100644
> --- a/fs/autofs/autofs_i.h
> +++ b/fs/autofs/autofs_i.h
> @@ -16,6 +16,7 @@
>   #include <linux/wait.h>
>   #include <linux/sched.h>
>   #include <linux/sched/signal.h>
> +#include <uapi/linux/mount.h>
>   #include <linux/mount.h>
>   #include <linux/namei.h>
>   #include <linux/uaccess.h>
> @@ -109,11 +110,14 @@ struct autofs_wait_queue {
>   #define AUTOFS_SBI_STRICTEXPIRE 0x0002
>   #define AUTOFS_SBI_IGNORE	0x0004
>   
>   struct autofs_sb_info {
>   	u32 magic;
>   	int pipefd;
>   	struct file *pipe;
>   	struct pid *oz_pgrp;
> +	u64 mnt_ns_id;
>   	int version;
>   	int sub_version;
>   	int min_proto;
> diff --git a/fs/autofs/inode.c b/fs/autofs/inode.c
> index f5c16ffba013..247a5784d192 100644
> --- a/fs/autofs/inode.c
> +++ b/fs/autofs/inode.c
> @@ -6,8 +6,10 @@
>   
>   #include <linux/seq_file.h>
>   #include <linux/pagemap.h>
> +#include <linux/ns_common.h>
>   
>   #include "autofs_i.h"
> +#include "../mount.h"

As a module I try really hard to avoid use of non-public definitions.

But if everyone is happy I'm happy too!


>   
>   struct autofs_info *autofs_new_ino(struct autofs_sb_info *sbi)
>   {
> @@ -251,6 +253,7 @@ static struct autofs_sb_info *autofs_alloc_sbi(void)
>   	sbi->min_proto = AUTOFS_MIN_PROTO_VERSION;
>   	sbi->max_proto = AUTOFS_MAX_PROTO_VERSION;
>   	sbi->pipefd = -1;
> +	sbi->mnt_ns_id = to_ns_common(current->nsproxy->mnt_ns)->ns_id;
>   
>   	set_autofs_type_indirect(&sbi->type);
>   	mutex_init(&sbi->wq_mutex);
> diff --git a/fs/autofs/root.c b/fs/autofs/root.c
> index 174c7205fee4..f06f62d23e76 100644
> --- a/fs/autofs/root.c
> +++ b/fs/autofs/root.c
> @@ -7,8 +7,10 @@
>   
>   #include <linux/capability.h>
>   #include <linux/compat.h>
> +#include <linux/ns_common.h>
>   
>   #include "autofs_i.h"
> +#include "../mount.h"
>   
>   static int autofs_dir_permission(struct mnt_idmap *, struct inode *, int);
>   static int autofs_dir_symlink(struct mnt_idmap *, struct inode *,
> @@ -341,6 +343,14 @@ static struct vfsmount *autofs_d_automount(struct path *path)
>   	if (autofs_oz_mode(sbi))
>   		return NULL;
>   
> +	/* Refuse to trigger mount if current namespace is not the owner
> +	 * and the mount is propagation private.
> +	 */
> +	if (sbi->mnt_ns_id != to_ns_common(current->nsproxy->mnt_ns)->ns_id) {
> +		if (vfsmount_to_propagation_flags(path->mnt) & MS_PRIVATE)
> +			return ERR_PTR(-EPERM);
> +	}
> +
>   	/*
>   	 * If an expire request is pending everyone must wait.
>   	 * If the expire fails we're still mounted so continue
> diff --git a/fs/namespace.c b/fs/namespace.c
> index d82910f33dc4..27bb12693cba 100644
> --- a/fs/namespace.c
> +++ b/fs/namespace.c
> @@ -5150,6 +5150,12 @@ static u64 mnt_to_propagation_flags(struct mount *m)
>   	return propagation;
>   }
>   
> +u64 vfsmount_to_propagation_flags(struct vfsmount *mnt)
> +{
> +	return mnt_to_propagation_flags(real_mount(mnt));
> +}
> +EXPORT_SYMBOL_GPL(vfsmount_to_propagation_flags);
> +
>   static void statmount_sb_basic(struct kstatmount *s)
>   {
>   	struct super_block *sb = s->mnt->mnt_sb;
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index c895146c1444..a5c2077ce6ed 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -3269,6 +3269,7 @@ extern struct file * open_exec(const char *);
>   /* fs/dcache.c -- generic fs support functions */
>   extern bool is_subdir(struct dentry *, struct dentry *);
>   extern bool path_is_under(const struct path *, const struct path *);
> +u64 vfsmount_to_propagation_flags(struct vfsmount *mnt);
>   
>   extern char *file_path(struct file *, char *, int);
>