On Wed, May 03 2017, David Howells <dhowells@xxxxxxxxxx> wrote:
> fs_type->fsopen() is called to set it up. fs_type->mc_size says how much
> should be added on to the mount context for the filesystem's use.
This is repeated several times in the documentation, but the code says
that ->mc_size should be the full size of the struct wrapping struct
mount_context.
> diff --git a/fs/mount.h b/fs/mount.h
> index 2826543a131d..b1e99b38f2ee 100644
> --- a/fs/mount.h
> +++ b/fs/mount.h
> @@ -108,9 +108,10 @@ static inline void detach_mounts(struct dentry *dentry)
> __detach_mounts(dentry);
> }
>
> -static inline void get_mnt_ns(struct mnt_namespace *ns)
> +static inline struct mnt_namespace *get_mnt_ns(struct mnt_namespace *ns)
> {
> atomic_inc(&ns->count);
> + return ns;
> }
>
> extern seqlock_t mount_lock;
it's not much, but at least this could go into a patch of its own.
> +/**
> + * __vfs_fsopen - Open a filesystem and create a mount context
> + * @fs_type: The filesystem type
> + * @src_sb: A superblock from which this one derives (or NULL)
> + * @ms_flags: Superblock flags and op flags (such as MS_REMOUNT)
> + * @mnt_flags: Mountpoint flags, such as MNT_READONLY
> + * @mount_type: Type of mount
> + *
> + * Open a filesystem and create a mount context. The mount context is
> + * initialised with the supplied flags and, if a submount/automount from
> + * another superblock (@src_sb), may have parameters such as namespaces copied
> + * across from that superblock.
> + */
> +struct mount_context *__vfs_fsopen(struct file_system_type *fs_type,
> + struct super_block *src_sb,
> + unsigned int ms_flags, unsigned int mnt_flags,
> + enum mount_type mount_type)
> +{
> + struct mount_context *mc;
> + int ret;
> +
> + if (fs_type->fsopen && fs_type->mc_size < sizeof(*mc))
> + BUG();
So ->mc_size can be 0 (i.e. not explicitly initialized) if fs_type does
not have ->fsopen. OK.
> + mc = kzalloc(max_t(size_t, fs_type->mc_size, sizeof(*mc)), GFP_KERNEL);
In which case we round up to sizeof(*mc). OK.
> + if (!mc)
> + return ERR_PTR(-ENOMEM);
> +
> + mc->mount_type = mount_type;
> + mc->ms_flags = ms_flags;
> + mc->mnt_flags = mnt_flags;
> + mc->fs_type = fs_type;
> + get_filesystem(fs_type);
Maybe get_filesystem should also be taught to return its argument so
this could be written like the below assignments.
> + mc->mnt_ns = get_mnt_ns(current->nsproxy->mnt_ns);
> + mc->pid_ns = get_pid_ns(task_active_pid_ns(current));
> + mc->net_ns = get_net(current->nsproxy->net_ns);
> + mc->user_ns = get_user_ns(current_user_ns());
> + mc->cred = get_current_cred();
> +
> +
> +/**
> + * vfs_dup_mount_context: Duplicate a mount context.
> + * @src: The mount context to copy.
> + */
> +struct mount_context *vfs_dup_mount_context(struct mount_context *src)
> +{
> + struct mount_context *mc;
> + int ret;
> +
> + if (!src->ops->dup)
> + return ERR_PTR(-ENOTSUPP);
> +
> + mc = kmemdup(src, src->fs_type->mc_size, GFP_KERNEL);
So this assumes that vfs_dup_mount_context is only used if ->mc_size is
explicitly initialized. A max_t here as well probably wouldn't hurt.
> + unsigned short mc_size; /* Size of mount context to allocate */
Any particular reason to use a short? The struct doesn't pack any better.
> +static int selinux_mount_ctx_dup(struct mount_context *mc,
> + struct mount_context *src_mc)
> +{
> + const struct security_mnt_opts *src = src_mc->security;
> + struct security_mnt_opts *opts;
> + int i, n;
> +
> + opts = kzalloc(sizeof(*opts), GFP_KERNEL);
> + if (!opts)
> + return -ENOMEM;
> + mc->security = opts;
> +
> + if (!src || !src->num_mnt_opts)
> + return 0;
> + n = opts->num_mnt_opts = src->num_mnt_opts;
> +
> + if (opts->mnt_opts) {
should probably be src->mnt_opts
