Some other tested and little UML fixes for 2.6.19 (not all ones are oneliner,
but those ones are tested).
--
Inform me of my mistakes, so I can keep imitating Homer Simpson's "Doh!".
Paolo Giarrusso, aka Blaisorblade
http://www.user-mode-linux.org/~blaisorblade
Chiacchiera con i tuoi amici in tempo reale!
http://it.yahoo.com/mail_it/foot/*http://it.messenger.yahoo.com
From: Paolo 'Blaisorblade' Giarrusso <[email protected]>
I happened to notice that this code is a leftover and it should be removed -
since there are sporadical efforts to revive the PPC port doing such cleanups
is not useless.
Signed-off-by: Paolo 'Blaisorblade' Giarrusso <[email protected]>
---
include/asm-um/archparam-ppc.h | 9 ---------
1 files changed, 0 insertions(+), 9 deletions(-)
diff --git a/include/asm-um/archparam-ppc.h b/include/asm-um/archparam-ppc.h
index 172cd6f..4269d8a 100644
--- a/include/asm-um/archparam-ppc.h
+++ b/include/asm-um/archparam-ppc.h
@@ -1,15 +1,6 @@
#ifndef __UM_ARCHPARAM_PPC_H
#define __UM_ARCHPARAM_PPC_H
-/********* Bits for asm-um/hw_irq.h **********/
-
-struct hw_interrupt_type;
-
-/********* Bits for asm-um/hardirq.h **********/
-
-#define irq_enter(cpu, irq) hardirq_enter(cpu)
-#define irq_exit(cpu, irq) hardirq_exit(cpu)
-
/********* Bits for asm-um/string.h **********/
#define __HAVE_ARCH_STRRCHR
Chiacchiera con i tuoi amici in tempo reale!
http://it.yahoo.com/mail_it/foot/*http://it.messenger.yahoo.com
From: Paolo 'Blaisorblade' Giarrusso <[email protected]>
When enabling the mmapper driver I got warnings because this "const" miscdevice
structure is passed to function as non-const pointer; unlike struct
tty_operations, however, I verified that misc_{de,}register _do_ modify their
parameter, so this const attribute must be removed.
Since the purpose of the change was to guarantee that no lock was needed, add a
comment to prove this differently.
Signed-off-by: Paolo 'Blaisorblade' Giarrusso <[email protected]>
---
arch/um/drivers/mmapper_kern.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/arch/um/drivers/mmapper_kern.c b/arch/um/drivers/mmapper_kern.c
index 9a3b5da..df3516e 100644
--- a/arch/um/drivers/mmapper_kern.c
+++ b/arch/um/drivers/mmapper_kern.c
@@ -95,7 +95,8 @@ static const struct file_operations mmap
.release = mmapper_release,
};
-static const struct miscdevice mmapper_dev = {
+/* No locking needed - only used (and modified) by below initcall and exitcall. */
+static struct miscdevice mmapper_dev = {
.minor = MISC_DYNAMIC_MINOR,
.name = "mmapper",
.fops = &mmapper_fops
Chiacchiera con i tuoi amici in tempo reale!
http://it.yahoo.com/mail_it/foot/*http://it.messenger.yahoo.com
From: Paolo 'Blaisorblade' Giarrusso <[email protected]>
CONFIG_MODE_TT does not work there, the UML_ prefixed version must be used -
this causes a link-time failure when CONFIG_MODE_TT is enabled (i.e. always
here, never by Jeff).
Signed-off-by: Paolo 'Blaisorblade' Giarrusso <[email protected]>
---
arch/um/os-Linux/time.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/arch/um/os-Linux/time.c b/arch/um/os-Linux/time.c
index 38be096..2115b8b 100644
--- a/arch/um/os-Linux/time.c
+++ b/arch/um/os-Linux/time.c
@@ -16,6 +16,7 @@ #include "user.h"
#include "process.h"
#include "kern_constants.h"
#include "os.h"
+#include "uml-config.h"
int set_interval(int is_virtual)
{
@@ -30,7 +31,7 @@ int set_interval(int is_virtual)
return 0;
}
-#ifdef CONFIG_MODE_TT
+#ifdef UML_CONFIG_MODE_TT
void enable_timer(void)
{
set_interval(1);
Chiacchiera con i tuoi amici in tempo reale!
http://it.yahoo.com/mail_it/foot/*http://it.messenger.yahoo.com
From: Paolo 'Blaisorblade' Giarrusso <[email protected]>
Freeing the stack is left uselessly to the caller of run_helper in some cases -
this is taken from run_helper_thread, but here it is useless, so no caller needs
it and the only place where this happens has a potential leak - in case of error
neither run_helper() nor xterm_open() call free_stack().
At this point passing a pointer is not needed - the stack pointer should be passed
directly, but this change is not done here.
Signed-off-by: Paolo 'Blaisorblade' Giarrusso <[email protected]>
---
arch/um/drivers/xterm.c | 2 --
arch/um/os-Linux/helper.c | 7 +++----
2 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/arch/um/drivers/xterm.c b/arch/um/drivers/xterm.c
index 386f8b9..850221d 100644
--- a/arch/um/drivers/xterm.c
+++ b/arch/um/drivers/xterm.c
@@ -136,8 +136,6 @@ int xterm_open(int input, int output, in
return(pid);
}
- if(data->stack == 0) free_stack(stack, 0);
-
if (data->direct_rcv) {
new = os_rcv_fd(fd, &data->helper_pid);
} else {
diff --git a/arch/um/os-Linux/helper.c b/arch/um/os-Linux/helper.c
index e887179..c316dfc 100644
--- a/arch/um/os-Linux/helper.c
+++ b/arch/um/os-Linux/helper.c
@@ -52,7 +52,8 @@ static int helper_child(void *arg)
}
/* Returns either the pid of the child process we run or -E* on failure.
- * XXX The alloc_stack here breaks if this is called in the tracing thread */
+ * XXX The alloc_stack here breaks if this is called in the tracing thread, so
+ * we need to receive a preallocated stack (a local buffer is ok). */
int run_helper(void (*pre_exec)(void *), void *pre_data, char **argv,
unsigned long *stack_out)
{
@@ -118,10 +119,8 @@ out_close:
close(fds[1]);
close(fds[0]);
out_free:
- if (stack_out == NULL)
+ if ((stack_out == NULL) || (*stack_out == 0))
free_stack(stack, 0);
- else
- *stack_out = stack;
return ret;
}
Chiacchiera con i tuoi amici in tempo reale!
http://it.yahoo.com/mail_it/foot/*http://it.messenger.yahoo.com
From: Paolo 'Blaisorblade' Giarrusso <[email protected]>
This should make sure that, for UML, host's configuration files are not
considered, which avoids various pains to the user. Our dependency are such that
the obtained Kconfig will be valid and will lead to successful compilation -
however they cannot prevent an user from disabling any boot device, and if an
option is not set in the read .config (say /boot/config-XXX), with make
menuconfig ARCH=um, it is not set. This always disables UBD and all console I/O
channels, which leads to non-working UML kernels, so this bothers users -
especially now, since it will happen on almost every machine
(/boot/config-`uname -r` exists almost on every machine). It can be workarounded
with make defconfig ARCH=um, but it is non-obvious and can be avoided, so please
_do_ merge this patch.
Given the existence of options, it could be interesting to implement
(additionally) "option required" - with it, Kconfig will refuse reading a
.config file (from wherever it comes) if the given option is not set. With this,
one could mark with it the option characteristic of the given architecture (it
was an old proposal of Roman Zippel, when I pointed out our problem):
config UML
option required
default y
However this should be further discussed:
*) for x86, it must support constructs like:
==arch/i386/Kconfig==
config 64BIT
option required
default n
where Kconfig must require that CONFIG_64BIT is disabled or not present in the
read .config.
*) do we want to do such checks only for the starting defconfig or also for
.config? Which leads to:
*) I may want to port a x86_64 .config to x86 and viceversa, or even among more
different archs. Should that be allowed, and in which measure (the user may
force skipping the check for a .config or it is only given a warning by
default)?
Cc: Roman Zippel <[email protected]>
Cc: [email protected]
Signed-off-by: Paolo 'Blaisorblade' Giarrusso <[email protected]>
---
arch/um/Kconfig | 5 +++++
init/Kconfig | 1 +
2 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/arch/um/Kconfig b/arch/um/Kconfig
index 78fb619..1e068b4 100644
--- a/arch/um/Kconfig
+++ b/arch/um/Kconfig
@@ -1,3 +1,8 @@
+config DEFCONFIG_LIST
+ string
+ option defconfig_list
+ default "arch/$ARCH/defconfig"
+
# UML uses the generic IRQ sugsystem
config GENERIC_HARDIRQS
bool
diff --git a/init/Kconfig b/init/Kconfig
index 1038293..c8b2624 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1,5 +1,6 @@
config DEFCONFIG_LIST
string
+ depends on !UML
option defconfig_list
default "/lib/modules/$UNAME_RELEASE/.config"
default "/etc/kernel-config"
Chiacchiera con i tuoi amici in tempo reale!
http://it.yahoo.com/mail_it/foot/*http://it.messenger.yahoo.com
From: Paolo 'Blaisorblade' Giarrusso <[email protected]>
Fix coding conventions violations is arch/um/os-Linux/helper.c.
Signed-off-by: Paolo 'Blaisorblade' Giarrusso <[email protected]>
---
arch/um/os-Linux/helper.c | 53 ++++++++++++++++++++++++---------------------
1 files changed, 28 insertions(+), 25 deletions(-)
diff --git a/arch/um/os-Linux/helper.c b/arch/um/os-Linux/helper.c
index f72c512..e887179 100644
--- a/arch/um/os-Linux/helper.c
+++ b/arch/um/os-Linux/helper.c
@@ -38,17 +38,17 @@ static int helper_child(void *arg)
char **argv = data->argv;
int errval;
- if(helper_pause){
+ if (helper_pause) {
signal(SIGHUP, helper_hup);
pause();
}
- if(data->pre_exec != NULL)
+ if (data->pre_exec != NULL)
(*data->pre_exec)(data->pre_data);
errval = execvp_noalloc(data->buf, argv[0], argv);
printk("helper_child - execvp of '%s' failed - errno = %d\n", argv[0], -errval);
os_write_file(data->fd, &errval, sizeof(errval));
kill(os_getpid(), SIGKILL);
- return(0);
+ return 0;
}
/* Returns either the pid of the child process we run or -E* on failure.
@@ -60,20 +60,21 @@ int run_helper(void (*pre_exec)(void *),
unsigned long stack, sp;
int pid, fds[2], ret, n;
- if((stack_out != NULL) && (*stack_out != 0))
+ if ((stack_out != NULL) && (*stack_out != 0))
stack = *stack_out;
- else stack = alloc_stack(0, __cant_sleep());
- if(stack == 0)
+ else
+ stack = alloc_stack(0, __cant_sleep());
+ if (stack == 0)
return -ENOMEM;
ret = os_pipe(fds, 1, 0);
- if(ret < 0){
+ if (ret < 0) {
printk("run_helper : pipe failed, ret = %d\n", -ret);
goto out_free;
}
ret = os_set_exec_close(fds[1], 1);
- if(ret < 0){
+ if (ret < 0) {
printk("run_helper : setting FD_CLOEXEC failed, ret = %d\n",
-ret);
goto out_close;
@@ -86,7 +87,7 @@ int run_helper(void (*pre_exec)(void *),
data.fd = fds[1];
data.buf = __cant_sleep() ? um_kmalloc_atomic(PATH_MAX) : um_kmalloc(PATH_MAX);
pid = clone(helper_child, (void *) sp, CLONE_VM | SIGCHLD, &data);
- if(pid < 0){
+ if (pid < 0) {
ret = -errno;
printk("run_helper : clone failed, errno = %d\n", errno);
goto out_free2;
@@ -98,10 +99,10 @@ int run_helper(void (*pre_exec)(void *),
/* Read the errno value from the child, if the exec failed, or get 0 if
* the exec succeeded because the pipe fd was set as close-on-exec. */
n = os_read_file(fds[0], &ret, sizeof(ret));
- if(n == 0)
+ if (n == 0) {
ret = pid;
- else {
- if(n < 0){
+ } else {
+ if (n < 0) {
printk("run_helper : read on pipe failed, ret = %d\n",
-n);
ret = n;
@@ -117,10 +118,11 @@ out_close:
close(fds[1]);
close(fds[0]);
out_free:
- if(stack_out == NULL)
+ if (stack_out == NULL)
free_stack(stack, 0);
- else *stack_out = stack;
- return(ret);
+ else
+ *stack_out = stack;
+ return ret;
}
int run_helper_thread(int (*proc)(void *), void *arg, unsigned int flags,
@@ -130,31 +132,32 @@ int run_helper_thread(int (*proc)(void *
int pid, status, err;
stack = alloc_stack(stack_order, __cant_sleep());
- if(stack == 0) return(-ENOMEM);
+ if (stack == 0)
+ return -ENOMEM;
sp = stack + (page_size() << stack_order) - sizeof(void *);
pid = clone(proc, (void *) sp, flags | SIGCHLD, arg);
- if(pid < 0){
+ if (pid < 0) {
err = -errno;
printk("run_helper_thread : clone failed, errno = %d\n",
errno);
return err;
}
- if(stack_out == NULL){
+ if (stack_out == NULL) {
CATCH_EINTR(pid = waitpid(pid, &status, 0));
- if(pid < 0){
+ if (pid < 0) {
err = -errno;
printk("run_helper_thread - wait failed, errno = %d\n",
errno);
pid = err;
}
- if(!WIFEXITED(status) || (WEXITSTATUS(status) != 0))
+ if (!WIFEXITED(status) || (WEXITSTATUS(status) != 0))
printk("run_helper_thread - thread returned status "
"0x%x\n", status);
free_stack(stack, stack_order);
- }
- else *stack_out = stack;
- return(pid);
+ } else
+ *stack_out = stack;
+ return pid;
}
int helper_wait(int pid)
@@ -162,9 +165,9 @@ int helper_wait(int pid)
int ret;
CATCH_EINTR(ret = waitpid(pid, NULL, WNOHANG));
- if(ret < 0){
+ if (ret < 0) {
ret = -errno;
printk("helper_wait : waitpid failed, errno = %d\n", errno);
}
- return(ret);
+ return ret;
}
Chiacchiera con i tuoi amici in tempo reale!
http://it.yahoo.com/mail_it/foot/*http://it.messenger.yahoo.com
From: Paolo 'Blaisorblade' Giarrusso <[email protected]>
Silence useless warning about undefined symbol in Kconfig.
Signed-off-by: Paolo 'Blaisorblade' Giarrusso <[email protected]>
---
arch/um/Kconfig.char | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/arch/um/Kconfig.char b/arch/um/Kconfig.char
index 62d87b7..e03e40c 100644
--- a/arch/um/Kconfig.char
+++ b/arch/um/Kconfig.char
@@ -190,6 +190,11 @@ config HOSTAUDIO
tristate
default UML_SOUND
+#It is selected elsewhere, so kconfig would warn without this.
+config HW_RANDOM
+ tristate
+ default n
+
config UML_RANDOM
tristate "Hardware random number generator"
help
Chiacchiera con i tuoi amici in tempo reale!
http://it.yahoo.com/mail_it/foot/*http://it.messenger.yahoo.com
From: Paolo 'Blaisorblade' Giarrusso <[email protected]>
Reimplement execvp for our purposes - after we call fork() it is fundamentally
unsafe to use the kernel allocator - current is not valid there. So we simply
pass to our modified execvp() a preallocated buffer. This fixes a real bug and
works very well in testing (I've seen indirectly warning messages from the
forked thread - they went on the pipe connected to its stdout and where read as
a number by UML, when calling read_output(). I verified the obtained number
corresponded to "BUG:").
The added use of __cant_sleep() is not a new bug since __cant_sleep() is already
used in the same function - passing an atomicity parameter would be better but
it would require huge change, stating that this function must not be called in
atomic context and can sleep is a better idea (will make sure of this gradually).
Signed-off-by: Paolo 'Blaisorblade' Giarrusso <[email protected]>
---
arch/um/include/os.h | 2 +
arch/um/os-Linux/Makefile | 10 ++-
arch/um/os-Linux/execvp.c | 149 +++++++++++++++++++++++++++++++++++++++++++++
arch/um/os-Linux/helper.c | 13 +++-
4 files changed, 165 insertions(+), 9 deletions(-)
diff --git a/arch/um/include/os.h b/arch/um/include/os.h
index 6516f6d..13a86bd 100644
--- a/arch/um/include/os.h
+++ b/arch/um/include/os.h
@@ -233,6 +233,8 @@ extern unsigned long __do_user_copy(void
void (*op)(void *to, const void *from,
int n), int *faulted_out);
+/* execvp.c */
+extern int execvp_noalloc(char *buf, const char *file, char *const argv[]);
/* helper.c */
extern int run_helper(void (*pre_exec)(void *), void *pre_data, char **argv,
unsigned long *stack_out);
diff --git a/arch/um/os-Linux/Makefile b/arch/um/os-Linux/Makefile
index b418392..2f8c794 100644
--- a/arch/um/os-Linux/Makefile
+++ b/arch/um/os-Linux/Makefile
@@ -3,8 +3,8 @@ # Copyright (C) 2000 Jeff Dike (jdike@ka
# Licensed under the GPL
#
-obj-y = aio.o elf_aux.o file.o helper.o irq.o main.o mem.o process.o sigio.o \
- signal.o start_up.o time.o trap.o tty.o uaccess.o umid.o tls.o \
+obj-y = aio.o elf_aux.o execvp.o file.o helper.o irq.o main.o mem.o process.o \
+ sigio.o signal.o start_up.o time.o trap.o tty.o uaccess.o umid.o tls.o \
user_syms.o util.o drivers/ sys-$(SUBARCH)/
obj-$(CONFIG_MODE_SKAS) += skas/
@@ -15,9 +15,9 @@ user-objs-$(CONFIG_MODE_TT) += tt.o
obj-$(CONFIG_TTY_LOG) += tty_log.o
user-objs-$(CONFIG_TTY_LOG) += tty_log.o
-USER_OBJS := $(user-objs-y) aio.o elf_aux.o file.o helper.o irq.o main.o mem.o \
- process.o sigio.o signal.o start_up.o time.o trap.o tty.o tls.o \
- uaccess.o umid.o util.o
+USER_OBJS := $(user-objs-y) aio.o elf_aux.o execvp.o file.o helper.o irq.o \
+ main.o mem.o process.o sigio.o signal.o start_up.o time.o trap.o tty.o \
+ tls.o uaccess.o umid.o util.o
CFLAGS_user_syms.o += -DSUBARCH_$(SUBARCH)
diff --git a/arch/um/os-Linux/execvp.c b/arch/um/os-Linux/execvp.c
new file mode 100644
index 0000000..66e583a
--- /dev/null
+++ b/arch/um/os-Linux/execvp.c
@@ -0,0 +1,149 @@
+/* Copyright (C) 2006 by Paolo Giarrusso - modified from glibc' execvp.c.
+ Original copyright notice follows:
+
+ Copyright (C) 1991,92,1995-99,2002,2004 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, write to the Free
+ Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
+ 02111-1307 USA. */
+#include <unistd.h>
+
+#include <stdbool.h>
+#include <stdlib.h>
+#include <string.h>
+#include <errno.h>
+#include <limits.h>
+
+#ifndef TEST
+#include "um_malloc.h"
+#else
+#include <stdio.h>
+#define um_kmalloc malloc
+#endif
+#include "os.h"
+
+/* Execute FILE, searching in the `PATH' environment variable if it contains
+ no slashes, with arguments ARGV and environment from `environ'. */
+int execvp_noalloc(char *buf, const char *file, char *const argv[])
+{
+ if (*file == '\0') {
+ return -ENOENT;
+ }
+
+ if (strchr (file, '/') != NULL) {
+ /* Don't search when it contains a slash. */
+ execv(file, argv);
+ } else {
+ int got_eacces;
+ size_t len, pathlen;
+ char *name, *p;
+ char *path = getenv("PATH");
+ if (path == NULL)
+ path = ":/bin:/usr/bin";
+
+ len = strlen(file) + 1;
+ pathlen = strlen(path);
+ /* Copy the file name at the top. */
+ name = memcpy(buf + pathlen + 1, file, len);
+ /* And add the slash. */
+ *--name = '/';
+
+ got_eacces = 0;
+ p = path;
+ do {
+ char *startp;
+
+ path = p;
+ //Let's avoid this GNU extension.
+ //p = strchrnul (path, ':');
+ p = strchr(path, ':');
+ if (!p)
+ p = strchr(path, '\0');
+
+ if (p == path)
+ /* Two adjacent colons, or a colon at the beginning or the end
+ of `PATH' means to search the current directory. */
+ startp = name + 1;
+ else
+ startp = memcpy(name - (p - path), path, p - path);
+
+ /* Try to execute this name. If it works, execv will not return. */
+ execv(startp, argv);
+
+ /*
+ if (errno == ENOEXEC) {
+ }
+ */
+
+ switch (errno) {
+ case EACCES:
+ /* Record the we got a `Permission denied' error. If we end
+ up finding no executable we can use, we want to diagnose
+ that we did find one but were denied access. */
+ got_eacces = 1;
+ case ENOENT:
+ case ESTALE:
+ case ENOTDIR:
+ /* Those errors indicate the file is missing or not executable
+ by us, in which case we want to just try the next path
+ directory. */
+ case ENODEV:
+ case ETIMEDOUT:
+ /* Some strange filesystems like AFS return even
+ stranger error numbers. They cannot reasonably mean
+ anything else so ignore those, too. */
+ case ENOEXEC:
+ /* We won't go searching for the shell
+ * if it is not executable - the Linux
+ * kernel already handles this enough,
+ * for us. */
+ break;
+
+ default:
+ /* Some other error means we found an executable file, but
+ something went wrong executing it; return the error to our
+ caller. */
+ return -errno;
+ }
+ } while (*p++ != '\0');
+
+ /* We tried every element and none of them worked. */
+ if (got_eacces)
+ /* At least one failure was due to permissions, so report that
+ error. */
+ return -EACCES;
+ }
+
+ /* Return the error from the last attempt (probably ENOENT). */
+ return -errno;
+}
+#ifdef TEST
+int main(int argc, char**argv)
+{
+ char buf[PATH_MAX];
+ int ret;
+ argc--;
+ if (!argc) {
+ fprintf(stderr, "Not enough arguments\n");
+ return 1;
+ }
+ argv++;
+ if (ret = execvp_noalloc(buf, argv[0], argv)) {
+ errno = -ret;
+ perror("execvp_noalloc");
+ }
+ return 0;
+}
+#endif
diff --git a/arch/um/os-Linux/helper.c b/arch/um/os-Linux/helper.c
index cd15b9d..f72c512 100644
--- a/arch/um/os-Linux/helper.c
+++ b/arch/um/os-Linux/helper.c
@@ -8,18 +8,21 @@ #include <stdlib.h>
#include <unistd.h>
#include <errno.h>
#include <sched.h>
+#include <limits.h>
#include <sys/signal.h>
#include <sys/wait.h>
#include "user.h"
#include "kern_util.h"
#include "user_util.h"
#include "os.h"
+#include "um_malloc.h"
struct helper_data {
void (*pre_exec)(void*);
void *pre_data;
char **argv;
int fd;
+ char *buf;
};
/* Debugging aid, changed only from gdb */
@@ -41,9 +44,8 @@ static int helper_child(void *arg)
}
if(data->pre_exec != NULL)
(*data->pre_exec)(data->pre_data);
- execvp(argv[0], argv);
- errval = -errno;
- printk("helper_child - execve of '%s' failed - errno = %d\n", argv[0], errno);
+ errval = execvp_noalloc(data->buf, argv[0], argv);
+ printk("helper_child - execvp of '%s' failed - errno = %d\n", argv[0], -errval);
os_write_file(data->fd, &errval, sizeof(errval));
kill(os_getpid(), SIGKILL);
return(0);
@@ -82,11 +84,12 @@ int run_helper(void (*pre_exec)(void *),
data.pre_data = pre_data;
data.argv = argv;
data.fd = fds[1];
+ data.buf = __cant_sleep() ? um_kmalloc_atomic(PATH_MAX) : um_kmalloc(PATH_MAX);
pid = clone(helper_child, (void *) sp, CLONE_VM | SIGCHLD, &data);
if(pid < 0){
ret = -errno;
printk("run_helper : clone failed, errno = %d\n", errno);
- goto out_close;
+ goto out_free2;
}
close(fds[1]);
@@ -107,6 +110,8 @@ int run_helper(void (*pre_exec)(void *),
CATCH_EINTR(waitpid(pid, NULL, 0));
}
+out_free2:
+ kfree(data.buf);
out_close:
if (fds[1] != -1)
close(fds[1]);
Chiacchiera con i tuoi amici in tempo reale!
http://it.yahoo.com/mail_it/foot/*http://it.messenger.yahoo.com
From: Paolo 'Blaisorblade' Giarrusso <[email protected]>
Fix prototypes in user.h - was needed when including user.h in kernelspace, as
we did in previous patch.
Signed-off-by: Paolo 'Blaisorblade' Giarrusso <[email protected]>
---
arch/um/include/user.h | 9 ++++++---
1 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/arch/um/include/user.h b/arch/um/include/user.h
index acadce3..6921f3e 100644
--- a/arch/um/include/user.h
+++ b/arch/um/include/user.h
@@ -6,6 +6,10 @@
#ifndef __USER_H__
#define __USER_H__
+/* Both on kernelspace and userspace this will provide the size_t definition. It should, at
+ * least. But on userspace it won't hurt surely. */
+#include <linux/types.h>
+
extern void panic(const char *fmt, ...)
__attribute__ ((format (printf, 1, 2)));
extern int printk(const char *fmt, ...)
@@ -13,9 +17,8 @@ extern int printk(const char *fmt, ...)
extern void schedule(void);
extern int in_aton(char *str);
extern int open_gdb_chan(void);
-/* These use size_t, however unsigned long is correct on both i386 and x86_64. */
-extern unsigned long strlcpy(char *, const char *, unsigned long);
-extern unsigned long strlcat(char *, const char *, unsigned long);
+extern size_t strlcpy(char *, const char *, size_t);
+extern size_t strlcat(char *, const char *, size_t);
#endif
Chiacchiera con i tuoi amici in tempo reale!
http://it.yahoo.com/mail_it/foot/*http://it.messenger.yahoo.com
From: Paolo 'Blaisorblade' Giarrusso <[email protected]>
user.h is too generic a header name. I've split out allocation routines from it.
Signed-off-by: Paolo 'Blaisorblade' Giarrusso <[email protected]>
---
arch/um/drivers/cow_sys.h | 1 +
arch/um/drivers/daemon_user.c | 1 +
arch/um/drivers/fd.c | 1 +
arch/um/drivers/mcast_user.c | 1 +
arch/um/drivers/net_user.c | 1 +
arch/um/drivers/pcap_user.c | 1 +
arch/um/drivers/port_user.c | 1 +
arch/um/drivers/pty.c | 1 +
arch/um/drivers/slip_user.c | 1 +
arch/um/drivers/tty.c | 1 +
arch/um/include/um_malloc.h | 17 +++++++++++++++++
arch/um/include/user.h | 6 ------
arch/um/include/user_util.h | 1 -
arch/um/kernel/irq.c | 1 +
arch/um/kernel/process.c | 1 +
arch/um/os-Linux/drivers/ethertap_user.c | 1 +
arch/um/os-Linux/irq.c | 1 +
arch/um/os-Linux/main.c | 1 +
arch/um/os-Linux/sigio.c | 1 +
19 files changed, 33 insertions(+), 7 deletions(-)
diff --git a/arch/um/drivers/cow_sys.h b/arch/um/drivers/cow_sys.h
index 7a5b4af..c6a3084 100644
--- a/arch/um/drivers/cow_sys.h
+++ b/arch/um/drivers/cow_sys.h
@@ -5,6 +5,7 @@ #include "kern_util.h"
#include "user_util.h"
#include "os.h"
#include "user.h"
+#include "um_malloc.h"
static inline void *cow_malloc(int size)
{
diff --git a/arch/um/drivers/daemon_user.c b/arch/um/drivers/daemon_user.c
index 77954ea..310af0f 100644
--- a/arch/um/drivers/daemon_user.c
+++ b/arch/um/drivers/daemon_user.c
@@ -17,6 +17,7 @@ #include "kern_util.h"
#include "user_util.h"
#include "user.h"
#include "os.h"
+#include "um_malloc.h"
#define MAX_PACKET (ETH_MAX_PACKET + ETH_HEADER_OTHER)
diff --git a/arch/um/drivers/fd.c b/arch/um/drivers/fd.c
index 108b7da..218aa0e 100644
--- a/arch/um/drivers/fd.c
+++ b/arch/um/drivers/fd.c
@@ -12,6 +12,7 @@ #include "user.h"
#include "user_util.h"
#include "chan_user.h"
#include "os.h"
+#include "um_malloc.h"
struct fd_chan {
int fd;
diff --git a/arch/um/drivers/mcast_user.c b/arch/um/drivers/mcast_user.c
index 4d2bd39..8138f5e 100644
--- a/arch/um/drivers/mcast_user.c
+++ b/arch/um/drivers/mcast_user.c
@@ -23,6 +23,7 @@ #include "kern_util.h"
#include "user_util.h"
#include "user.h"
#include "os.h"
+#include "um_malloc.h"
#define MAX_PACKET (ETH_MAX_PACKET + ETH_HEADER_OTHER)
diff --git a/arch/um/drivers/net_user.c b/arch/um/drivers/net_user.c
index f3a3f8a..0ffd7ac 100644
--- a/arch/um/drivers/net_user.c
+++ b/arch/um/drivers/net_user.c
@@ -18,6 +18,7 @@ #include "user_util.h"
#include "kern_util.h"
#include "net_user.h"
#include "os.h"
+#include "um_malloc.h"
int tap_open_common(void *dev, char *gate_addr)
{
diff --git a/arch/um/drivers/pcap_user.c b/arch/um/drivers/pcap_user.c
index 2ef641d..11921a7 100644
--- a/arch/um/drivers/pcap_user.c
+++ b/arch/um/drivers/pcap_user.c
@@ -12,6 +12,7 @@ #include <asm/types.h>
#include "net_user.h"
#include "pcap_user.h"
#include "user.h"
+#include "um_malloc.h"
#define MAX_PACKET (ETH_MAX_PACKET + ETH_HEADER_OTHER)
diff --git a/arch/um/drivers/port_user.c b/arch/um/drivers/port_user.c
index f2e8fc4..bc6afaf 100644
--- a/arch/um/drivers/port_user.c
+++ b/arch/um/drivers/port_user.c
@@ -19,6 +19,7 @@ #include "user.h"
#include "chan_user.h"
#include "port.h"
#include "os.h"
+#include "um_malloc.h"
struct port_chan {
int raw;
diff --git a/arch/um/drivers/pty.c b/arch/um/drivers/pty.c
index abec620..829a5ec 100644
--- a/arch/um/drivers/pty.c
+++ b/arch/um/drivers/pty.c
@@ -13,6 +13,7 @@ #include "user.h"
#include "user_util.h"
#include "kern_util.h"
#include "os.h"
+#include "um_malloc.h"
struct pty_chan {
void (*announce)(char *dev_name, int dev);
diff --git a/arch/um/drivers/slip_user.c b/arch/um/drivers/slip_user.c
index 8460285..7eddacc 100644
--- a/arch/um/drivers/slip_user.c
+++ b/arch/um/drivers/slip_user.c
@@ -15,6 +15,7 @@ #include "net_user.h"
#include "slip.h"
#include "slip_common.h"
#include "os.h"
+#include "um_malloc.h"
void slip_user_init(void *data, void *dev)
{
diff --git a/arch/um/drivers/tty.c b/arch/um/drivers/tty.c
index 11de3ac..d95d643 100644
--- a/arch/um/drivers/tty.c
+++ b/arch/um/drivers/tty.c
@@ -11,6 +11,7 @@ #include "chan_user.h"
#include "user_util.h"
#include "user.h"
#include "os.h"
+#include "um_malloc.h"
struct tty_chan {
char *dev;
diff --git a/arch/um/include/um_malloc.h b/arch/um/include/um_malloc.h
new file mode 100644
index 0000000..0363a9b
--- /dev/null
+++ b/arch/um/include/um_malloc.h
@@ -0,0 +1,17 @@
+/*
+ * Copyright (C) 2005 Paolo 'Blaisorblade' Giarrusso <[email protected]>
+ * Licensed under the GPL
+ */
+
+#ifndef __UM_MALLOC_H__
+#define __UM_MALLOC_H__
+
+extern void *um_kmalloc(int size);
+extern void *um_kmalloc_atomic(int size);
+extern void kfree(const void *ptr);
+
+extern void *um_vmalloc(int size);
+extern void *um_vmalloc_atomic(int size);
+extern void vfree(void *ptr);
+
+#endif /* __UM_MALLOC_H__ */
diff --git a/arch/um/include/user.h b/arch/um/include/user.h
index 39f8c88..acadce3 100644
--- a/arch/um/include/user.h
+++ b/arch/um/include/user.h
@@ -11,17 +11,11 @@ extern void panic(const char *fmt, ...)
extern int printk(const char *fmt, ...)
__attribute__ ((format (printf, 1, 2)));
extern void schedule(void);
-extern void *um_kmalloc(int size);
-extern void *um_kmalloc_atomic(int size);
-extern void kfree(void *ptr);
extern int in_aton(char *str);
extern int open_gdb_chan(void);
/* These use size_t, however unsigned long is correct on both i386 and x86_64. */
extern unsigned long strlcpy(char *, const char *, unsigned long);
extern unsigned long strlcat(char *, const char *, unsigned long);
-extern void *um_vmalloc(int size);
-extern void *um_vmalloc_atomic(int size);
-extern void vfree(void *ptr);
#endif
diff --git a/arch/um/include/user_util.h b/arch/um/include/user_util.h
index 802d784..06625fe 100644
--- a/arch/um/include/user_util.h
+++ b/arch/um/include/user_util.h
@@ -52,7 +52,6 @@ extern int linux_main(int argc, char **a
extern void set_cmdline(char *cmd);
extern void input_cb(void (*proc)(void *), void *arg, int arg_len);
extern int get_pty(void);
-extern void *um_kmalloc(int size);
extern int switcheroo(int fd, int prot, void *from, void *to, int size);
extern void do_exec(int old_pid, int new_pid);
extern void tracer_panic(char *msg, ...)
diff --git a/arch/um/kernel/irq.c b/arch/um/kernel/irq.c
index ef25956..5c1e611 100644
--- a/arch/um/kernel/irq.c
+++ b/arch/um/kernel/irq.c
@@ -31,6 +31,7 @@ #include "irq_user.h"
#include "irq_kern.h"
#include "os.h"
#include "sigio.h"
+#include "um_malloc.h"
#include "misc_constants.h"
/*
diff --git a/arch/um/kernel/process.c b/arch/um/kernel/process.c
index fe6c64a..348b272 100644
--- a/arch/um/kernel/process.c
+++ b/arch/um/kernel/process.c
@@ -46,6 +46,7 @@ #include "os.h"
#include "mode.h"
#include "mode_kern.h"
#include "choose-mode.h"
+#include "um_malloc.h"
/* This is a per-cpu array. A processor only modifies its entry and it only
* cares about its entry, so it's OK if another processor is modifying its
diff --git a/arch/um/os-Linux/drivers/ethertap_user.c b/arch/um/os-Linux/drivers/ethertap_user.c
index f559bdf..863981b 100644
--- a/arch/um/os-Linux/drivers/ethertap_user.c
+++ b/arch/um/os-Linux/drivers/ethertap_user.c
@@ -20,6 +20,7 @@ #include "user_util.h"
#include "net_user.h"
#include "etap.h"
#include "os.h"
+#include "um_malloc.h"
#define MAX_PACKET ETH_MAX_PACKET
diff --git a/arch/um/os-Linux/irq.c b/arch/um/os-Linux/irq.c
index a97206d..d46b818 100644
--- a/arch/um/os-Linux/irq.c
+++ b/arch/um/os-Linux/irq.c
@@ -18,6 +18,7 @@ #include "process.h"
#include "sigio.h"
#include "irq_user.h"
#include "os.h"
+#include "um_malloc.h"
static struct pollfd *pollfds = NULL;
static int pollfds_num = 0;
diff --git a/arch/um/os-Linux/main.c b/arch/um/os-Linux/main.c
index d1c5670..685feaa 100644
--- a/arch/um/os-Linux/main.c
+++ b/arch/um/os-Linux/main.c
@@ -23,6 +23,7 @@ #include "mode.h"
#include "choose-mode.h"
#include "uml-config.h"
#include "os.h"
+#include "um_malloc.h"
/* Set in set_stklim, which is called from main and __wrap_malloc.
* __wrap_malloc only calls it if main hasn't started.
diff --git a/arch/um/os-Linux/sigio.c b/arch/um/os-Linux/sigio.c
index f645776..925a652 100644
--- a/arch/um/os-Linux/sigio.c
+++ b/arch/um/os-Linux/sigio.c
@@ -19,6 +19,7 @@ #include "kern_util.h"
#include "user_util.h"
#include "sigio.h"
#include "os.h"
+#include "um_malloc.h"
/* Protected by sigio_lock(), also used by sigio_cleanup, which is an
* exitcall.
Chiacchiera con i tuoi amici in tempo reale!
http://it.yahoo.com/mail_it/foot/*http://it.messenger.yahoo.com
On Tue, Oct 17, 2006 at 11:27:09PM +0200, Paolo 'Blaisorblade' Giarrusso wrote:
> Fix prototypes in user.h - was needed when including user.h in kernelspace,
> as we did in previous patch.
user.h shouldn't be included in kernelspace - its purpose is to
provide kernel prototypes to userspace code.
Jeff
On Tue, Oct 17, 2006 at 11:27:11PM +0200, Paolo 'Blaisorblade' Giarrusso wrote:
> Reimplement execvp for our purposes - after we call fork() it is
> fundamentally unsafe to use the kernel allocator - current is not valid
> there.
This is horriby ugly. Can we instead do something different like
check out the paths of helpers at early boot, before the kernel is
running, save them, and simply execve them later?
At that point, something like running "which foo" would be fine by me.
Jeff
On Tue, Oct 17, 2006 at 11:19:43PM +0200, Paolo 'Blaisorblade' Giarrusso wrote:
> Some other tested and little UML fixes for 2.6.19 (not all ones are oneliner,
> but those ones are tested).
These are acked, except as noted. I don't like uml-fix-prototypes.patch
or uml-make-execvp-safe-for-our-usage.patch - don't kick them out of
-mm, but don't send them to Linus as yet.
Jeff
On Wednesday 18 October 2006 20:37, Jeff Dike wrote:
> On Tue, Oct 17, 2006 at 11:27:11PM +0200, Paolo 'Blaisorblade' Giarrusso
wrote:
> > Reimplement execvp for our purposes - after we call fork() it is
> > fundamentally unsafe to use the kernel allocator - current is not valid
> > there.
>
> This is horriby ugly.
Detail why. The code of execvp()? Passing in the buffer?
I'm not saying it's the brightest code around here, but it's ok for me.
> Can we instead do something different like
> check out the paths of helpers at early boot, before the kernel is
> running, save them, and simply execve them later?
I initially thought to design a two-steps API with a "which" operation (where
memory allocation was used) to call later execvp(); when I saw the glibc
implementation (it allocates one single fixed-size buffer) I saw it was
simpler this way.
Additionally, error handling cannot be done properly without trying an exec -
I think it is also ok to drop this execvp semantic, so that if the first
binary found in path is marked executable but has the wrong binary format the
whole thing just does not start.
The current implementation already diverges from glibc - it never calls
directly the shell passing a script, because IMHO execve() will care for that
(and testing confirmed this IIRC).
I'd not do that at boot, but just before the fork()+execve() - it is
conceivable that a given user will install a support binary after booting
UML.
I must say that I've seen files without the shebang working ok (if having the
executable bit set) when executed from the shell, and I've had the doubt
execvp() would handle that.
> At that point, something like running "which foo" would be fine by me.
--
Inform me of my mistakes, so I can keep imitating Homer Simpson's "Doh!".
Paolo Giarrusso, aka Blaisorblade
http://www.user-mode-linux.org/~blaisorblade
Chiacchiera con i tuoi amici in tempo reale!
http://it.yahoo.com/mail_it/foot/*http://it.messenger.yahoo.com
On Wednesday 18 October 2006 20:37, Jeff Dike wrote:
> On Tue, Oct 17, 2006 at 11:27:11PM +0200, Paolo 'Blaisorblade' Giarrusso
wrote:
> > Reimplement execvp for our purposes - after we call fork() it is
> > fundamentally unsafe to use the kernel allocator - current is not valid
> > there.
>
> This is horriby ugly. Can we instead do something different like
> check out the paths of helpers at early boot, before the kernel is
> running, save them, and simply execve them later?
>
> At that point, something like running "which foo" would be fine by me.
I'd add that this can IMHO cause hard-to-diagnose crashes (I've seen strange
behaviours in debug mode, and even schedule-while-atomic warnings, maybe
because the creator thread had gone in atomic mode), and since this is a
working fix, either this or a replacement should go in.
--
Inform me of my mistakes, so I can keep imitating Homer Simpson's "Doh!".
Paolo Giarrusso, aka Blaisorblade
http://www.user-mode-linux.org/~blaisorblade
Chiacchiera con i tuoi amici in tempo reale!
http://it.yahoo.com/mail_it/foot/*http://it.messenger.yahoo.com
On Wednesday 18 October 2006 20:32, Jeff Dike wrote:
> On Tue, Oct 17, 2006 at 11:27:09PM +0200, Paolo 'Blaisorblade' Giarrusso
wrote:
> > Fix prototypes in user.h - was needed when including user.h in
> > kernelspace, as we did in previous patch.
>
> user.h shouldn't be included in kernelspace - its purpose is to
> provide kernel prototypes to userspace code.
Actually I now think (and just verified) I do not even include it any more in
kernelspace - that comment is outdated. Having a more correct prototype is
anyway nicer, so that patch can go in with a rewritten changelog (but that's
a taste matter). I'm rewriting it anyway.
--
Inform me of my mistakes, so I can keep imitating Homer Simpson's "Doh!".
Paolo Giarrusso, aka Blaisorblade
http://www.user-mode-linux.org/~blaisorblade
Chiacchiera con i tuoi amici in tempo reale!
http://it.yahoo.com/mail_it/foot/*http://it.messenger.yahoo.com
On Sat, Oct 21, 2006 at 02:11:28AM +0200, Blaisorblade wrote:
> > This is horriby ugly.
>
> Detail why. The code of execvp()? Passing in the buffer?
> I'm not saying it's the brightest code around here, but it's ok for me.
My initial reaction was mostly due to the look of the code, which is
fixable. I also don't like carrying around bits of libc (although we
do have setjmp/longjmp, but that's a special case). However, it's
unlikely that it will need much maintenance, so this is more a taste
thing as well.
> I initially thought to design a two-steps API with a "which" operation (where
> memory allocation was used) to call later execvp(); when I saw the glibc
> implementation (it allocates one single fixed-size buffer) I saw it was
> simpler this way.
I think I still like the two-stage thing better. If the 'which' part
finds something that doesn't exec, then we can just spit out a nice error.
> I'd not do that at boot, but just before the fork()+execve() - it is
> conceivable that a given user will install a support binary after booting
> UML.
I was envisioning it being part of bootup, but doing it just before
the exec would be OK, too.
Jeff