[PATCH 05/34] fs: introduce MOUNT_ATTR_IDMAP

Christoph Hellwig hch at infradead.org
Sun Nov 1 14:45:44 UTC 2020


On Thu, Oct 29, 2020 at 01:32:23AM +0100, Christian Brauner wrote:
> Introduce a new mount bind mount property to allow idmapping mounts. The
> MOUNT_ATTR_IDMAP flag can be set via the new mount_setattr() syscall
> together with a file descriptor referring to a user namespace.

Shouldn't this go to the end of the series once all the infrastructure
is in place?

> +config IDMAP_MOUNTS
> +	bool "Support id mappings per mount"
> +	default n

n is the default default.

But why do we need a config option here anyway?

> +#ifdef CONFIG_IDMAP_MOUNTS
> +		if (kattr->attr_set & MNT_IDMAPPED) {
> +			struct user_namespace *user_ns;
> +			struct vfsmount *vmnt;

All the code here looks like it should go into a helper.

> +				struct user_namespace *user_ns = READ_ONCE(m->mnt.mnt_user_ns);
> +				WRITE_ONCE(m->mnt.mnt_user_ns, get_user_ns(kattr->userns));

More unreadable long lines.

> +	if (attr->attr_set & MOUNT_ATTR_IDMAP) {
> +		struct ns_common *ns;
> +		struct user_namespace *user_ns;
> +		struct file *file;
> +
> +		file = fget(attr->userns);

The code here looks like another candidate for a self contained helper.

> +
> +static inline struct user_namespace *mnt_user_ns(const struct vfsmount *mnt)
> +{
> +#ifdef CONFIG_IDMAP_MOUNTS
> +	return READ_ONCE(mnt->mnt_user_ns);
> +#else
> +	return &init_user_ns;
> +#endif

How is the READ_ONCE on a pointer going to work?



More information about the Linux-security-module-archive mailing list