Re: [PATCH 2/3] uml: Create a private mount of proc for mconsole

[Christian Brauner]

On Fri, Feb 28, 2020 at 02:18:43PM -0600, Eric W. Biederman wrote:
> 
> The mconsole code only ever accesses proc for the initial pid
> namespace.  Instead of depending upon the proc_mnt which is
> for proc_flush_task have uml create it's own mount of proc
> instead.
> 
> This allows proc_flush_task to evolve and remove the
> need for having a proc_mnt to do it's job.
> 
> Cc: Jeff Dike <jdike@xxxxxxxxxxx>
> Cc: Richard Weinberger <richard@xxxxxx>
> Cc: Anton Ivanov <anton.ivanov@xxxxxxxxxxxxxxxxxx>
> Signed-off-by: Eric W. Biederman <ebiederm@xxxxxxxxxxxx>
> ---
>  arch/um/drivers/mconsole_kern.c | 28 +++++++++++++++++++++++++++-
>  1 file changed, 27 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/um/drivers/mconsole_kern.c b/arch/um/drivers/mconsole_kern.c
> index e8f5c81c2c6c..30575bd92975 100644
> --- a/arch/um/drivers/mconsole_kern.c
> +++ b/arch/um/drivers/mconsole_kern.c
> @@ -36,6 +36,8 @@
>  #include "mconsole_kern.h"
>  #include <os.h>
>  
> +static struct vfsmount *proc_mnt = NULL;
> +
>  static int do_unlink_socket(struct notifier_block *notifier,
>  			    unsigned long what, void *data)
>  {
> @@ -123,7 +125,7 @@ void mconsole_log(struct mc_request *req)
>  
>  void mconsole_proc(struct mc_request *req)
>  {
> -	struct vfsmount *mnt = init_pid_ns.proc_mnt;
> +	struct vfsmount *mnt = proc_mnt;
>  	char *buf;
>  	int len;
>  	struct file *file;
> @@ -134,6 +136,10 @@ void mconsole_proc(struct mc_request *req)
>  	ptr += strlen("proc");
>  	ptr = skip_spaces(ptr);
>  
> +	if (!mnt) {
> +		mconsole_reply(req, "Proc not available", 1, 0);
> +		goto out;
> +	}
>  	file = file_open_root(mnt->mnt_root, mnt, ptr, O_RDONLY, 0);
>  	if (IS_ERR(file)) {
>  		mconsole_reply(req, "Failed to open file", 1, 0);
> @@ -683,6 +689,24 @@ void mconsole_stack(struct mc_request *req)
>  	with_console(req, stack_proc, to);
>  }
>  
> +static int __init mount_proc(void)
> +{
> +	struct file_system_type *proc_fs_type;
> +	struct vfsmount *mnt;
> +
> +	proc_fs_type = get_fs_type("proc");
> +	if (!proc_fs_type)
> +		return -ENODEV;
> +
> +	mnt = kern_mount(proc_fs_type);
> +	put_filesystem(proc_fs_type);
> +	if (IS_ERR(mnt))
> +		return PTR_ERR(mnt);
> +
> +	proc_mnt = mnt;
> +	return 0;
> +}
> +
>  /*
>   * Changed by mconsole_setup, which is __setup, and called before SMP is
>   * active.
> @@ -696,6 +720,8 @@ static int __init mconsole_init(void)
>  	int err;
>  	char file[UNIX_PATH_MAX];
>  
> +	mount_proc();

Hm, either check the return value or make the mount_proc() void?
Probably worth logging something but moving on without proc.

I guess this is user visible in some scenarios but the patch series
seems worth it!

Christian