Re: [PATCH 05/34] fs: introduce MOUNT_ATTR_IDMAP
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?