execve() in reboot.c ?

execve() in reboot.c ?

[aq]

Hello,

Domain kernel has arch/xen/kernel/reboot.c, which executes shutdown
and halt on request. But one problem is that we have the path and
command options fixed in the kernel, like this:


    static char *restart_argv[]  = { "/sbin/shutdown", "-r", "now", NULL };
    static char *poweroff_argv[] = { "/sbin/halt",     "-p",        NULL };


That is kind of violating the rule: kernel should never enforce the
policy to the user. We can see the problem if for example domU uses
busybox instead of sysvinit: busybox doesnt support "halt -p", so "xm
shutdown" cannot shutdown the domU.


Should we care enough to fix this problem?


regards,
aq

RE: execve() in reboot.c ?

[Ian Pratt]

 > Domain kernel has arch/xen/kernel/reboot.c, which executes 
> shutdown and halt on request. But one problem is that we have 
> the path and command options fixed in the kernel, like this:
> 
> 
>     static char *restart_argv[]  = { "/sbin/shutdown", "-r", 
> "now", NULL };
>     static char *poweroff_argv[] = { "/sbin/halt",     "-p",  
>       NULL };
> 
> 
> That is kind of violating the rule: kernel should never 
> enforce the policy to the user. We can see the problem if for 
> example domU uses busybox instead of sysvinit: busybox doesnt 
> support "halt -p", so "xm shutdown" cannot shutdown the domU.

Would 'telinit 1' / 'telinit 6' work on busybox ?

Ian

Re: execve() in reboot.c ?

[aq]

[-- Attachment #1: Type: text/plain, Size: 1441 bytes --]

On 7/25/05, Ian Pratt <m+Ian.Pratt@cl.cam.ac.uk> wrote:
>  > Domain kernel has arch/xen/kernel/reboot.c, which executes
> > shutdown and halt on request. But one problem is that we have
> > the path and command options fixed in the kernel, like this:
> >
> >
> >     static char *restart_argv[]  = { "/sbin/shutdown", "-r",
> > "now", NULL };
> >     static char *poweroff_argv[] = { "/sbin/halt",     "-p",
> >       NULL };
> >
> >
> > That is kind of violating the rule: kernel should never
> > enforce the policy to the user. We can see the problem if for
> > example domU uses busybox instead of sysvinit: busybox doesnt
> > support "halt -p", so "xm shutdown" cannot shutdown the domU.
> 
> Would 'telinit 1' / 'telinit 6' work on busybox ?
> 

Unfortunately telinit is not available in busybox. 

Here are 2 patches (for -testing and -unstable), in which i replaced
shutdown and halt with poweroff and reboot, and executes those without
any options. This patch confirms to work with both sysvinit (on
Ubuntu) and busybox (ttylinux - without these patch i couldnt shutdown
ttylinux with "xm shutdown"). Please apply.


Signed-off-by: Nguyen Anh Quynh <aquynh@gmail.com>


$ diffstat shutdown.testing.patch 
 reboot.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

$ diffstat shutdown.unstable.patch 
 reboot.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

[-- Attachment #2: shutdown.unstable.patch --]
[-- Type: application/octet-stream, Size: 1406 bytes --]

diff -r 48aed1403fe3 linux-2.6-xen-sparse/arch/xen/kernel/reboot.c
--- a/linux-2.6-xen-sparse/arch/xen/kernel/reboot.c	Fri Jul 22 16:44:33 2005
+++ b/linux-2.6-xen-sparse/arch/xen/kernel/reboot.c	Tue Jul 26 02:22:34 2005
@@ -172,8 +172,8 @@
 {
     static char *envp[] = { "HOME=/", "TERM=linux", 
                             "PATH=/sbin:/usr/sbin:/bin:/usr/bin", NULL };
-    static char *restart_argv[]  = { "/sbin/shutdown", "-r", "now", NULL };
-    static char *poweroff_argv[] = { "/sbin/halt",     "-p",        NULL };
+    static char *restart_argv[]  = { "/sbin/reboot", NULL };
+    static char *poweroff_argv[] = { "/sbin/poweroff", NULL };
 
     extern asmlinkage long sys_reboot(int magic1, int magic2,
                                       unsigned int cmd, void *arg);
@@ -187,7 +187,7 @@
     switch ( shutting_down )
     {
     case CMSG_SHUTDOWN_POWEROFF:
-        if ( execve("/sbin/halt", poweroff_argv, envp) < 0 )
+        if ( execve("/sbin/poweroff", poweroff_argv, envp) < 0 )
         {
             sys_reboot(LINUX_REBOOT_MAGIC1,
                        LINUX_REBOOT_MAGIC2,
@@ -197,7 +197,7 @@
         break;
 
     case CMSG_SHUTDOWN_REBOOT:
-        if ( execve("/sbin/shutdown", restart_argv, envp) < 0 )
+        if ( execve("/sbin/reboot", restart_argv, envp) < 0 )
         {
             sys_reboot(LINUX_REBOOT_MAGIC1,
                        LINUX_REBOOT_MAGIC2,

[-- Attachment #3: shutdown.testing.patch --]
[-- Type: application/octet-stream, Size: 1415 bytes --]

diff -r 026436ebd99c linux-2.6.11-xen-sparse/arch/xen/kernel/reboot.c
--- a/linux-2.6.11-xen-sparse/arch/xen/kernel/reboot.c	Mon Jul 18 07:45:41 2005
+++ b/linux-2.6.11-xen-sparse/arch/xen/kernel/reboot.c	Tue Jul 26 02:24:38 2005
@@ -159,8 +159,8 @@
 {
     static char *envp[] = { "HOME=/", "TERM=linux", 
                             "PATH=/sbin:/usr/sbin:/bin:/usr/bin", NULL };
-    static char *restart_argv[]  = { "/sbin/shutdown", "-r", "now", NULL };
-    static char *poweroff_argv[] = { "/sbin/halt",     "-p",        NULL };
+    static char *restart_argv[]  = { "/sbin/reboot", NULL };
+    static char *poweroff_argv[] = { "/sbin/poweroff", NULL };
 
     extern asmlinkage long sys_reboot(int magic1, int magic2,
                                       unsigned int cmd, void *arg);
@@ -174,7 +174,7 @@
     switch ( shutting_down )
     {
     case CMSG_SHUTDOWN_POWEROFF:
-        if ( execve("/sbin/halt", poweroff_argv, envp) < 0 )
+        if ( execve("/sbin/poweroff", poweroff_argv, envp) < 0 )
         {
             sys_reboot(LINUX_REBOOT_MAGIC1,
                        LINUX_REBOOT_MAGIC2,
@@ -184,7 +184,7 @@
         break;
 
     case CMSG_SHUTDOWN_REBOOT:
-        if ( execve("/sbin/shutdown", restart_argv, envp) < 0 )
+        if ( execve("/sbin/reboot", restart_argv, envp) < 0 )
         {
             sys_reboot(LINUX_REBOOT_MAGIC1,
                        LINUX_REBOOT_MAGIC2,

[-- Attachment #4: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: execve() in reboot.c ?

[Mark Williamson]

Can't we just fire off a kernel thread to call sys_reboot and friends 
directly?  It seems unfortunate to have to rely on userspace applications, 
having gone to the trouble of handling the cmsg in the kernel.

Cheers,
Mark

On Monday 25 July 2005 18:41, aq wrote:
> On 7/25/05, Ian Pratt <m+Ian.Pratt@cl.cam.ac.uk> wrote:
> >  > Domain kernel has arch/xen/kernel/reboot.c, which executes
> > >
> > > shutdown and halt on request. But one problem is that we have
> > > the path and command options fixed in the kernel, like this:
> > >
> > >
> > >     static char *restart_argv[]  = { "/sbin/shutdown", "-r",
> > > "now", NULL };
> > >     static char *poweroff_argv[] = { "/sbin/halt",     "-p",
> > >       NULL };
> > >
> > >
> > > That is kind of violating the rule: kernel should never
> > > enforce the policy to the user. We can see the problem if for
> > > example domU uses busybox instead of sysvinit: busybox doesnt
> > > support "halt -p", so "xm shutdown" cannot shutdown the domU.
> >
> > Would 'telinit 1' / 'telinit 6' work on busybox ?
>
> Unfortunately telinit is not available in busybox.
>
> Here are 2 patches (for -testing and -unstable), in which i replaced
> shutdown and halt with poweroff and reboot, and executes those without
> any options. This patch confirms to work with both sysvinit (on
> Ubuntu) and busybox (ttylinux - without these patch i couldnt shutdown
> ttylinux with "xm shutdown"). Please apply.
>
>
> Signed-off-by: Nguyen Anh Quynh <aquynh@gmail.com>
>
>
> $ diffstat shutdown.testing.patch
>  reboot.c |    8 ++++----
>  1 files changed, 4 insertions(+), 4 deletions(-)
>
> $ diffstat shutdown.unstable.patch
>  reboot.c |    8 ++++----
>  1 files changed, 4 insertions(+), 4 deletions(-)

Re: execve() in reboot.c ?

[Jeremy Katz]

On Mon, 2005-07-25 at 18:47 +0100, Mark Williamson wrote:
> Can't we just fire off a kernel thread to call sys_reboot and friends 
> directly?  It seems unfortunate to have to rely on userspace applications, 
> having gone to the trouble of handling the cmsg in the kernel.

That doesn't do a graceful shutdown of running processes, does it?

Jeremy

Re: execve() in reboot.c ?

[Mark Williamson]

> > Can't we just fire off a kernel thread to call sys_reboot and friends
> > directly?  It seems unfortunate to have to rely on userspace
> > applications, having gone to the trouble of handling the cmsg in the
> > kernel.
>
> That doesn't do a graceful shutdown of running processes, does it?

Hmmmm, I suppose not.  Ah well, I think I'll withdraw my previous statement as 
I can't think of another way to do this right now :-)

It'd be nice if Linux had a generic way of communicating a reboot request to 
userspace.  It ought to be useful on other VMMs - maybe this is something for 
Rik's common infrastructure wishlist?

Cheers,
Mark

Re: execve() in reboot.c ?

[Rik van Riel]

On Mon, 25 Jul 2005, Mark Williamson wrote:

> It'd be nice if Linux had a generic way of communicating a reboot 
> request to userspace.  It ought to be useful on other VMMs - maybe this 
> is something for Rik's common infrastructure wishlist?

How about simply reusing whatever signal is sent to init
when ctrl-alt-del is pressed on PCs ?

-- 
The Theory of Escalating Commitment: "The cost of continuing mistakes is
borne by others, while the cost of admitting mistakes is borne by yourself."
  -- Joseph Stiglitz, Nobel Laureate in Economics

Re: execve() in reboot.c ?

[Vincent Hanquez]

On Mon, Jul 25, 2005 at 02:13:25PM -0400, Rik van Riel wrote:
> On Mon, 25 Jul 2005, Mark Williamson wrote:
> 
> > It'd be nice if Linux had a generic way of communicating a reboot 
> > request to userspace.  It ought to be useful on other VMMs - maybe this 
> > is something for Rik's common infrastructure wishlist?
> 
> How about simply reusing whatever signal is sent to init
> when ctrl-alt-del is pressed on PCs ?

SIGINT

That's not a good idea though, if somebody put nothing in the ctrlaltdel
action, then it's not going to do anything.

-- 
Vincent Hanquez

Re: execve() in reboot.c ?

[aq]

On 7/26/05, Mark Williamson <mark.williamson@cl.cam.ac.uk> wrote:
> > > Can't we just fire off a kernel thread to call sys_reboot and friends
> > > directly?  It seems unfortunate to have to rely on userspace
> > > applications, having gone to the trouble of handling the cmsg in the
> > > kernel.
> >
> > That doesn't do a graceful shutdown of running processes, does it?
> 
> Hmmmm, I suppose not.  Ah well, I think I'll withdraw my previous statement as
> I can't think of another way to do this right now :-)
> 

actually what you suggested is in use now ;-). firstly, the code use
daemonize() to become kernel thread, then execute userspace code
(shutdown/poweroff/halt...). if that failed (for example in case there
is no such userspace binary), sys_reboot is called.

i dont see why we try to call userspace comand first, then fail thru
to sys_reboot(). why just call sys_reboot()?


regards,
aq

Re: execve() in reboot.c ?

[Mark Williamson]

On Monday 25 July 2005 19:13, Rik van Riel wrote:
> On Mon, 25 Jul 2005, Mark Williamson wrote:
> > It'd be nice if Linux had a generic way of communicating a reboot
> > request to userspace.  It ought to be useful on other VMMs - maybe this
> > is something for Rik's common infrastructure wishlist?
>
> How about simply reusing whatever signal is sent to init
> when ctrl-alt-del is pressed on PCs ?

Sounds good.  Something like this must be done for smart power button handling 
in the ACPI code, so that might be a good place to refer to.

It'd be nice if we could communicate the difference between a reboot and a 
halt request though.  Alternatively, the tools could just remember what we 
"meant" the restart to do and do the right thing when the domain quits.

Cheers,
Mark

Re: execve() in reboot.c ?

[Mark Williamson]

> actually what you suggested is in use now ;-). firstly, the code use
> daemonize() to become kernel thread, then execute userspace code
> (shutdown/poweroff/halt...). if that failed (for example in case there
> is no such userspace binary), sys_reboot is called.

Yeah, I just looked at the code and realised I was just spouting rubbish 
earlier :-)

> i dont see why we try to call userspace comand first, then fail thru
> to sys_reboot(). why just call sys_reboot()?

As Jeremy said, the reboot command does rather more than just call sys_reboot.  
The sys_reboot call means "really reboot now", without doing the nice things 
init does (unmounting filesystems, stopping processes politely, etc).

I think there must be another way of achieving this effect without exec-ing 
things, though.

Cheers,
Mark

Re: execve() in reboot.c ?

[Anthony Liguori]

Vincent Hanquez wrote:

>On Mon, Jul 25, 2005 at 02:13:25PM -0400, Rik van Riel wrote:
>  
>
>>On Mon, 25 Jul 2005, Mark Williamson wrote:
>>
>>    
>>
>>>It'd be nice if Linux had a generic way of communicating a reboot 
>>>request to userspace.  It ought to be useful on other VMMs - maybe this 
>>>is something for Rik's common infrastructure wishlist?
>>>      
>>>
>>How about simply reusing whatever signal is sent to init
>>when ctrl-alt-del is pressed on PCs ?
>>    
>>
>
>SIGINT
>
>That's not a good idea though, if somebody put nothing in the ctrlaltdel
>action, then it's not going to do anything.
>  
>
Aren't we trying to something a bit unnatural here?  Clearly, shutting 
down is something best done from userspace.

Maybe it's time to start thinking about domU tools for Xen userspace?  
If the XenStore API can be accessible via userspace we could easily 
write some patches (or special daemons) that could do this sort of 
stuff.  Might be particularily useful if we want to provide more 
sophisticated stuff in the future.

Not something for Xen 3.0 but might be interesting for 3.1.x.  
Thoughts?  Am I crazy? :-)

Regards,

Anthony Liguori

Re: execve() in reboot.c ?

[Vincent Hanquez]

On Tue, Jul 26, 2005 at 03:26:34AM +0900, aq wrote:
> i dont see why we try to call userspace comand first, then fail thru
> to sys_reboot(). why just call sys_reboot()?

because sys_reboot doesn't do:

- a clean shutdown of all processes of the machines.
- unmount partitions.
- sync data to disk.
- problably some more reasons.

-- 
Vincent Hanquez

Re: execve() in reboot.c ?

[Rik van Riel]

On Mon, 25 Jul 2005, Vincent Hanquez wrote:
> On Mon, Jul 25, 2005 at 02:13:25PM -0400, Rik van Riel wrote:

> > How about simply reusing whatever signal is sent to init
> > when ctrl-alt-del is pressed on PCs ?
> 
> SIGINT
> 
> That's not a good idea though, if somebody put nothing in the ctrlaltdel
> action, then it's not going to do anything.

But isn't that exactly what we want ?   A userspace configurable
way to trigger a system reboot.

-- 
The Theory of Escalating Commitment: "The cost of continuing mistakes is
borne by others, while the cost of admitting mistakes is borne by yourself."
  -- Joseph Stiglitz, Nobel Laureate in Economics

RE: execve() in reboot.c ?

[Ian Pratt]

 
> > > How about simply reusing whatever signal is sent to init when 
> > > ctrl-alt-del is pressed on PCs ?
> > 
> > SIGINT
> > 
> > That's not a good idea though, if somebody put nothing in the 
> > ctrlaltdel action, then it's not going to do anything.
> 
> But isn't that exactly what we want ?   A userspace configurable
> way to trigger a system reboot.

I kind of like this approach, but it does mean that we can't distinguish
between a poweroff and a reboot. Do we care?

As I recall, the current approach was copied from one of the other Linux
architectures. 

Ian

Re: execve() in reboot.c ?

[Sean Dague]

[-- Attachment #1.1: Type: text/plain, Size: 1790 bytes --]

On Mon, Jul 25, 2005 at 08:48:12PM +0100, Ian Pratt wrote:
>  
> > > > How about simply reusing whatever signal is sent to init when 
> > > > ctrl-alt-del is pressed on PCs ?
> > > 
> > > SIGINT
> > > 
> > > That's not a good idea though, if somebody put nothing in the 
> > > ctrlaltdel action, then it's not going to do anything.
> > 
> > But isn't that exactly what we want ?   A userspace configurable
> > way to trigger a system reboot.
> 
> I kind of like this approach, but it does mean that we can't distinguish
> between a poweroff and a reboot. Do we care?
> 
> As I recall, the current approach was copied from one of the other Linux
> architectures. 

Additionally modern init supports "powerfail" which is the action to be
triggered when the power fails due to UPS.  Using powerfail and ctrlaltdel
as the 2 actions would provide the different triggers needed for shutdown. 
It would allow for userspace configurability very easily.

Though the original problem could probably be solved just by changing the
hard coded user callouts to "/sbin/reboot" and "/sbin/poweroff", which I
believe both exist in busybox (or can both exist).  Given that other things
like hotplug and modprobe are directly called out from the kernel, this
might be an ok approach.

	-Sean

-- 
__________________________________________________________________

Sean Dague                                       Mid-Hudson Valley
sean at dague dot net                            Linux Users Group
http://dague.net                                 http://mhvlug.org

There is no silver bullet.  Plus, werewolves make better neighbors
than zombies, and they tend to keep the vampire population down.
__________________________________________________________________

[-- Attachment #1.2: Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: execve() in reboot.c ?

[Philip R Auld]

Rumor has it that on Mon, Jul 25, 2005 at 08:48:12PM +0100 Ian Pratt said:
>  
> > > > How about simply reusing whatever signal is sent to init when 
> > > > ctrl-alt-del is pressed on PCs ?
> > > 
> > > SIGINT
> > > 
> > > That's not a good idea though, if somebody put nothing in the 
> > > ctrlaltdel action, then it's not going to do anything.
> > 
> > But isn't that exactly what we want ?   A userspace configurable
> > way to trigger a system reboot.
> 
> I kind of like this approach, but it does mean that we can't distinguish
> between a poweroff and a reboot. Do we care?
> 

yes, I think there's a difference... If I halt a guest I don't expect
it to reboot.


> As I recall, the current approach was copied from one of the other Linux
> architectures. 

Could these be new hotplug events? That would get them out to
userspace where they would presumably be configurable.


Cheers,

Phil

> 
> Ian
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

-- 
Philip R. Auld, Ph.D.  	        	       Egenera, Inc.    
Software Architect                            165 Forest St.
(508) 858-2628                            Marlboro, MA 01752

Re: execve() in reboot.c ?

[Keir Fraser]

On 25 Jul 2005, at 21:48, Mark Williamson wrote:

>> As I recall, the current approach was copied from one of the other 
>> Linux
>> architectures.
>
> Personally I'd vote for signalling userspace rather than shelling a 
> command
> but I'll leave it up to whoever wants to write the code ;-)

Exec'ing a userspace program is a recognised method for signalling 
userspace these days, I believe. That's how hotplug works, certainly, 
and it does have the benefit of 'obviousness'.

  -- Keir

Re: execve() in reboot.c ?

[Mark Williamson]

<snip signalling to init>
> I kind of like this approach, but it does mean that we can't distinguish
> between a poweroff and a reboot. Do we care?

The tools could perhaps distinguish: they just have to remember if you asked 
for a reboot or a halt, then do the right thing.  It seems unlikely you'd 
want to ask for "reboot" on the command line but really have the domain halt.

> As I recall, the current approach was copied from one of the other Linux
> architectures.

Personally I'd vote for signalling userspace rather than shelling a command 
but I'll leave it up to whoever wants to write the code ;-)

Cheers,
Mark

Re: execve() in reboot.c ?

[Rik van Riel]

On Mon, 25 Jul 2005, Mark Williamson wrote:

> > As I recall, the current approach was copied from one of the other Linux
> > architectures.
> 
> Personally I'd vote for signalling userspace rather than shelling a command 
> but I'll leave it up to whoever wants to write the code ;-)

A hotplug event might work too...

-- 
The Theory of Escalating Commitment: "The cost of continuing mistakes is
borne by others, while the cost of admitting mistakes is borne by yourself."
  -- Joseph Stiglitz, Nobel Laureate in Economics

Re: execve() in reboot.c ?

[Gerd Knorr]

Philip R Auld <pauld@egenera.com> writes:

> > I kind of like this approach, but it does mean that we can't distinguish
> > between a poweroff and a reboot. Do we care?
> 
> yes, I think there's a difference... If I halt a guest I don't expect
> it to reboot.

Especially if you want to reboot the (physical) machine with something
like "xm shutdown -a -w && reboot" in domain0 ...

  Gerd

-- 
panic("it works"); /* avoid being flooded with debug messages */