All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Andrew Morton <akpm@linux-foundation.org>,
	Linus Torvalds <torvalds@linux-foundation.org>
Cc: Roland McGrath <roland@redhat.com>,
	Andi Kleen <andi@firstfloor.org>,
	"H. Peter Anvin" <hpa@zytor.com>,
	Richard Henderson <rth@twiddle.net>,
	wezhang@redhat.com, linux-kernel@vger.kernel.org,
	Michael Kerrisk <mtk.manpages@gmail.com>,
	William Cohen <wcohen@redhat.com>
Subject: [PATCH 2/2] remove the bogus checks in sys_personality()->__set_personality() path
Date: Fri, 28 May 2010 21:12:31 +0200	[thread overview]
Message-ID: <20100528191231.GD12090@redhat.com> (raw)
In-Reply-To: <20100528191139.GB12090@redhat.com>

Cleanup, no functional changes.

- __set_personality() always changes ->exec_domain/personality, the
  special case when ->exec_domain remains the same buys nothing but
  complicates the code. Unify both cases to simplify the code.

- The -EINVAL check in sys_personality() was never right. If we assume
  that set_personality() can fail we should check the value it returns
  instead of verifying that task->personality was actually changed.

  Remove it. Before the previous patch it was possible to hit this case
  due to overflow problems, but this -EINVAL just indicated the kernel
  bug.

OTOH, probably it makes sense to change lookup_exec_domain() to return
ERR_PTR() instead of default_exec_domain if the search in exec_domains
list fails, and report this error to the user-space. But this means
another user-space change, and we have in-kernel users which need fixes.
For example, PER_OSF4 falls into PER_MASK for unkown reason and nobody
cares to register this domain.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---

 kernel/exec_domain.c |   22 +++++-----------------
 1 file changed, 5 insertions(+), 17 deletions(-)

--- 34-rc1/kernel/exec_domain.c~2_CLEANUP	2010-05-28 18:23:25.000000000 +0200
+++ 34-rc1/kernel/exec_domain.c	2010-05-28 20:07:48.000000000 +0200
@@ -134,23 +134,14 @@ unregister:
 	return 0;
 }
 
-int
-__set_personality(unsigned int personality)
+int __set_personality(unsigned int personality)
 {
-	struct exec_domain	*ep, *oep;
-
-	ep = lookup_exec_domain(personality);
-	if (ep == current_thread_info()->exec_domain) {
-		current->personality = personality;
-		module_put(ep->module);
-		return 0;
-	}
+	struct exec_domain *oep = current_thread_info()->exec_domain;
 
+	current_thread_info()->exec_domain = lookup_exec_domain(personality);
 	current->personality = personality;
-	oep = current_thread_info()->exec_domain;
-	current_thread_info()->exec_domain = ep;
-
 	module_put(oep->module);
+
 	return 0;
 }
 
@@ -192,11 +183,8 @@ SYSCALL_DEFINE1(personality, unsigned in
 {
 	unsigned int old = current->personality;
 
-	if (personality != 0xffffffff) {
+	if (personality != 0xffffffff)
 		set_personality(personality);
-		if (current->personality != personality)
-			return -EINVAL;
-	}
 
 	return old;
 }


  parent reply	other threads:[~2010-05-28 19:14 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-05-25 14:17 Q: sys_personality() && misc oddities Oleg Nesterov
2010-05-25 19:33 ` Roland McGrath
2010-05-26 12:36   ` Oleg Nesterov
2010-05-26 20:31     ` Roland McGrath
2010-05-26 20:35       ` H. Peter Anvin
2010-05-27 15:35       ` [PATCH 0/3] (Was: Q: sys_personality() && misc oddities) Oleg Nesterov
2010-05-27 15:35         ` [PATCH 1/3] sys_personality: validate personality before set_personality() Oleg Nesterov
2010-05-27 16:39           ` Linus Torvalds
2010-05-27 17:15             ` Oleg Nesterov
2010-05-27 17:51               ` Linus Torvalds
2010-05-27 18:13                 ` Oleg Nesterov
2010-05-27 18:18                 ` Andi Kleen
2010-05-28 19:11                 ` [PATCH 0/2] sys_personality fixes v2 Oleg Nesterov
2010-05-28 19:12                   ` [PATCH 1/2] change sys_personality() to accept "unsigned int" instead of u_long Oleg Nesterov
2010-05-28 19:12                   ` Oleg Nesterov [this message]
2010-05-28 19:28                   ` [PATCH 0/2] sys_personality fixes v2 Linus Torvalds
2010-05-28 19:58                     ` H. Peter Anvin
2010-05-28 19:59                     ` Oleg Nesterov
2010-05-27 15:36         ` [PATCH 2/3] sys_personality: make sure (int)personality >= 0 Oleg Nesterov
2010-05-27 20:02           ` H. Peter Anvin
2010-05-28 19:03             ` Oleg Nesterov
2010-05-27 15:36         ` [PATCH 3/3] __set_personality: no need to check the old ->exec_domain Oleg Nesterov

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=20100528191231.GD12090@redhat.com \
    --to=oleg@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=andi@firstfloor.org \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mtk.manpages@gmail.com \
    --cc=roland@redhat.com \
    --cc=rth@twiddle.net \
    --cc=torvalds@linux-foundation.org \
    --cc=wcohen@redhat.com \
    --cc=wezhang@redhat.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.