All of lore.kernel.org
 help / color / mirror / Atom feed
From: ebiederm@xmission.com (Eric W. Biederman)
To: Cyrill Gorcunov <gorcunov@openvz.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	richard -rw- weinberger <richard.weinberger@gmail.com>,
	LKML <linux-kernel@vger.kernel.org>,
	Oleg Nesterov <oleg@redhat.com>,
	KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>,
	Pavel Emelyanov <xemul@parallels.com>,
	Kees Cook <keescook@chromium.org>, Tejun Heo <tj@kernel.org>,
	Matt Helsley <matthltc@us.ibm.com>
Subject: Re: [patch 1/2] c/r: prctl: Add ability to set new mm_struct::exe_file
Date: Thu, 22 Mar 2012 16:38:43 -0700	[thread overview]
Message-ID: <m14ntgxl30.fsf@fess.ebiederm.org> (raw)
In-Reply-To: <20120320065543.GB14269@moon> (Cyrill Gorcunov's message of "Tue, 20 Mar 2012 10:55:43 +0400")

Cyrill Gorcunov <gorcunov@openvz.org> writes:

> On Mon, Mar 19, 2012 at 03:46:49PM -0700, Andrew Morton wrote:
>> > >>
>> > >> What is this mysterious "security reason"?
>> > >>
>> > >
>> > > Oh, sorry I should have included Matt's comment here
>> 
>> Please send a patch with the updated changelog and improved comment?
>> 
>
> Andrew, take a look please, will the changelog and comments look
> better?

Can you change this to take an actual address and get the exe_file
from an mmapped area and make certain that the mmaped_area is already
mapped MAP_EXEC.

That will prevent out-right lies.

At least then we will know that exe_file will at least be a file that is
mapped executable in the process's address space.  It's not a lot better
but it makes /proc/<pid>/exe at almost as trustable as it is now.

> 	Cyrill
> ---
> From: Cyrill Gorcunov <gorcunov@openvz.org>
> Subject: c/r: prctl: add ability to set new mm_struct::exe_file
>
> When we do restore we would like to have a way to setup
> a former mm_struct::exe_file so that /proc/pid/exe would
> point to the original executable file a process had at
> checkpoint time.
>
> For this the PR_SET_MM_EXE_FILE code is introduced.
> This option takes a file descriptor which will be
> set as a source for new /proc/$pid/exe symlink.
>
> Note it allows to change /proc/$pid/exe iif there
> are no VM_EXECUTABLE vmas present for current process,
> simply because this feature is a special to C/R
> and mm::num_exe_file_vmas become meaningless after
> that.
>
> To minimize the amount of transition the /proc/pid/exe
> symlink might have, this feature is implemented in one-shot
> manner. Thus once changed the symlink can't be changed
> again. This should help sysadmins to monitor the symlinks
> over all process running in a system.
>
> In particular one could make a snapshot of processes and
> ring alarm if there unexpected changes of /proc/pid/exe's
> in a system.
>
> Note -- this feature is available iif CONFIG_CHECKPOINT_RESTORE
> is set and the caller must have CAP_SYS_RESOURCE capability
> granted, otherwise the request to change symlink will be
> rejected.
>
> Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
> Reviewed-by: Oleg Nesterov <oleg@redhat.com>
> CC: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
> CC: Pavel Emelyanov <xemul@parallels.com>
> CC: Kees Cook <keescook@chromium.org>
> CC: Tejun Heo <tj@kernel.org>
> CC: Matt Helsley <matthltc@us.ibm.com>
> CC: richard -rw- weinberger <richard.weinberger@gmail.com>
> ---
>  include/linux/prctl.h |    1 
>  kernel/sys.c          |   56 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 57 insertions(+)
>
> Index: linux-2.6.git/include/linux/prctl.h
> ===================================================================
> --- linux-2.6.git.orig/include/linux/prctl.h
> +++ linux-2.6.git/include/linux/prctl.h
> @@ -118,5 +118,6 @@
>  # define PR_SET_MM_ENV_START		10
>  # define PR_SET_MM_ENV_END		11
>  # define PR_SET_MM_AUXV			12
> +# define PR_SET_MM_EXE_FILE		13
>  
>  #endif /* _LINUX_PRCTL_H */
> Index: linux-2.6.git/kernel/sys.c
> ===================================================================
> --- linux-2.6.git.orig/kernel/sys.c
> +++ linux-2.6.git/kernel/sys.c
> @@ -36,6 +36,8 @@
>  #include <linux/personality.h>
>  #include <linux/ptrace.h>
>  #include <linux/fs_struct.h>
> +#include <linux/file.h>
> +#include <linux/mount.h>
>  #include <linux/gfp.h>
>  #include <linux/syscore_ops.h>
>  #include <linux/version.h>
> @@ -1701,6 +1703,57 @@ static bool vma_flags_mismatch(struct vm
>  		(vma->vm_flags & banned);
>  }
>  
> +static int prctl_set_mm_exe_file(struct mm_struct *mm, unsigned int fd)
> +{
> +	struct file *exe_file;
> +	struct dentry *dentry;
> +	int err;
> +
> +	/*
> +	 * Setting new mm::exe_file is only allowed when no VM_EXECUTABLE vma's
> +	 * remain. So perform a quick test first.
> +	 */
> +	if (mm->num_exe_file_vmas)
> +		return -EBUSY;
> +
> +	exe_file = fget(fd);
> +	if (!exe_file)
> +		return -EBADF;
> +
> +	dentry = exe_file->f_path.dentry;
> +
> +	/*
> +	 * Because the original mm->exe_file points to executable file, make
> +	 * sure that this one is executable as well, to avoid breaking an
> +	 * overall picture.
> +	 */
> +	err = -EACCES;
> +	if (!S_ISREG(dentry->d_inode->i_mode)	||
> +	    exe_file->f_path.mnt->mnt_flags & MNT_NOEXEC)
> +		goto exit;
> +
> +	err = inode_permission(dentry->d_inode, MAY_EXEC);
> +	if (err)
> +		goto exit;
> +
> +	/*
> +	 * The symlink can be changed only once, just to disallow arbitrary
> +	 * transitions malicious software might bring in. This means one
> +	 * could make a snapshot over all processes running and monitor
> +	 * /proc/pid/exe changes to notice unusual activity if needed.
> +	 */
> +	down_write(&mm->mmap_sem);
> +	if (likely(!mm->exe_file))
> +		set_mm_exe_file(mm, exe_file);
> +	else
> +		err = -EBUSY;
> +	up_write(&mm->mmap_sem);
> +
> +exit:
> +	fput(exe_file);
> +	return err;
> +}
> +
>  static int prctl_set_mm(int opt, unsigned long addr,
>  			unsigned long arg4, unsigned long arg5)
>  {
> @@ -1715,6 +1768,9 @@ static int prctl_set_mm(int opt, unsigne
>  	if (!capable(CAP_SYS_RESOURCE))
>  		return -EPERM;
>  
> +	if (opt == PR_SET_MM_EXE_FILE)
> +		return prctl_set_mm_exe_file(mm, (unsigned int)addr);
> +
>  	if (addr >= TASK_SIZE)
>  		return -EINVAL;
>  
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

  reply	other threads:[~2012-03-22 23:35 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-03-16 20:55 [patch 0/2] [PATCH 0/2] prctl extension in a sake of c/r Cyrill Gorcunov
2012-03-16 20:55 ` [patch 1/2] c/r: prctl: Add ability to set new mm_struct::exe_file Cyrill Gorcunov
2012-03-19 22:15   ` Andrew Morton
2012-03-19 22:39     ` Cyrill Gorcunov
2012-03-19 22:41       ` richard -rw- weinberger
2012-03-19 22:46         ` Andrew Morton
2012-03-19 22:50           ` Cyrill Gorcunov
2012-03-19 22:59             ` Andrew Morton
2012-03-19 23:12               ` Cyrill Gorcunov
2012-03-19 23:02           ` richard -rw- weinberger
2012-03-19 23:17             ` Cyrill Gorcunov
2012-03-19 23:23               ` richard -rw- weinberger
2012-03-20  6:55           ` Cyrill Gorcunov
2012-03-22 23:38             ` Eric W. Biederman [this message]
2012-03-23  6:41               ` Cyrill Gorcunov
2012-03-23  6:47                 ` Cyrill Gorcunov
2012-03-23 17:06               ` Matt Helsley
2012-03-19 22:47         ` Cyrill Gorcunov
2012-03-16 20:55 ` [patch 2/2] c/r: prctl: Add ability to get clear_tid_address Cyrill Gorcunov
2012-03-19 16:51   ` Kees Cook
2012-03-19 16:55     ` Cyrill Gorcunov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=m14ntgxl30.fsf@fess.ebiederm.org \
    --to=ebiederm@xmission.com \
    --cc=akpm@linux-foundation.org \
    --cc=gorcunov@openvz.org \
    --cc=keescook@chromium.org \
    --cc=kosaki.motohiro@jp.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=matthltc@us.ibm.com \
    --cc=oleg@redhat.com \
    --cc=richard.weinberger@gmail.com \
    --cc=tj@kernel.org \
    --cc=xemul@parallels.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.