From: Paolo 'B. G. <bla...@ya...> - 2006-10-17 21:27:09
|
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 'B. G. <bla...@ya...> - 2006-10-17 21:27:12
|
From: Paolo 'Blaisorblade' Giarrusso <bla...@ya...> 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 <bla...@ya...> --- 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 'B. G. <bla...@ya...> - 2006-10-17 21:27:13
|
From: Paolo 'Blaisorblade' Giarrusso <bla...@ya...> user.h is too generic a header name. I've split out allocation routines from it. Signed-off-by: Paolo 'Blaisorblade' Giarrusso <bla...@ya...> --- 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 <bla...@ya...> + * 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 |
From: Paolo 'B. G. <bla...@ya...> - 2006-10-17 21:27:15
|
From: Paolo 'Blaisorblade' Giarrusso <bla...@ya...> 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 <bla...@ya...> --- 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 'B. G. <bla...@ya...> - 2006-10-17 21:27:19
|
From: Paolo 'Blaisorblade' Giarrusso <bla...@ya...> Fix coding conventions violations is arch/um/os-Linux/helper.c. Signed-off-by: Paolo 'Blaisorblade' Giarrusso <bla...@ya...> --- 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 'B. G. <bla...@ya...> - 2006-10-17 21:27:19
|
From: Paolo 'Blaisorblade' Giarrusso <bla...@ya...> 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 <bla...@ya...> --- 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 'B. G. <bla...@ya...> - 2006-10-17 21:27:21
|
From: Paolo 'Blaisorblade' Giarrusso <bla...@ya...> 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 <bla...@ya...> --- 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 'B. G. <bla...@ya...> - 2006-10-17 21:27:24
|
From: Paolo 'Blaisorblade' Giarrusso <bla...@ya...> 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 <zi...@li...> Cc: kbu...@li... Signed-off-by: Paolo 'Blaisorblade' Giarrusso <bla...@ya...> --- 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 'B. G. <bla...@ya...> - 2006-10-17 21:27:27
|
From: Paolo 'Blaisorblade' Giarrusso <bla...@ya...> 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 <bla...@ya...> --- 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 'B. G. <bla...@ya...> - 2006-10-17 21:27:28
|
From: Paolo 'Blaisorblade' Giarrusso <bla...@ya...> Silence useless warning about undefined symbol in Kconfig. Signed-off-by: Paolo 'Blaisorblade' Giarrusso <bla...@ya...> --- 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 'B. G. <bla...@ya...> - 2006-10-17 21:27:31
|
From: Paolo 'Blaisorblade' Giarrusso <bla...@ya...> 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 <bla...@ya...> --- 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: Jeff D. <jd...@ad...> - 2006-10-18 18:33:46
|
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 |
From: Blaisorblade <bla...@ya...> - 2006-10-21 01:43:04
|
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 |
From: Jeff D. <jd...@ad...> - 2006-10-18 18:38:23
|
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 |
From: Blaisorblade <bla...@ya...> - 2006-10-21 01:16:48
|
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 |
From: Jeff D. <jd...@ad...> - 2006-10-18 18:42:38
|
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 |
From: Blaisorblade <bla...@ya...> - 2006-10-21 00:11:44
|
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 |
From: Jeff D. <jd...@ad...> - 2006-10-25 15:12:18
|
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 |