call_usermodehelper does not report exit status?

call_usermodehelper does not report exit status?

[Omen Wild]

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

As part of a LSM I am writing, I need to call a user-space program and
check its return status.  I found the call_usermodehelper function and
call it with the wait flag set, but I cannot get a non-zero return
status of the program to propagate into the kernel.  If I try to run a
non-existent program then call_usermodehelper returns -1, so at least
some errors propagate properly.  I have attached a trivial LSM test
program that hooks inode_rename, runs /bin/false and prints the return
status of call_usermodehelper.  

This is with kernel 2.6.0-test5-mm3 compiled on an up to date Debian
unstable.

For simplicity I have also attached a patch to security/Makefile to
build this test LSM as a kernel module.

Before I break out UML or the kernel debugger, does anyone have any
ideas what I am doing wrong?

Thanks,
  Omen

-- 
There is much Obi-Wan did not tell you.

[-- Attachment #1.2: Makefile.patch --]
[-- Type: text/plain, Size: 319 bytes --]

--- Makefile.orig       2003-09-19 12:18:20.000000000 -0400
+++ Makefile    2003-09-19 12:14:11.000000000 -0400
@@ -18,3 +18,6 @@
 
 obj-$(CONFIG_SECURITY_CAPABILITIES)    += commoncap.o capability.o
 obj-$(CONFIG_SECURITY_ROOTPLUG)                += commoncap.o root_plug.o
+
+# Test modules
+obj-m += test.o

[-- Attachment #1.3: test.c --]
[-- Type: text/plain, Size: 1748 bytes --]

#include <linux/config.h>
#include <linux/kernel.h>
#include <linux/security.h>
#include <linux/stat.h>
#include <linux/skbuff.h>
#include <linux/netlink.h>
#include <linux/ctype.h>
#include <linux/file.h>
#include <linux/spinlock.h>

#include <linux/fs.h>
#include <net/tcp.h>
#include <linux/sched.h>
#include <linux/string.h>
#include <linux/vmalloc.h>
#include <linux/dcache.h>
#include <linux/list.h>
#include <linux/namespace.h>
#include <linux/writeback.h>
#include <linux/quotaops.h>

#if defined(CONFIG_SECURITY_enforcer_MODULE)
#define MY_NAME THIS_MODULE->name
#else
#define MY_NAME "Test"
#endif

static int test_inode_rename (struct inode *inode, int mask)
{
   int status;
   char *envp[] = {
	  "HOME=/",
	  "TERM=linux",
	  "PATH=/usr/sbin:/sbin:/bin:/usr/bin",
	  NULL };

   char *argv[] = {
	  "/bin/false",
	  NULL };

   printk(KERN_INFO "calling helper '%s'\n",
		  argv[0]);

   status = call_usermodehelper(argv[0], argv, envp, 1);

   printk(KERN_INFO "helper returned = %d\n", status);

   return 0;
}

static struct security_operations test_ops = {
   .inode_rename = test_inode_rename,
};

static int __init test_init (void)
{
   /* register ourselves with the security framework */
   if (register_security (&test_ops)) {
	  printk (KERN_INFO "Failure registering " MY_NAME " module with the kernel\n");
	  return -EINVAL;
   }

   printk (KERN_INFO MY_NAME " LSM initialized.\n");
   return 0;
}

static void __exit test_exit (void)
{
   if (unregister_security (&test_ops))
	  printk (KERN_INFO MY_NAME ": failure unregistering with the kernel.\n");
}


module_init (test_init);
module_exit (test_exit);

MODULE_AUTHOR("Omen Wild <Omen.Wild@Dartmouth.EDU>");
MODULE_DESCRIPTION("Test Module");
MODULE_LICENSE("GPL");

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

Re: call_usermodehelper does not report exit status?

[Andrew Morton]

Omen Wild <Omen.Wild@Dartmouth.EDU> wrote:
>
> I found the call_usermodehelper function and
> call it with the wait flag set, but I cannot get a non-zero return
> status of the program to propagate into the kernel.

This might fix it.

 25-akpm/kernel/exit.c |   21 +++++++++++++++++----
 25-akpm/kernel/kmod.c |    2 +-
 2 files changed, 18 insertions(+), 5 deletions(-)

diff -puN kernel/kmod.c~call_usermodehelper-retval-fix kernel/kmod.c
--- 25/kernel/kmod.c~call_usermodehelper-retval-fix	Fri Sep 19 11:14:47 2003
+++ 25-akpm/kernel/kmod.c	Fri Sep 19 11:20:14 2003
@@ -190,7 +190,7 @@ static int wait_for_helper(void *data)
 		/* We don't have a SIGCHLD signal handler, so this
 		 * always returns -ECHILD, but the important thing is
 		 * that it blocks. */
-		sys_wait4(pid, NULL, 0, NULL);
+		sys_wait4(pid, &sub_info->retval, 0, NULL);
 
 	complete(sub_info->complete);
 	return 0;
diff -puN kernel/exit.c~call_usermodehelper-retval-fix kernel/exit.c
--- 25/kernel/exit.c~call_usermodehelper-retval-fix	Fri Sep 19 11:16:59 2003
+++ 25-akpm/kernel/exit.c	Fri Sep 19 11:20:10 2003
@@ -883,10 +883,17 @@ static int wait_task_zombie(task_t *p, u
 
 	retval = ru ? getrusage(p, RUSAGE_BOTH, ru) : 0;
 	if (!retval && stat_addr) {
+		int stat;
+
 		if (p->signal->group_exit)
-			retval = put_user(p->signal->group_exit_code, stat_addr);
+			stat = p->signal->group_exit_code;
+		else
+			stat = p->exit_code;
+
+		if (current->mm)
+			retval = put_user(stat, stat_addr);
 		else
-			retval = put_user(p->exit_code, stat_addr);
+			retval = __put_user(stat, stat_addr);
 	}
 	if (retval) {
 		p->state = TASK_ZOMBIE;
@@ -987,8 +994,14 @@ static int wait_task_stopped(task_t *p, 
 	write_unlock_irq(&tasklist_lock);
 
 	retval = ru ? getrusage(p, RUSAGE_BOTH, ru) : 0;
-	if (!retval && stat_addr)
-		retval = put_user((exit_code << 8) | 0x7f, stat_addr);
+	if (!retval && stat_addr) {
+		int stat = (exit_code << 8) | 0x7f;
+
+		if (current->mm)
+			retval = put_user(stat, stat_addr);
+		else
+			retval = __put_user(stat, stat_addr);
+	}
 	if (!retval)
 		retval = p->pid;
 	put_task_struct(p);

_

Re: call_usermodehelper does not report exit status?

[Omen Wild]

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

Quoting Andrew Morton <akpm@osdl.org> on Fri, Sep 19 11:21:
>
> This might fix it.

Hmmm, that did not fix it for me.  No change in behavior.

-- 
There are more ways into the woods than out.

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

Re: call_usermodehelper does not report exit status?

[Andrew Morton]

Omen Wild <Omen.Wild@Dartmouth.EDU> wrote:
>
> Quoting Andrew Morton <akpm@osdl.org> on Fri, Sep 19 11:21:
> >
> > This might fix it.
> 
> Hmmm, that did not fix it for me.  No change in behavior.
> 

Curses, foiled again.

I agree that as long as we are supporting synchronous callouts we should be
correctly returning the exit code.   I'll work on it a bit.

Re: call_usermodehelper does not report exit status?

[Milton D. Miller II]

Andrew Morton <akpm@osdl.org> wrote:
> Omen Wild <Omen.Wild@Dartmouth.EDU> wrote:
> >
> > I found the call_usermodehelper function and
> > call it with the wait flag set, but I cannot get a non-zero return
> > status of the program to propagate into the kernel.
> 
> This might fix it.


I think you missed the why behind the comment just above your first change.

 		/* We don't have a SIGCHLD signal handler, so this
 		 * always returns -ECHILD, but the important thing is
 		 * that it blocks. */
-		sys_wait4(pid, NULL, 0, NULL);
+		sys_wait4(pid, &sub_info->retval, 0, NULL);


The exit code notices that there is no signal handler for SIGCHILD and
does a fast exit, then we notice when woken up the child no longer exists.

Rusty discovered this back in June when we were trying to fix a checker
error on the wait call, and decided that at the time no one was using the
return value, hence the simpler fix.  

http://linux.bkbits.net:8080/linux-2.5/cset@1.1046.366.23?nav=index.html|src/|src/kernel|related/kernel/kmod.c


milton

Re: call_usermodehelper does not report exit status?

[Chris Wright]

* Milton D. Miller II (miltonm@realtime.net) wrote:
> Andrew Morton <akpm@osdl.org> wrote:
> > This might fix it.
> 
> I think you missed the why behind the comment just above your first change.

Anything wrong with just setting a SIG_DFL handler?  W.R.T. the kernel
pointer, either Andrew's patch which does put_user/__put_user depending
on context, or some ugly set_fs() should work.  This simplistic approach
works for me, thoughts?

thanks,
-chris

-- 
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net


--- linux-2.6.0-test5-mm4/kernel/kmod.c	2003-09-08 12:49:59.000000000 -0700
+++ 2.6.0-test5-mm4/kernel/kmod.c	2003-09-25 11:34:59.000000000 -0700
@@ -181,16 +181,24 @@
 {
 	struct subprocess_info *sub_info = data;
 	pid_t pid;
+	struct k_sigaction sa;
+
+	sa.sa.sa_handler = SIG_DFL;
+	sa.sa.sa_flags = 0;
+	siginitset(&sa.sa.sa_mask, sigmask(SIGCHLD));
+	do_sigaction(SIGCHLD, &sa, (struct k_sigaction *)0);
 
 	sub_info->retval = 0;
 	pid = kernel_thread(____call_usermodehelper, sub_info, SIGCHLD);
 	if (pid < 0)
 		sub_info->retval = pid;
-	else
-		/* We don't have a SIGCHLD signal handler, so this
-		 * always returns -ECHILD, but the important thing is
-		 * that it blocks. */
-		sys_wait4(pid, NULL, 0, NULL);
+	else {
+		mm_segment_t old_fs;
+		old_fs = get_fs();
+		set_fs(KERNEL_DS);
+		sys_wait4(pid, &sub_info->retval, 0, NULL);
+		set_fs(old_fs);
+	}
 
 	complete(sub_info->complete);
 	return 0;

Re: call_usermodehelper does not report exit status?

[Andrew Morton]

Chris Wright <chrisw@osdl.org> wrote:
>
> Anything wrong with just setting a SIG_DFL handler?

Seems that any time we make a change here it looks fine, tests out fine,
and explodes messily three weeks later.

> W.R.T. the kernel
> pointer, either Andrew's patch which does put_user/__put_user depending
> on context, or some ugly set_fs() should work.  This simplistic approach
> works for me, thoughts?

Yes, set_fs() is better.

Re: call_usermodehelper does not report exit status?

[Chris Wright]

* Andrew Morton (akpm@osdl.org) wrote:
> Chris Wright <chrisw@osdl.org> wrote:
> >
> > Anything wrong with just setting a SIG_DFL handler?
> 
> Seems that any time we make a change here it looks fine, tests out fine,
> and explodes messily three weeks later.

Heh.  Care to try it? ;-)

thanks,
-chris
-- 
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net