All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vineet Gupta <Vineet.Gupta1@synopsys.com>
To: Al Viro <viro@ZenIV.linux.org.uk>
Cc: linux-arch@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [RFC] status of execve() work - per-architecture patches solicited
Date: Wed, 19 Sep 2012 17:50:34 +0530	[thread overview]
Message-ID: <5059B892.4030806@synopsys.com> (raw)
In-Reply-To: <20120907182004.GE13973@ZenIV.linux.org.uk>

On Friday 07 September 2012 11:50 PM, Al Viro wrote:
> 	To architecture maintainers: please, review the current
> situation in git.kernel.org/pub/scm/linux/kernel/git/viro/signal #execve2
> and consider sending the corresponding patches for missing architectures.
> 
> 	What's getting done is unification of sys_execve()/kernel_execve()
> into arch-independent code.  x86, alpha, arm, s390, um and ppc are already
> converted in #execve2.  The plan is:
> 
> * provide a new primitive - ret_from_kernel_execve(); it takes two pointers
> to struct pt_regs, one being the normal location of pt_regs for a userland
> process, another - new pt_regs just filled by do_execve().  It should copy
> the latter to the former and bugger off to userland.  Called from generic
> kernel_execve() implementation (see fs/exec.c in #execve2).  It almost always
> has to be done in assembler - normally it does equivalent of something
> along the lines of
> 	memmove(normal, new, sizeof(struct pt_regs))
> 	sp = normal, or whatever is needed to get a valid stack
> frame (e.g. on s390 there's ->back_chain that needs to be set to
> NULL)
> 	set other registers ret_from_sys_call expects to be set (e.g.
> i386 syscall entry has current_thread_info() value cached in %ebp and
> since it's a callee-saved register there, ret_from_sys_call expects to
> find that value still in %ebp, so we need to set it); basically, check
> what has to be set in ret_from_fork - it tends to jump to the same place.
> 	goto ret_from_sys_call, or whatever the equivalent is called on
> particular architecture.
> * define __ARCH_WANT_KERNEL_EXECVE in unistd.h, remove your old kernel_execve()
> * pull whatever work you'd been doing *after* do_execve() call in your
> sys_execve() (most of the architectures don't do anything after that anyway)
> into start_thread(); that's the point of no return for execve(2) and if we
> get there, we'll either succeed or get killed with SIGKILL.  The same goes
> for compat variant of execve(), with s/start_thread/compat_start_thread/.
> * define __ARCH_WANT_SYS_EXECVE in unistd.h, kill your sys_execve() and
> compat counterpart (if any).
> * if there's a better way to calculate task_pt_regs(current), you can provide
> it in your ptrace.h - macro should be called current_pt_regs(); it's optional.
> 
> 	Status: x86, arm, um, s390 - converted, tested, seem to work. alpha
> and ppc - need testing.  The rest - hadn't touched yet.  unicore32 and
> blackfin should be trivial to convert (they are doing kernel_execve() in
> that manner already).  Other may be more or less tricky - depends on how
> gnarly their return from syscall path happens to be.  I'll do what I can
> and test what I can (some on emulators, some on real hardware), but for quite
> a few architectures I've no way to test.  Nor am I fond of sniffing dozens
> of variants of assembler glue, to put it mildly.
> 
> 	Patches and/or help with testing setups would be very welcome.
> 


Hi Al,

It must be noted that despite having seemingly independent
__ARCH_WANT_(KERNEL|SYS)_EXECVE, arches which have a kernel syscall trap
based kernel_execve(), e.g. MIPS, can't implement __ARCH_WANT_SYS_EXECVE
alone - they need to first convert
to __ARCH_WANT_KERNEL_EXECVE as well (although it probably doesn't make
sense for anyone to just implement one - but in terms of staging -
having only one, breaks stuff IMHO).

The reason being, for non converted kernel_execve(), the call-stack
leading to sys_execve (e.g. init_post -> run_init_process ->
kernel_execve ->..)  would cause the pt_regs layout to be slightly
offsetted from bottom of stack - not exactly where
current_pt_regs()/task_pt_regs(current) would point to in general. Thus
on return path the update by start_thread() won't be visible to asm glue
at expected location.

I ran into this myself - when doing the execve switch for ARC Linux port
(currently being "pre-reviewed" by tglx before submission to lkml).


-Vineet

WARNING: multiple messages have this Message-ID (diff)
From: Vineet Gupta <Vineet.Gupta1@synopsys.com>
To: Al Viro <viro@ZenIV.linux.org.uk>
Cc: <linux-arch@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [RFC] status of execve() work - per-architecture patches solicited
Date: Wed, 19 Sep 2012 17:50:34 +0530	[thread overview]
Message-ID: <5059B892.4030806@synopsys.com> (raw)
In-Reply-To: <20120907182004.GE13973@ZenIV.linux.org.uk>

On Friday 07 September 2012 11:50 PM, Al Viro wrote:
> 	To architecture maintainers: please, review the current
> situation in git.kernel.org/pub/scm/linux/kernel/git/viro/signal #execve2
> and consider sending the corresponding patches for missing architectures.
> 
> 	What's getting done is unification of sys_execve()/kernel_execve()
> into arch-independent code.  x86, alpha, arm, s390, um and ppc are already
> converted in #execve2.  The plan is:
> 
> * provide a new primitive - ret_from_kernel_execve(); it takes two pointers
> to struct pt_regs, one being the normal location of pt_regs for a userland
> process, another - new pt_regs just filled by do_execve().  It should copy
> the latter to the former and bugger off to userland.  Called from generic
> kernel_execve() implementation (see fs/exec.c in #execve2).  It almost always
> has to be done in assembler - normally it does equivalent of something
> along the lines of
> 	memmove(normal, new, sizeof(struct pt_regs))
> 	sp = normal, or whatever is needed to get a valid stack
> frame (e.g. on s390 there's ->back_chain that needs to be set to
> NULL)
> 	set other registers ret_from_sys_call expects to be set (e.g.
> i386 syscall entry has current_thread_info() value cached in %ebp and
> since it's a callee-saved register there, ret_from_sys_call expects to
> find that value still in %ebp, so we need to set it); basically, check
> what has to be set in ret_from_fork - it tends to jump to the same place.
> 	goto ret_from_sys_call, or whatever the equivalent is called on
> particular architecture.
> * define __ARCH_WANT_KERNEL_EXECVE in unistd.h, remove your old kernel_execve()
> * pull whatever work you'd been doing *after* do_execve() call in your
> sys_execve() (most of the architectures don't do anything after that anyway)
> into start_thread(); that's the point of no return for execve(2) and if we
> get there, we'll either succeed or get killed with SIGKILL.  The same goes
> for compat variant of execve(), with s/start_thread/compat_start_thread/.
> * define __ARCH_WANT_SYS_EXECVE in unistd.h, kill your sys_execve() and
> compat counterpart (if any).
> * if there's a better way to calculate task_pt_regs(current), you can provide
> it in your ptrace.h - macro should be called current_pt_regs(); it's optional.
> 
> 	Status: x86, arm, um, s390 - converted, tested, seem to work. alpha
> and ppc - need testing.  The rest - hadn't touched yet.  unicore32 and
> blackfin should be trivial to convert (they are doing kernel_execve() in
> that manner already).  Other may be more or less tricky - depends on how
> gnarly their return from syscall path happens to be.  I'll do what I can
> and test what I can (some on emulators, some on real hardware), but for quite
> a few architectures I've no way to test.  Nor am I fond of sniffing dozens
> of variants of assembler glue, to put it mildly.
> 
> 	Patches and/or help with testing setups would be very welcome.
> 


Hi Al,

It must be noted that despite having seemingly independent
__ARCH_WANT_(KERNEL|SYS)_EXECVE, arches which have a kernel syscall trap
based kernel_execve(), e.g. MIPS, can't implement __ARCH_WANT_SYS_EXECVE
alone - they need to first convert
to __ARCH_WANT_KERNEL_EXECVE as well (although it probably doesn't make
sense for anyone to just implement one - but in terms of staging -
having only one, breaks stuff IMHO).

The reason being, for non converted kernel_execve(), the call-stack
leading to sys_execve (e.g. init_post -> run_init_process ->
kernel_execve ->..)  would cause the pt_regs layout to be slightly
offsetted from bottom of stack - not exactly where
current_pt_regs()/task_pt_regs(current) would point to in general. Thus
on return path the update by start_thread() won't be visible to asm glue
at expected location.

I ran into this myself - when doing the execve switch for ARC Linux port
(currently being "pre-reviewed" by tglx before submission to lkml).


-Vineet

  parent reply	other threads:[~2012-09-19 12:44 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-07 18:20 [RFC] status of execve() work - per-architecture patches solicited Al Viro
2012-09-07 18:22 ` Al Viro
2012-09-10 13:40 ` Greg Ungerer
2012-09-10 16:49   ` Al Viro
2012-09-11  3:39     ` Greg Ungerer
2012-09-13 13:27     ` Greg Ungerer
2012-09-13 13:27       ` Greg Ungerer
2012-09-10 22:20 ` Mark Salter
2012-09-10 22:20   ` [PATCH 1/2] c6x: implement ret_from_kernel_execve() and switch to generic kernel_execve() Mark Salter
2012-09-10 22:20   ` [PATCH 2/2] c6x: switch to generic sys_execve() Mark Salter
2012-09-17  3:26   ` [RFC] status of execve() work - per-architecture patches solicited Al Viro
2012-09-21 16:26     ` Mark Salter
2012-09-21 16:26       ` [PATCH 1/3] c6x: add ret_from_kernel_thread(), simplify kernel_thread() Mark Salter
2012-09-21 16:26       ` [PATCH 2/3] c6x: switch to generic kernel_execve Mark Salter
2012-09-21 16:26       ` [PATCH 3/3] c6x: switch to generic sys_execve Mark Salter
2012-09-21 18:39       ` [RFC] status of execve() work - per-architecture patches solicited Al Viro
2012-09-22 11:16         ` Greg Ungerer
2012-09-23  0:46           ` Al Viro
2012-09-24 10:59             ` Vineet Gupta
2012-09-24 10:59               ` Vineet Gupta
2012-09-17  9:29 ` Michal Simek
2012-09-17 22:57   ` Al Viro
2012-09-19 12:20 ` Vineet Gupta [this message]
2012-09-19 12:20   ` Vineet Gupta
2012-09-19 13:32   ` Al Viro

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=5059B892.4030806@synopsys.com \
    --to=vineet.gupta1@synopsys.com \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=viro@ZenIV.linux.org.uk \
    /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.