Hi all,
On Wed, 20 Jun 2018 12:39:05 +1000 Stephen Rothwell <sfr@xxxxxxxxxxxxxxxx> wrote:
>
> Today's linux-next merge of the userns tree got a conflict in:
>
> fs/proc/inode.c
> fs/proc/root.c
>
> between commit:
>
> 0223e0999be2 ("procfs: Move proc_fill_super() to fs/proc/root.c")
> 83cd45075c36 ("proc: Add fs_context support to procfs")
>
> from the vfs tree and commit:
>
> cc8cda3af2ba ("proc: Simplify and fix proc by removing the kernel mount")
> 9303f5f81bd4 ("proc: Change proc_parse_options to return an errno value")
> 04035aa33a12 ("proc: Don't change mount options on remount failure.")
>
> from the userns tree.
>
> I effectively reverted 9303f5f81bd4 and 04035aa33a12 since (I think)
> they are subsumed by the vfs tree changes.
>
> I fixed it up (I think I got it mostly right this time - see below) and
> can carry the fix as necessary. This is now fixed as far as linux-next
> is concerned, but any non trivial conflicts should be mentioned to your
> upstream maintainer when your tree is submitted for merging. You may
> also want to consider cooperating with the maintainer of the
> conflicting tree to minimise any particularly complex conflicts.
>
> --
> Cheers,
> Stephen Rothwell
>
> diff --cc fs/proc/root.c
> index efbdc08a3c86,3dceff6cd121..91e9b417bc6d
> --- a/fs/proc/root.c
> +++ b/fs/proc/root.c
> @@@ -80,71 -78,21 +80,74 @@@ static int proc_parse_option(struct fs_
> return 0;
> }
>
> -int proc_remount(struct super_block *sb, int *flags, char *data)
> +static void proc_set_options(struct super_block *s,
> + struct fs_context *fc,
> + struct pid_namespace *pid_ns,
> + struct user_namespace *user_ns)
> {
> - struct pid_namespace *pid = sb->s_fs_info;
> - struct proc_mount_options opts = {
> - .pid_gid = pid->pid_gid,
> - .hide_pid = pid->hide_pid,
> - };
> + struct proc_fs_context *ctx = fc->fs_private;
> +
> + if (ctx->mask & (1 << Opt_gid))
> + pid_ns->pid_gid = make_kgid(user_ns, ctx->gid);
> + if (ctx->mask & (1 << Opt_hidepid))
> + pid_ns->hide_pid = ctx->hidepid;
> +}
> +
> +static int proc_fill_super(struct super_block *s, struct fs_context *fc)
> +{
> + struct pid_namespace *pid_ns = get_pid_ns(s->s_fs_info);
> + struct inode *root_inode;
> int ret;
>
> - sync_filesystem(sb);
> - ret = proc_parse_options(data, &opts);
> - if (ret)
> + proc_set_options(s, fc, pid_ns, current_user_ns());
> +
> + /* User space would break if executables or devices appear on proc */
> + s->s_iflags |= SB_I_USERNS_VISIBLE | SB_I_NOEXEC | SB_I_NODEV;
> + s->s_flags |= SB_NODIRATIME | SB_NOSUID | SB_NOEXEC;
> + s->s_blocksize = 1024;
> + s->s_blocksize_bits = 10;
> + s->s_magic = PROC_SUPER_MAGIC;
> + s->s_op = &proc_sops;
> + s->s_time_gran = 1;
> +
> + /*
> + * procfs isn't actually a stacking filesystem; however, there is
> + * too much magic going on inside it to permit stacking things on
> + * top of it
> + */
> + s->s_stack_depth = FILESYSTEM_MAX_STACK_DEPTH;
> +
> + pde_get(&proc_root);
> + root_inode = proc_get_inode(s, &proc_root);
> + if (!root_inode) {
> + pr_err("proc_fill_super: get root inode failed\n");
> + return -ENOMEM;
> + }
> +
> + s->s_root = d_make_root(root_inode);
> + if (!s->s_root) {
> + pr_err("proc_fill_super: allocate dentry failed\n");
> + return -ENOMEM;
> + }
> +
> + ret = proc_setup_self(s);
> + if (ret) {
> return ret;
> - pid->pid_gid = opts.pid_gid;
> - pid->hide_pid = opts.hide_pid;
> + }
> - return proc_setup_thread_self(s);
> ++ ret = proc_setup_thread_self(s);
> ++
> ++ rcu_assign_pointer(pid_ns->proc_super, s);
> ++ return ret;
> +}
> +
> +int proc_reconfigure(struct super_block *sb, struct fs_context *fc)
> +{
> + struct pid_namespace *pid = sb->s_fs_info;
> +
> + sync_filesystem(sb);
> +
> + if (fc)
> + proc_set_options(sb, fc, pid, current_user_ns());
> return 0;
> }
>
> @@@ -288,44 -208,23 +292,22 @@@ struct proc_dir_entry proc_root =
> .proc_fops = &proc_root_operations,
> .parent = &proc_root,
> .subdir = RB_ROOT,
> - .name = proc_root.inline_name,
> - .inline_name = "/proc",
> + .name = "/proc",
> };
>
> - int pid_ns_prepare_proc(struct pid_namespace *ns)
> + #if defined(CONFIG_SYSCTL_SYSCALL) || defined(CONFIG_MCONSOLE)
> + struct file *file_open_proc(const char *pathname, int flags, umode_t mode)
> {
> - struct proc_fs_context *ctx;
> - struct fs_context *fc;
> struct vfsmount *mnt;
> - int ret;
> -
> - fc = vfs_new_fs_context(&proc_fs_type, NULL, 0,
> - FS_CONTEXT_FOR_KERNEL_MOUNT);
> - if (IS_ERR(fc))
> - return PTR_ERR(fc);
> -
> - ctx = fc->fs_private;
> - if (ctx->pid_ns != ns) {
> - put_pid_ns(ctx->pid_ns);
> - get_pid_ns(ns);
> - ctx->pid_ns = ns;
> - }
> + struct file *file;
>
> - ret = vfs_get_tree(fc);
> - if (ret < 0) {
> - put_fs_context(fc);
> - return ret;
> - }
> -
> - mnt = vfs_create_mount(fc, 0);
> - put_fs_context(fc);
> + mnt = kern_mount(&proc_fs_type);
> if (IS_ERR(mnt))
> - return PTR_ERR(mnt);
> + return ERR_CAST(mnt);
>
> - ns->proc_mnt = mnt;
> - return 0;
> - }
> + file = file_open_root(mnt->mnt_root, mnt, pathname, flags, mode);
> + kern_unmount(mnt);
>
> - void pid_ns_release_proc(struct pid_namespace *ns)
> - {
> - kern_unmount(ns->proc_mnt);
> + return file;
> }
> + #endif
Are there any comments on this resolution. I just had to do it all
again due to slight changes in the vfs tree. What are you guys going
to tell Linus when he comes to merge this?
--
Cheers,
Stephen Rothwell
Attachment:
pgpTxCQTWzbWL.pgp
Description: OpenPGP digital signature
