* Re: [PATCH] revert- sys_setaltroot [not found] <200410261928.i9QJS7h3011015@hera.kernel.org> @ 2004-12-22 10:18 ` David Woodhouse 2004-12-22 11:03 ` Andrew Morton 0 siblings, 1 reply; 6+ messages in thread From: David Woodhouse @ 2004-12-22 10:18 UTC (permalink / raw) To: Linux Kernel Mailing List; +Cc: akpm, alan [-- Attachment #1: Type: text/plain, Size: 1174 bytes --] On Tue, 2004-10-26 at 15:22 +0000, Linux Kernel Mailing List wrote: > ChangeSet 1.2187, 2004/10/26 08:22:01-07:00, akpm@osdl.org > > [PATCH] revert- sys_setaltroot > We decided to do this a different way. Can you elaborate? We don't have open_exec() in userspace, so I'm not sure it's possible to do this properly for setuid binaries and execute-only binaries in userspace without being seriously exploitable. I assume the problem with it was that you can set the altroot to a directory containing a bogus /etc/shadow and then 'su' with a password of your own choosing, or something like that? There's no reason to make the user-specified altroot survive across exec() though -- and as long it _doesn't_ persist, it should be perfectly safe to let users set it. I'm working on a patch which puts sys_setaltroot back but discards it on exec. This allows me to ditch all the horrid path walking stuff from qemu, and as soon as I make qemu understand AT_EXECFD it should let me run execute-only i386 binaries on my ppc box too. T'would be useful to confirm the original objections to it and that I haven't missed a viable 'different way' to do it, though. -- dwmw2 [-- Attachment #2: altroot-3.patch --] [-- Type: text/x-patch, Size: 12759 bytes --] ===== arch/i386/kernel/entry.S 1.88 vs edited ===== --- 1.88/arch/i386/kernel/entry.S 2004-12-12 18:37:55 +00:00 +++ edited/arch/i386/kernel/entry.S 2004-12-20 13:08:02 +00:00 @@ -861,7 +861,7 @@ .long sys_mq_getsetattr .long sys_ni_syscall /* reserved for kexec */ .long sys_waitid - .long sys_ni_syscall /* 285 */ /* available */ + .long sys_setaltroot /* 285 */ .long sys_add_key .long sys_request_key .long sys_keyctl ===== arch/ia64/kernel/entry.S 1.69 vs edited ===== --- 1.69/arch/ia64/kernel/entry.S 2004-11-23 20:18:30 +00:00 +++ edited/arch/ia64/kernel/entry.S 2004-12-20 13:08:02 +00:00 @@ -1531,7 +1531,7 @@ data8 sys_add_key data8 sys_request_key data8 sys_keyctl - data8 sys_ni_syscall + data8 sys_setaltroot data8 sys_ni_syscall // 1275 data8 sys_ni_syscall data8 sys_ni_syscall ===== arch/ppc/kernel/misc.S 1.63 vs edited ===== --- 1.63/arch/ppc/kernel/misc.S 2004-10-22 10:27:40 +01:00 +++ edited/arch/ppc/kernel/misc.S 2004-12-20 13:08:02 +00:00 @@ -1450,3 +1450,4 @@ .long sys_add_key .long sys_request_key /* 270 */ .long sys_keyctl + .long sys_setaltroot ===== arch/ppc64/kernel/misc.S 1.93 vs edited ===== --- 1.93/arch/ppc64/kernel/misc.S 2004-11-11 08:23:03 +00:00 +++ edited/arch/ppc64/kernel/misc.S 2004-12-20 13:08:02 +00:00 @@ -966,7 +966,8 @@ .llong .sys32_add_key .llong .sys32_request_key .llong .compat_sys_keyctl - + .llong .sys_setaltroot + .balign 8 _GLOBAL(sys_call_table) .llong .sys_restart_syscall /* 0 */ @@ -1241,3 +1242,4 @@ .llong .sys_add_key .llong .sys_request_key /* 270 */ .llong .sys_keyctl + .llong .sys_setaltroot ===== arch/sparc/kernel/systbls.S 1.29 vs edited ===== --- 1.29/arch/sparc/kernel/systbls.S 2004-11-08 02:08:03 +00:00 +++ edited/arch/sparc/kernel/systbls.S 2004-12-20 13:08:02 +00:00 @@ -75,7 +75,7 @@ /*265*/ .long sys_timer_delete, sys_timer_create, sys_nis_syscall, sys_io_setup, sys_io_destroy /*270*/ .long sys_io_submit, sys_io_cancel, sys_io_getevents, sys_mq_open, sys_mq_unlink /*275*/ .long sys_mq_timedsend, sys_mq_timedreceive, sys_mq_notify, sys_mq_getsetattr, sys_waitid -/*280*/ .long sys_ni_syscall, sys_add_key, sys_request_key, sys_keyctl +/*280*/ .long sys_setaltroot, sys_add_key, sys_request_key, sys_keyctl #ifdef CONFIG_SUNOS_EMUL /* Now the SunOS syscall table. */ ===== arch/sparc64/kernel/systbls.S 1.66 vs edited ===== --- 1.66/arch/sparc64/kernel/systbls.S 2004-11-11 08:23:03 +00:00 +++ edited/arch/sparc64/kernel/systbls.S 2004-12-20 13:08:02 +00:00 @@ -76,7 +76,7 @@ .word sys_timer_delete, sys32_timer_create, sys_ni_syscall, compat_sys_io_setup, sys_io_destroy /*270*/ .word sys32_io_submit, sys_io_cancel, compat_sys_io_getevents, sys32_mq_open, sys_mq_unlink .word sys_mq_timedsend, sys_mq_timedreceive, compat_sys_mq_notify, compat_sys_mq_getsetattr, compat_sys_waitid -/*280*/ .word sys_ni_syscall, sys_add_key, sys_request_key, sys_keyctl +/*280*/ .word sys_setaltroot, sys_add_key, sys_request_key, sys_keyctl #endif /* CONFIG_COMPAT */ @@ -142,7 +142,7 @@ .word sys_timer_delete, sys_timer_create, sys_ni_syscall, sys_io_setup, sys_io_destroy /*270*/ .word sys_io_submit, sys_io_cancel, sys_io_getevents, sys_mq_open, sys_mq_unlink .word sys_mq_timedsend, sys_mq_timedreceive, sys_mq_notify, sys_mq_getsetattr, sys_waitid -/*280*/ .word sys_nis_syscall, sys_add_key, sys_request_key, sys_keyctl +/*280*/ .word sys_setaltroot, sys_add_key, sys_request_key, sys_keyctl #if defined(CONFIG_SUNOS_EMUL) || defined(CONFIG_SOLARIS_EMUL) || \ defined(CONFIG_SOLARIS_EMUL_MODULE) ===== fs/binfmt_elf.c 1.92 vs edited ===== --- 1.92/fs/binfmt_elf.c 2004-11-16 19:01:21 +00:00 +++ edited/fs/binfmt_elf.c 2004-12-20 13:08:02 +00:00 @@ -37,6 +37,7 @@ #include <linux/pagemap.h> #include <linux/security.h> #include <linux/syscalls.h> +#include <linux/namei.h> #include <asm/uaccess.h> #include <asm/param.h> @@ -626,7 +627,7 @@ */ SET_PERSONALITY(loc->elf_ex, ibcs2_interpreter); - interpreter = open_exec(elf_interpreter); + interpreter = open_exec(elf_interpreter, LOOKUP_NOALT); retval = PTR_ERR(interpreter); if (IS_ERR(interpreter)) goto out_free_interp; ===== fs/binfmt_em86.c 1.8 vs edited ===== --- 1.8/fs/binfmt_em86.c 2004-05-10 12:25:55 +01:00 +++ edited/fs/binfmt_em86.c 2004-12-20 13:08:02 +00:00 @@ -19,6 +19,7 @@ #include <linux/fs.h> #include <linux/file.h> #include <linux/errno.h> +#include <linux/namei.h> #define EM86_INTERP "/usr/bin/em86" @@ -82,7 +83,7 @@ * Note that we use open_exec() as the name is now in kernel * space, and we don't need to copy it. */ - file = open_exec(interp); + file = open_exec(interp, LOOKUP_NOALT); if (IS_ERR(file)) return PTR_ERR(file); ===== fs/binfmt_flat.c 1.11 vs edited ===== --- 1.11/fs/binfmt_flat.c 2004-10-19 06:26:36 +01:00 +++ edited/fs/binfmt_flat.c 2004-12-20 13:08:02 +00:00 @@ -36,6 +36,7 @@ #include <linux/personality.h> #include <linux/init.h> #include <linux/flat.h> +#include <linux/namei.h> #include <asm/byteorder.h> #include <asm/system.h> @@ -774,7 +775,7 @@ /* Open the file up */ bprm.filename = buf; - bprm.file = open_exec(bprm.filename); + bprm.file = open_exec(bprm.filename, LOOKUP_NOALT); res = PTR_ERR(bprm.file); if (IS_ERR(bprm.file)) return res; ===== fs/binfmt_misc.c 1.30 vs edited ===== --- 1.30/fs/binfmt_misc.c 2004-06-29 15:43:10 +01:00 +++ edited/fs/binfmt_misc.c 2004-12-20 13:08:02 +00:00 @@ -179,7 +179,7 @@ bprm->interp = iname; /* for binfmt_script */ - interp_file = open_exec (iname); + interp_file = open_exec (iname, LOOKUP_NOALT); retval = PTR_ERR (interp_file); if (IS_ERR (interp_file)) goto _error; ===== fs/binfmt_script.c 1.9 vs edited ===== --- 1.9/fs/binfmt_script.c 2004-06-29 22:35:34 +01:00 +++ edited/fs/binfmt_script.c 2004-12-20 13:08:03 +00:00 @@ -85,9 +85,14 @@ /* * OK, now restart the process with the interpreter's dentry. */ - file = open_exec(interp); + file = open_exec(interp, 0); if (IS_ERR(file)) return PTR_ERR(file); + + /* If we had an altroot (which may be untrusted) we can reset it + to the default for the personality now that we've got the + script's interpreter itself open. */ + set_fs_altroot(); bprm->file = file; retval = prepare_binprm(bprm); ===== fs/compat.c 1.47 vs edited ===== --- 1.47/fs/compat.c 2004-12-10 17:57:46 +00:00 +++ edited/fs/compat.c 2004-12-20 13:08:03 +00:00 @@ -1393,7 +1393,7 @@ goto out_ret; memset(bprm, 0, sizeof(*bprm)); - file = open_exec(filename); + file = open_exec(filename, 0); retval = PTR_ERR(file); if (IS_ERR(file)) goto out_kfree; @@ -1447,6 +1447,7 @@ free_arg_pages(bprm); /* execve success */ + set_fs_altroot(); security_bprm_free(bprm); kfree(bprm); return retval; ===== fs/exec.c 1.146 vs edited ===== --- 1.146/fs/exec.c 2004-12-10 17:57:46 +00:00 +++ edited/fs/exec.c 2004-12-20 13:08:03 +00:00 @@ -473,14 +473,14 @@ #endif /* CONFIG_MMU */ -struct file *open_exec(const char *name) +struct file *open_exec(const char *name, int flags) { struct nameidata nd; int err; struct file *file; nd.intent.open.flags = FMODE_READ; - err = path_lookup(name, LOOKUP_FOLLOW|LOOKUP_OPEN, &nd); + err = path_lookup(name, LOOKUP_FOLLOW|LOOKUP_OPEN|flags, &nd); file = ERR_PTR(err); if (!err) { @@ -1007,7 +1007,7 @@ loader = PAGE_SIZE*MAX_ARG_PAGES-sizeof(void *); - file = open_exec("/sbin/loader"); + file = open_exec("/sbin/loader", LOOKUP_NOALT); retval = PTR_ERR(file); if (IS_ERR(file)) return retval; @@ -1100,7 +1100,7 @@ goto out_ret; memset(bprm, 0, sizeof(*bprm)); - file = open_exec(filename); + file = open_exec(filename, 0); retval = PTR_ERR(file); if (IS_ERR(file)) goto out_kfree; @@ -1155,6 +1155,7 @@ free_arg_pages(bprm); /* execve success */ + set_fs_altroot(); security_bprm_free(bprm); kfree(bprm); return retval; ===== fs/namei.c 1.115 vs edited ===== --- 1.115/fs/namei.c 2004-10-28 08:39:57 +01:00 +++ edited/fs/namei.c 2004-12-20 13:08:03 +00:00 @@ -913,20 +913,20 @@ return 1; } -void set_fs_altroot(void) +int __set_fs_altroot(const char *altroot) { - char *emul = __emul_prefix(); struct nameidata nd; struct vfsmount *mnt = NULL, *oldmnt; struct dentry *dentry = NULL, *olddentry; int err; - - if (!emul) + if (!altroot) goto set_it; - err = path_lookup(emul, LOOKUP_FOLLOW|LOOKUP_DIRECTORY|LOOKUP_NOALT, &nd); + err = path_lookup(altroot, LOOKUP_FOLLOW|LOOKUP_DIRECTORY|LOOKUP_NOALT, &nd); if (!err) { mnt = nd.mnt; dentry = nd.dentry; + } else { + return err; } set_it: write_lock(¤t->fs->lock); @@ -939,6 +939,58 @@ dput(olddentry); mntput(oldmnt); } + return 0; +} + +void set_fs_altroot(void) +{ + char *emul = __emul_prefix(); + + __set_fs_altroot(emul); +} + +asmlinkage long sys_setaltroot(const char __user * altroot) +{ + char *emul = NULL; + int ret; + + if (altroot) { + emul = getname(altroot); + if (IS_ERR(emul)) { + ret = PTR_ERR(emul); + goto out; + } + } + + if (atomic_read(¤t->fs->count) != 1) { + struct fs_struct *fsp, *ofsp; + + fsp = copy_fs_struct(current->fs); + if (fsp == NULL) { + ret = -ENOMEM; + goto out_putname; + } + + task_lock(current); + ofsp = current->fs; + current->fs = fsp; + task_unlock(current); + + put_fs_struct(ofsp); + } + + /* + * At that point we are guaranteed to be the sole owner of + * current->fs. + */ + + ret = __set_fs_altroot(emul); + +out_putname: + if (emul) + putname(emul); +out: + return ret; } int fastcall path_lookup(const char *name, unsigned int flags, struct nameidata *nd) ===== include/asm-i386/unistd.h 1.43 vs edited ===== --- 1.43/include/asm-i386/unistd.h 2004-10-24 11:32:46 +01:00 +++ edited/include/asm-i386/unistd.h 2004-12-20 13:08:41 +00:00 @@ -290,7 +290,7 @@ #define __NR_mq_getsetattr (__NR_mq_open+5) #define __NR_sys_kexec_load 283 #define __NR_waitid 284 -/* #define __NR_sys_setaltroot 285 */ +#define __NR_setaltroot 285 #define __NR_add_key 286 #define __NR_request_key 287 #define __NR_keyctl 288 ===== include/asm-ia64/unistd.h 1.53 vs edited ===== --- 1.53/include/asm-ia64/unistd.h 2004-11-23 20:18:59 +00:00 +++ edited/include/asm-ia64/unistd.h 2004-12-20 13:08:03 +00:00 @@ -263,6 +263,7 @@ #define __NR_add_key 1271 #define __NR_request_key 1272 #define __NR_keyctl 1273 +#define __NR_setaltroot 1274 #ifdef __KERNEL__ ===== include/asm-ppc/unistd.h 1.34 vs edited ===== --- 1.34/include/asm-ppc/unistd.h 2004-10-22 10:27:40 +01:00 +++ edited/include/asm-ppc/unistd.h 2004-12-20 13:08:03 +00:00 @@ -276,8 +276,9 @@ #define __NR_add_key 269 #define __NR_request_key 270 #define __NR_keyctl 271 +#define __NR_setaltroot 272 -#define __NR_syscalls 272 +#define __NR_syscalls 273 #define __NR(n) #n ===== include/asm-ppc64/unistd.h 1.36 vs edited ===== --- 1.36/include/asm-ppc64/unistd.h 2004-11-16 03:29:08 +00:00 +++ edited/include/asm-ppc64/unistd.h 2004-12-20 13:08:03 +00:00 @@ -282,8 +282,9 @@ #define __NR_add_key 269 #define __NR_request_key 270 #define __NR_keyctl 271 +#define __NR_setaltroot 272 -#define __NR_syscalls 272 +#define __NR_syscalls 273 #ifdef __KERNEL__ #define NR_syscalls __NR_syscalls #endif ===== include/asm-sparc64/unistd.h 1.36 vs edited ===== --- 1.36/include/asm-sparc64/unistd.h 2004-10-24 10:47:46 +01:00 +++ edited/include/asm-sparc64/unistd.h 2004-12-20 13:09:06 +00:00 @@ -298,7 +298,7 @@ #define __NR_mq_notify 277 #define __NR_mq_getsetattr 278 #define __NR_waitid 279 -/*#define __NR_sys_setaltroot 280 available (was setaltroot) */ +#define __NR_setaltroot 280 #define __NR_add_key 281 #define __NR_request_key 282 #define __NR_keyctl 283 ===== include/linux/fs.h 1.362 vs edited ===== --- 1.362/include/linux/fs.h 2004-10-29 09:14:03 +01:00 +++ edited/include/linux/fs.h 2004-12-20 13:08:03 +00:00 @@ -1376,7 +1376,7 @@ extern int may_open(struct nameidata *, int, int); extern int kernel_read(struct file *, unsigned long, char *, unsigned long); -extern struct file * open_exec(const char *); +extern struct file * open_exec(const char *, int); /* fs/dcache.c -- generic fs support functions */ extern int is_subdir(struct dentry *, struct dentry *); ===== include/linux/syscalls.h 1.17 vs edited ===== --- 1.17/include/linux/syscalls.h 2004-10-24 10:47:46 +01:00 +++ edited/include/linux/syscalls.h 2004-12-20 13:08:03 +00:00 @@ -490,6 +490,7 @@ void __user *res); asmlinkage long sys_syslog(int type, char __user *buf, int len); asmlinkage long sys_uselib(const char __user *library); +asmlinkage long sys_setaltroot(const char __user *altroot); asmlinkage long sys_ni_syscall(void); asmlinkage long sys_add_key(const char __user *_type, ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] revert- sys_setaltroot 2004-12-22 10:18 ` [PATCH] revert- sys_setaltroot David Woodhouse @ 2004-12-22 11:03 ` Andrew Morton 2004-12-22 11:45 ` David Woodhouse 2005-01-17 13:35 ` David Woodhouse 0 siblings, 2 replies; 6+ messages in thread From: Andrew Morton @ 2004-12-22 11:03 UTC (permalink / raw) To: David Woodhouse; +Cc: linux-kernel, alan David Woodhouse <dwmw2@infradead.org> wrote: > > On Tue, 2004-10-26 at 15:22 +0000, Linux Kernel Mailing List wrote: > > ChangeSet 1.2187, 2004/10/26 08:22:01-07:00, akpm@osdl.org > > > > [PATCH] revert- sys_setaltroot > > We decided to do this a different way. > > Can you elaborate? There were security problems and suggestions that a namespace-based approach would be better. umm, have a random sprinkle of emails: Begin forwarded message: Date: Tue, 19 Oct 2004 13:14:55 -0700 From: "Seth, Rohit" <rohit.seth@intel.com> To: "Andrew Morton" <akpm@osdl.org> Cc: <torvalds@osdl.org> Subject: RE: FW: [PATCH] Support ia32 exec domains without CONFIG_IA32_SUPPORT Seth, Rohit <> wrote on Tuesday, October 19, 2004 10:14 AM: > Andrew Morton <mailto:akpm@osdl.org> wrote on Tuesday, October 19, > 2004 12:35 AM: > >> "Seth, Rohit" <rohit.seth@intel.com> wrote: >>> >>> [sys-altroot4.patch application/octet-stream (7320 bytes)] >> >> hm. Should sys_setaltroot() really not require any privileges? >> > > As the extent of change is limited to current execution thread (just > like set_personality) so it is fine to not require any privileges. > I need to take this back. There is indeed a security hole that is introduced by this new system call. And that is mainly coming because of the fact that (unlike set_personality) alternate root is maintained across execs (and also the fact that there is a random string that can become the alternate root for a user). So any program can exec a suid program and then use the alternate root to exploit system security. I will be sending you the patch to fix this issue soon. Thanks, rohit >> Not that I can see any problems with it, but it needs >> consideration... Begin forwarded message: Date: Mon, 25 Oct 2004 19:14:05 -0700 From: "Seth, Rohit" <rohit.seth@intel.com> To: "Andrew Morton" <akpm@osdl.org>, <torvalds@osdl.org>, <viro@parcelfarce.linux.theplanet.co.uk> Subject: RE: FW: [PATCH] Support ia32 exec domains without CONFIG_IA32_SUPPORT Andrew Morton <mailto:akpm@osdl.org> wrote on Monday, October 25, 2004 12:31 PM: > "Seth, Rohit" <rohit.seth@intel.com> wrote: >> >> altroot() > > Do we have a decision/direction on this? I have a > revert-sys_setaltroot() > patch lined up and my trigger finger is itchy! I know there is already enough said about altroot and the need to restrict its usage. Because of the critical need having another stab at it. Would it make sense to have different semantics of the new system call (call it sys_setexecdomain). Here super-user makes a new system call to register a new exec_domain. And as part of this system call, a new data structure, something that specify a mapping from personality to alternate root. Whenever any app makes a new setpersonality system call, if that correspond to one of these specified then alternate root takes into affect....much on the same lines that is happening in kernel today. Sys_setaltroot, I agree, in its current form should be reverted. Please let me know what you think about this. Thanks, rohit Begin forwarded message: Date: Tue, 19 Oct 2004 15:32:57 -0700 (PDT) From: Linus Torvalds <torvalds@osdl.org> To: "Seth, Rohit" <rohit.seth@intel.com>, Al Viro <viro@parcelfarce.linux.theplanet.co.uk> Cc: Andrew Morton <akpm@osdl.org>, "David S. Miller" <davem@davemloft.net> Subject: RE: FW: [PATCH] Support ia32 exec domains without CONFIG_IA32_SUPPORT Hmm. Do we actually want to clear the alternate root? It looks like that would make altroot useless. Who uses it? I thought it was only really used for the emulation environments on sparc64 etc. And clearly now ia64. But not saving it across an exec would make it useless, no? Linus On Tue, 19 Oct 2004, Seth, Rohit wrote: > Seth, Rohit <> wrote on Tuesday, October 19, 2004 1:15 PM: > > I need to take this back. There is indeed a security hole that is > > introduced by this new system call. And that is mainly coming > > because of the fact that (unlike set_personality) alternate root is > > maintained across execs (and also the fact that there is a random > > string that can become the alternate root for a user). So any > > program can exec a suid program and then use the alternate root to > > exploit system security. > > > > I will be sending you the patch to fix this issue soon. > > > Andrew, > > Please find attached a small patch that fixes the above security issue > with set_altroot system call. > > Thanks, rohit > > ---- Previously, altroot was cleared only on crossing exec-domain boundaries. Specifically, it was not cleared on exec. This has security implications. This patch resolves the issue by clearing the altroot on exec. Signed-off-by: Arun Sharma <arun.sharma@intel.com> Signed-off-by: Tony Luck <tony.luck@intel.com> Signed-off-by: Rohit Seth <rohit.seth@intel.com> Index: linux-2.6.8/fs/exec.c =================================================================== --- linux-2.6.8.orig/fs/exec.c 2004-08-13 22:36:56.000000000 -0700 +++ linux-2.6.8/fs/exec.c 2004-10-19 14:34:25.805323416 -0700 @@ -786,6 +786,14 @@ spin_unlock(&files->file_lock); } +static inline void clear_altroot(void) +{ + dput(current->fs->altroot); + mntput(current->fs->altrootmnt); + current->fs->altrootmnt = NULL; + current->fs->altroot = NULL; +} + int flush_old_exec(struct linux_binprm * bprm) { char * name; @@ -821,6 +829,8 @@ /* This is the point of no return */ steal_locks(files); put_files_struct(files); + if (current->fs->altroot) + clear_altroot(); current->sas_ss_sp = current->sas_ss_size = 0; Begin forwarded message: Date: Mon, 25 Oct 2004 19:46:30 -0700 (PDT) From: Linus Torvalds <torvalds@osdl.org> To: Andrew Morton <akpm@osdl.org> Cc: "Seth, Rohit" <rohit.seth@intel.com>, viro@parcelfarce.linux.theplanet.co.uk Subject: Re: FW: [PATCH] Support ia32 exec domains without CONFIG_IA32_SUPPORT On Mon, 25 Oct 2004, Andrew Morton wrote: > > This is an area in which I have never worked and I don't feel that I'm in a > position to offer advice - we need to keep poking Linus & Al. I think it's ok to just revert sys_altroot() for now. The VFS-internal altroot usage doesn't seem to be any different from sparc64, so.. > But from a high level one does ask "if the namespace stuff cannot satisfy > your requirement, then is it broken?". namespaces are about private mounts, not directly about emulation. altroot definitely _is_ directly about emulation, and I suspect we could do away with altroot if we had some kind of "per-personality namespace", which would switch together with the emulated environment. The reason I'd like to get rid of altroot (and would still prefer something namespace-based) is simply that altroot has these really nasty conceptual problems with dual lookups etc. Dammit, but it's an ugly wart. Linus Begin forwarded message: Date: Fri, 22 Oct 2004 17:28:35 -0700 (PDT) From: Linus Torvalds <torvalds@osdl.org> To: "Seth, Rohit" <rohit.seth@intel.com> Cc: Andrew Morton <akpm@osdl.org>, Al Viro <viro@parcelfarce.linux.theplanet.co.uk> Subject: RE: FW: [PATCH] Support ia32 exec domains without CONFIG_IA32_SUPPORT On Fri, 22 Oct 2004, Seth, Rohit wrote: > > Could you please let me know how we should proceed with this issue so > that emulators can seemlessly support different execution domains. Well, my _personal_ preference by far is to use the equivalent of "chroot" for the emulated environment. So whenever you hit a x86 binary, you just chroot to /emul, and then you run it there. NOTE! I say "equivalent", because you could do it by creating a separate namespace instead, if you wanted to - and just overmounting /lib and /usr/lib with the emulation environment versions in that particular namespace. Also, note the bind mounts: if you do end up having "/emul", you want to have the local /lib and /usr/lib be there, but the rest would be just bind mounts back to the original root, making things pretty similar (ie people would still see all their normal files in their home directories etc, it would be just /lib that effectively gets swapped out). I agree that the altroot() thing is very _convenient_, but it's always been an ugly wart on the VFS layer, and so I'd rather get rid of it if we can, rather than add users. Al may have more intelligent suggestions. Linus Begin forwarded message: Date: Wed, 20 Oct 2004 23:07:57 -0700 (PDT) From: Linus Torvalds <torvalds@osdl.org> To: "Seth, Rohit" <rohit.seth@intel.com> Cc: viro@parcelfarce.linux.theplanet.co.uk, Andrew Morton <akpm@osdl.org>, "David S. Miller" <davem@davemloft.net> Subject: RE: FW: [PATCH] Support ia32 exec domains without CONFIG_IA32_SUPPORT On Wed, 20 Oct 2004, Seth, Rohit wrote: > > Without the sys_altroot system call, the kernel still has the support of > providing alternate root. Though that support is embeded in > exec_domains. The new system call support is extending that mechanism > to normal applications. In its current form, emulators will be the most > immediate customers for this system call. No. You're not thinking this through. You not only expose the dang thing with a system call, you also _clear_ it at every execve. Which, as far as I can tell, makes it totally useless for what it used to do. I might be wrong, but the point is that you seem to have appropriated a (bad) interface, removing the usefullness from the original use, and taking it over for your own uses. So no, the kernel does _not_ support alternate root any more, afaik. The code is there, but the support is gone. And quite frankly, if we're going to break the only previous user of altroot, I'd rather remove the feature entirely than replace one horrid user with another. I bet you can do your emulation thing more cleanly with namespaces anyway. Linus ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] revert- sys_setaltroot 2004-12-22 11:03 ` Andrew Morton @ 2004-12-22 11:45 ` David Woodhouse 2005-01-17 13:35 ` David Woodhouse 1 sibling, 0 replies; 6+ messages in thread From: David Woodhouse @ 2004-12-22 11:45 UTC (permalink / raw) To: Andrew Morton; +Cc: linux-kernel, alan On Wed, 2004-12-22 at 03:03 -0800, Andrew Morton wrote: > There were security problems and suggestions that a namespace-based > approach would be better. umm, have a random sprinkle of emails: Thanks. > Begin forwarded message: > > Date: Tue, 19 Oct 2004 15:32:57 -0700 (PDT) > From: Linus Torvalds <torvalds@osdl.org> > To: "Seth, Rohit" <rohit.seth@intel.com>, Al Viro <viro@parcelfarce.linux.theplanet.co.uk> > Cc: Andrew Morton <akpm@osdl.org>, "David S. Miller" <davem@davemloft.net> > Subject: RE: FW: [PATCH] Support ia32 exec domains without CONFIG_IA32_SUPPORT > > > > Hmm. Do we actually want to clear the alternate root? It looks like that > would make altroot useless. > > Who uses it? I thought it was only really used for the emulation > environments on sparc64 etc. And clearly now ia64. But not saving it > across an exec would make it useless, no? No, clearing the altroot doesn't make it useless. If we're executing another non-native binary, the emulator will set the altroot again. This isn't _supposed_ to be a chroot. It's mostly to keep the dynamic linker happy, and resetting it on exec is fine. We were doing that anyway when it was a function of the personality, because executing new binaries also resets the personality. I'm using it on ppc to run i386 binaries -- it's certainly not limited to sparc64 and ia64. > Begin forwarded message: > > Date: Fri, 22 Oct 2004 17:28:35 -0700 (PDT) > From: Linus Torvalds <torvalds@osdl.org> > To: "Seth, Rohit" <rohit.seth@intel.com> > Cc: Andrew Morton <akpm@osdl.org>, Al Viro <viro@parcelfarce.linux.theplanet.co.uk> > Subject: RE: FW: [PATCH] Support ia32 exec domains without CONFIG_IA32_SUPPORT > > On Fri, 22 Oct 2004, Seth, Rohit wrote: > > > > Could you please let me know how we should proceed with this issue so > > that emulators can seemlessly support different execution domains. > > Well, my _personal_ preference by far is to use the equivalent of "chroot" > for the emulated environment. So whenever you hit a x86 binary, you just > chroot to /emul, and then you run it there. > > NOTE! I say "equivalent", because you could do it by creating a separate > namespace instead, if you wanted to - and just overmounting /lib and > /usr/lib with the emulation environment versions in that particular > namespace. > > Also, note the bind mounts: if you do end up having "/emul", you want to > have the local /lib and /usr/lib be there, but the rest would be just bind > mounts back to the original root, making things pretty similar (ie people > would still see all their normal files in their home directories etc, it > would be just /lib that effectively gets swapped out). I see the point but I'm not sure I understand in detail how this can work with namespace. It's not just /lib and /usr/lib which exist in the altroot -- we also need certain files from /etc too, while leaving the rest of /etc as it was. Do we have to bind-mount just those specific files? And can we arrange for the alternate namespace to be automatically dropped on exec() too? And can we do this without needing the interpreter to be setuid in all cases? > Al may have more intelligent suggestions. Hopefully :) -- dwmw2 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] revert- sys_setaltroot 2004-12-22 11:03 ` Andrew Morton 2004-12-22 11:45 ` David Woodhouse @ 2005-01-17 13:35 ` David Woodhouse 2005-01-17 13:42 ` Arjan van de Ven 1 sibling, 1 reply; 6+ messages in thread From: David Woodhouse @ 2005-01-17 13:35 UTC (permalink / raw) To: Andrew Morton; +Cc: viro, linux-kernel, alan On Wed, 2004-12-22 at 03:03 -0800, Andrew Morton wrote: > There were security problems and suggestions that a namespace-based > approach would be better. umm, have a random sprinkle of emails: The security problems have been addressed, and the namespace-based approach was never coherently explained. I can't see how such would work. -- dwmw2 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] revert- sys_setaltroot 2005-01-17 13:35 ` David Woodhouse @ 2005-01-17 13:42 ` Arjan van de Ven 2005-01-19 14:44 ` David Woodhouse 0 siblings, 1 reply; 6+ messages in thread From: Arjan van de Ven @ 2005-01-17 13:42 UTC (permalink / raw) To: David Woodhouse; +Cc: Andrew Morton, viro, linux-kernel, alan On Mon, 2005-01-17 at 13:35 +0000, David Woodhouse wrote: > On Wed, 2004-12-22 at 03:03 -0800, Andrew Morton wrote: > > There were security problems and suggestions that a namespace-based > > approach would be better. umm, have a random sprinkle of emails: > > The security problems have been addressed, and the namespace-based > approach was never coherently explained. I can't see how such would > work. see viro's recent proposal for bindmounts and the like. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] revert- sys_setaltroot 2005-01-17 13:42 ` Arjan van de Ven @ 2005-01-19 14:44 ` David Woodhouse 0 siblings, 0 replies; 6+ messages in thread From: David Woodhouse @ 2005-01-19 14:44 UTC (permalink / raw) To: Arjan van de Ven; +Cc: Andrew Morton, viro, linux-kernel, alan On Mon, 2005-01-17 at 14:42 +0100, Arjan van de Ven wrote: > > The security problems have been addressed, and the namespace-based > > approach was never coherently explained. I can't see how such would > > work. > > see viro's recent proposal for bindmounts and the like. Is that likely to be usable any time soon? -- dwmw2 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2005-01-19 14:46 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <200410261928.i9QJS7h3011015@hera.kernel.org>
2004-12-22 10:18 ` [PATCH] revert- sys_setaltroot David Woodhouse
2004-12-22 11:03 ` Andrew Morton
2004-12-22 11:45 ` David Woodhouse
2005-01-17 13:35 ` David Woodhouse
2005-01-17 13:42 ` Arjan van de Ven
2005-01-19 14:44 ` David Woodhouse
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).