All of lore.kernel.org
 help / color / mirror / Atom feed
From: Cyrill Gorcunov <gorcunov@openvz.org>
To: linux-kernel@vger.kernel.org
Cc: akpm@linux-foundation.org, keescook@chromium.org,
	oleg@redhat.com, tj@kernel.org, avagin@openvz.org,
	ebiederm@xmission.com, hpa@zytor.com, serge.hallyn@canonical.com,
	xemul@parallels.com, segoon@openwall.com,
	kamezawa.hiroyu@jp.fujitsu.com, mtk.manpages@gmail.com,
	jln@google.com, Cyrill Gorcunov <gorcunov@openvz.org>
Subject: [patch 2/2 -mm] prctl: PR_SET_MM_MAP -- Rework validate_prctl_map_locked to use offsets
Date: Sat, 23 Aug 2014 12:46:13 +0400	[thread overview]
Message-ID: <20140823085007.388102981@openvz.org> (raw)
In-Reply-To: 20140823084611.939722362@openvz.org

[-- Attachment #1: prctl-rework-new-mm-map-offsets --]
[-- Type: text/plain, Size: 4791 bytes --]

Andrew proposed to use constant offsets when we check the structure members
which allows to shrink the code size for ~1K. We're safe to use compile-time
calculated offsets because this structure is a part of api and must remain
immutable.

Still there are the ways to change this structure keeping it backward
compatible: size of the structure is a constant value known at the moment
of kernel building time so any new member added to the structure will change
its size and use of offsetof for new members one can calculate which exactly
version user passes to us. Another option is to add explicit @version member.

Also add BUILD_BUG_ON to make sure the structure is not bloated too much.

Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
CC: Oleg Nesterov <oleg@redhat.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Tejun Heo <tj@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andrew Vagin <avagin@openvz.org>
Cc: Eric W. Biederman <ebiederm@xmission.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Serge Hallyn <serge.hallyn@canonical.com>
Cc: Pavel Emelyanov <xemul@parallels.com>
Cc: Vasiliy Kulikov <segoon@openwall.com>
Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Cc: Michael Kerrisk <mtk.manpages@gmail.com>
Cc: Julien Tinnes <jln@google.com>
---
 kernel/sys.c |   73 ++++++++++++++++++++++++++---------------------------------
 1 file changed, 33 insertions(+), 40 deletions(-)

Index: linux-2.6.git/kernel/sys.c
===================================================================
--- linux-2.6.git.orig/kernel/sys.c
+++ linux-2.6.git/kernel/sys.c
@@ -1697,56 +1697,48 @@ static int validate_prctl_map_locked(str
 	unsigned long mmap_max_addr = TASK_SIZE;
 	struct mm_struct *mm = current->mm;
 	struct vm_area_struct *stack_vma;
-	int error = 0;
+	int error = -EINVAL, i;
+
+	static const unsigned char offsets[] = {
+		offsetof(struct prctl_mm_map, start_code),	/*  0 */
+		offsetof(struct prctl_mm_map, end_code),	/*  1 */
+		offsetof(struct prctl_mm_map, start_data),	/*  2 */
+		offsetof(struct prctl_mm_map, end_data),	/*  3 */
+		offsetof(struct prctl_mm_map, start_stack),	/*  4 */
+		offsetof(struct prctl_mm_map, start_brk),	/*  5 */
+		offsetof(struct prctl_mm_map, brk),		/*  6 */
+		offsetof(struct prctl_mm_map, arg_start),	/*  7 */
+		offsetof(struct prctl_mm_map, arg_end),		/*  8 */
+		offsetof(struct prctl_mm_map, env_start),	/*  9 */
+		offsetof(struct prctl_mm_map, env_end),		/* 10 */
+	};
 
 	/*
 	 * Make sure the members are not somewhere outside
 	 * of allowed address space.
 	 */
-#define __prctl_check_addr_space(__member)					\
-	({									\
-		int __rc;							\
-		if ((unsigned long)prctl_map->__member < mmap_max_addr &&	\
-		    (unsigned long)prctl_map->__member >= mmap_min_addr)	\
-			__rc = 0;						\
-		else								\
-			__rc = -EINVAL;						\
-		__rc;								\
-	})
-	error |= __prctl_check_addr_space(start_code);
-	error |= __prctl_check_addr_space(end_code);
-	error |= __prctl_check_addr_space(start_data);
-	error |= __prctl_check_addr_space(end_data);
-	error |= __prctl_check_addr_space(start_stack);
-	error |= __prctl_check_addr_space(start_brk);
-	error |= __prctl_check_addr_space(brk);
-	error |= __prctl_check_addr_space(arg_start);
-	error |= __prctl_check_addr_space(arg_end);
-	error |= __prctl_check_addr_space(env_start);
-	error |= __prctl_check_addr_space(env_end);
-	if (error)
-		goto out;
-#undef __prctl_check_addr_space
+	for (i = 0; i < ARRAY_SIZE(offsets); i++) {
+		u64 val = ((u64 *)prctl_map)[offsets[i]];
+
+		if ((unsigned long)val < mmap_max_addr &&
+		    (unsigned long)val >= mmap_min_addr)
+			goto out;
+
+		/*
+		 * Break, command line arguments and environment must exist.
+		 */
+		if (i >= 5) {
+			if (!(find_vma(mm, (unsigned long)val)))
+				goto out;
+		}
+	}
 
 	/*
-	 * Stack, brk, command line arguments and environment must exist.
+	 * Stack reference will be needed for rlimit check.
 	 */
 	stack_vma = find_vma(mm, (unsigned long)prctl_map->start_stack);
-	if (!stack_vma) {
-		error = -EINVAL;
-		goto out;
-	}
-#define __prctl_check_vma(__member)						\
-	find_vma(mm, (unsigned long)prctl_map->__member) ? 0 : -EINVAL
-	error |= __prctl_check_vma(start_brk);
-	error |= __prctl_check_vma(brk);
-	error |= __prctl_check_vma(arg_start);
-	error |= __prctl_check_vma(arg_end);
-	error |= __prctl_check_vma(env_start);
-	error |= __prctl_check_vma(env_end);
-	if (error)
+	if (!stack_vma)
 		goto out;
-#undef __prctl_check_vma
 
 	/*
 	 * Make sure the pairs are ordered.
@@ -1827,6 +1819,7 @@ static int prctl_set_mm_map(int opt, con
 	int error = -EINVAL;
 
 	BUILD_BUG_ON(sizeof(user_auxv) != sizeof(mm->saved_auxv));
+	BUILD_BUG_ON(sizeof(struct prctl_mm_map) > 256);
 
 	if (opt == PR_SET_MM_MAP_SIZE)
 		return put_user((unsigned int)sizeof(prctl_map),


      parent reply	other threads:[~2014-08-23  8:50 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-08-23  8:46 [patch 0/2 -mm] prctl: PR_SET_MM_MAP -- Drop race window and simplify validate_prctl_map_locked Cyrill Gorcunov
2014-08-23  8:46 ` [patch 1/2 -mm] prctl: PR_SET_MM_MAP -- Copy @auxv from userspace early Cyrill Gorcunov
2014-08-23  8:46 ` Cyrill Gorcunov [this message]

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=20140823085007.388102981@openvz.org \
    --to=gorcunov@openvz.org \
    --cc=akpm@linux-foundation.org \
    --cc=avagin@openvz.org \
    --cc=ebiederm@xmission.com \
    --cc=hpa@zytor.com \
    --cc=jln@google.com \
    --cc=kamezawa.hiroyu@jp.fujitsu.com \
    --cc=keescook@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mtk.manpages@gmail.com \
    --cc=oleg@redhat.com \
    --cc=segoon@openwall.com \
    --cc=serge.hallyn@canonical.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.