* [PATCH] core_pattern: set core helpers root and namespace to crashing process
@ 2012-12-11 19:59 Neil Horman
2012-12-14 21:04 ` [PATCH v2] " Neil Horman
[not found] ` <1355255996-25953-1-git-send-email-nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
0 siblings, 2 replies; 20+ messages in thread
From: Neil Horman @ 2012-12-11 19:59 UTC (permalink / raw)
To: linux-kernel
Cc: Neil Horman, Daniel Berrange, Alexander Viro, Andrew Morton,
Serge Hallyn, containers
As its currently implemented, redirection of core dumps to a pipe reader should
be executed such that the reader runs in the namespace of the crashing process,
and it currently does not. This is the only sane way to deal with namespaces
properly it seems to me, and this patch implements that functionality.
Theres one problem I currently see with it, and that is that I'm not sure we can
change the current behavior of how the root fs is set for the pipe reader, lest
we break some user space expectations. As such, I've added a sysctl in this
patch to allow administrators to globally select if a core reader specified via
/proc/sys/kernel/core_pattern should use the global rootfs, or the (possibly)
chrooted fs of the crashing process.
I've tested this using both settings for the new sysctl, and it works well.
For the sake of history, this was discussed before:
http://www.spinics.net/lists/linux-containers/msg21531.html
It seems there was some modicum of consensus as to how this should work, but
there was no action taken on it.
Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
Reported-by: Daniel Berrange <berrange@redhat.com>
CC: Daniel Berrange <berrange@redhat.com>
CC: Alexander Viro <viro@zeniv.linux.org.uk>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: Serge Hallyn <serge.hallyn@canonical.com>
CC: containers@lists.linux-foundation.org
---
fs/coredump.c | 11 +++++++++++
include/linux/binfmts.h | 1 +
kernel/sysctl.c | 8 ++++++++
3 files changed, 20 insertions(+)
diff --git a/fs/coredump.c b/fs/coredump.c
index ce47379..e7d8ca2 100644
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -47,6 +47,7 @@
int core_uses_pid;
char core_pattern[CORENAME_MAX_SIZE] = "core";
unsigned int core_pipe_limit;
+unsigned int core_pipe_global_root;
struct core_name {
char *corename;
@@ -443,6 +444,7 @@ static void wait_for_dump_helpers(struct file *file)
static int umh_pipe_setup(struct subprocess_info *info, struct cred *new)
{
struct file *files[2];
+ struct path root;
struct coredump_params *cp = (struct coredump_params *)info->data;
int err = create_pipe_files(files, 0);
if (err)
@@ -455,6 +457,14 @@ static int umh_pipe_setup(struct subprocess_info *info, struct cred *new)
/* and disallow core files too */
current->signal->rlim[RLIMIT_CORE] = (struct rlimit){1, 1};
+
+ if (!core_pipe_global_root) {
+ get_fs_root(cp->cprocess->fs, &root);
+ set_fs_root(current->fs, &root);
+ }
+
+ switch_task_namespaces(current, cp->cprocess->nsproxy);
+
return err;
}
@@ -476,6 +486,7 @@ void do_coredump(siginfo_t *siginfo, struct pt_regs *regs)
.siginfo = siginfo,
.regs = regs,
.limit = rlimit(RLIMIT_CORE),
+ .cprocess = current,
/*
* We must use the same mm->flags while dumping core to avoid
* inconsistency of bit flags, since this flag is not protected
diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
index cfcc6bf..08e7bcb 100644
--- a/include/linux/binfmts.h
+++ b/include/linux/binfmts.h
@@ -61,6 +61,7 @@ struct coredump_params {
siginfo_t *siginfo;
struct pt_regs *regs;
struct file *file;
+ struct task_struct *cprocess;
unsigned long limit;
unsigned long mm_flags;
};
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 26f65ea..be7b69d 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -102,6 +102,7 @@ extern int suid_dumpable;
extern int core_uses_pid;
extern char core_pattern[];
extern unsigned int core_pipe_limit;
+extern unsigned int core_pipe_global_root;
#endif
extern int pid_max;
extern int min_free_kbytes;
@@ -430,6 +431,13 @@ static struct ctl_table kern_table[] = {
.mode = 0644,
.proc_handler = proc_dointvec,
},
+ {
+ .procname = "core_pipe_global_root",
+ .data = &core_pipe_global_root,
+ .maxlen = sizeof(unsigned int),
+ .mode = 0644,
+ .proc_handler = proc_dointvec,
+ },
#endif
#ifdef CONFIG_PROC_SYSCTL
{
--
1.7.11.7
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v2] core_pattern: set core helpers root and namespace to crashing process 2012-12-11 19:59 [PATCH] core_pattern: set core helpers root and namespace to crashing process Neil Horman @ 2012-12-14 21:04 ` Neil Horman [not found] ` <1355519048-28473-1-git-send-email-nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org> [not found] ` <1355255996-25953-1-git-send-email-nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org> 1 sibling, 1 reply; 20+ messages in thread From: Neil Horman @ 2012-12-14 21:04 UTC (permalink / raw) To: linux-kernel Cc: Neil Horman, Daniel Berrange, Alexander Viro, Andrew Morton, Serge Hallyn, containers As its currently implemented, redirection of core dumps to a pipe reader should be executed such that the reader runs in the namespace of the crashing process, and it currently does not. This is the only sane way to deal with namespaces properly it seems to me, and this patch implements that functionality. Theres one problem I currently see with it, and that is that I'm not sure we can change the current behavior of how the root fs is set for the pipe reader, lest we break some user space expectations. As such I've made the switching of namespaces configurable based on the addition of an extra '|' token in the core_pattern sysctl. 1 '|' indicates a pipe using the global namespace, while 2 '|' indicates that the namespace and root of the crashing process should be used. I've tested this using both settings for the new sysctl, and it works well. For the sake of history, this was discussed before: http://www.spinics.net/lists/linux-containers/msg21531.html It seems there was some modicum of consensus as to how this should work, but there was no action taken on it. Signed-off-by: Neil Horman <nhorman@tuxdriver.com> Reported-by: Daniel Berrange <berrange@redhat.com> CC: Daniel Berrange <berrange@redhat.com> CC: Alexander Viro <viro@zeniv.linux.org.uk> CC: Andrew Morton <akpm@linux-foundation.org> CC: Serge Hallyn <serge.hallyn@canonical.com> CC: containers@lists.linux-foundation.org --- Documentation/sysctl/kernel.txt | 7 ++++++- fs/coredump.c | 23 +++++++++++++++++++++++ include/linux/binfmts.h | 2 ++ 3 files changed, 31 insertions(+), 1 deletion(-) diff --git a/Documentation/sysctl/kernel.txt b/Documentation/sysctl/kernel.txt index 2907ba6..f853fca 100644 --- a/Documentation/sysctl/kernel.txt +++ b/Documentation/sysctl/kernel.txt @@ -191,7 +191,12 @@ core_pattern is used to specify a core dumpfile pattern name. %<OTHER> both are dropped . If the first character of the pattern is a '|', the kernel will treat the rest of the pattern as a command to run. The core dump will be - written to the standard input of that program instead of to a file. + written to the standard input of that program instead of to a file. Note that + when using |, the core pipe reader that is executed will be run in the global + namespace and root filesystem. If two | tokens (i.e. ||) are supplied as the + first two characters of the core_pattern sysctl, the kernel will preform the + same pipe operation, but the core pipe reader will be executed using the + namespace and root fs of the crashing process. ============================================================== diff --git a/fs/coredump.c b/fs/coredump.c index ce47379..d4b71c0 100644 --- a/fs/coredump.c +++ b/fs/coredump.c @@ -164,6 +164,18 @@ static int format_corename(struct core_name *cn, struct coredump_params *cprm) if (!cn->corename) return -ENOMEM; + if (ispipe) { + /* + * If we have 2 | tokens at the head of core_pattern, it + * indicates we are a pipe and the reader should inherit the + * namespaces of the crashing process + */ + cprm->switch_ns = (*(pat_ptr+1) == '|') ? true : false; + if (cprm->switch_ns) + /* Advance pat_ptr so as not to mess up corename */ + pat_ptr++; + } + /* Repeat as long as we have more pattern to process and more output space */ while (*pat_ptr) { @@ -443,6 +455,7 @@ static void wait_for_dump_helpers(struct file *file) static int umh_pipe_setup(struct subprocess_info *info, struct cred *new) { struct file *files[2]; + struct path root; struct coredump_params *cp = (struct coredump_params *)info->data; int err = create_pipe_files(files, 0); if (err) @@ -455,6 +468,14 @@ static int umh_pipe_setup(struct subprocess_info *info, struct cred *new) /* and disallow core files too */ current->signal->rlim[RLIMIT_CORE] = (struct rlimit){1, 1}; + + if (cp->switch_ns) { + get_fs_root(cp->cprocess->fs, &root); + set_fs_root(current->fs, &root); + switch_task_namespaces(current, cp->cprocess->nsproxy); + } + + return err; } @@ -476,6 +497,8 @@ void do_coredump(siginfo_t *siginfo, struct pt_regs *regs) .siginfo = siginfo, .regs = regs, .limit = rlimit(RLIMIT_CORE), + .cprocess = current, + .switch_ns = false, /* * We must use the same mm->flags while dumping core to avoid * inconsistency of bit flags, since this flag is not protected diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h index cfcc6bf..3f06261 100644 --- a/include/linux/binfmts.h +++ b/include/linux/binfmts.h @@ -61,6 +61,8 @@ struct coredump_params { siginfo_t *siginfo; struct pt_regs *regs; struct file *file; + struct task_struct *cprocess; + bool switch_ns; unsigned long limit; unsigned long mm_flags; }; -- 1.7.11.7 ^ permalink raw reply related [flat|nested] 20+ messages in thread
[parent not found: <1355519048-28473-1-git-send-email-nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>]
* Re: [PATCH v2] core_pattern: set core helpers root and namespace to crashing process 2012-12-14 21:04 ` [PATCH v2] " Neil Horman @ 2012-12-14 21:49 ` Andrew Morton 0 siblings, 0 replies; 20+ messages in thread From: Andrew Morton @ 2012-12-14 21:49 UTC (permalink / raw) To: Neil Horman Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Alexander Viro On Fri, 14 Dec 2012 16:04:08 -0500 Neil Horman <nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org> wrote: > As its currently implemented, redirection of core dumps to a pipe reader should > be executed such that the reader runs in the namespace of the crashing process, > and it currently does not. This is the only sane way to deal with namespaces > properly it seems to me, and this patch implements that functionality. > > Theres one problem I currently see with it, and that is that I'm not sure we can > change the current behavior of how the root fs is set for the pipe reader, lest > we break some user space expectations. As such I've made the switching of > namespaces configurable based on the addition of an extra '|' token in the > core_pattern sysctl. 1 '|' indicates a pipe using the global namespace, while > 2 '|' indicates that the namespace and root of the crashing process should be > used. > > I've tested this using both settings for the new sysctl, and it works well. > > For the sake of history, this was discussed before: > http://www.spinics.net/lists/linux-containers/msg21531.html > > It seems there was some modicum of consensus as to how this should work, but > there was no action taken on it. > > --- a/fs/coredump.c > +++ b/fs/coredump.c > @@ -164,6 +164,18 @@ static int format_corename(struct core_name *cn, struct coredump_params *cprm) > if (!cn->corename) > return -ENOMEM; > > + if (ispipe) { > + /* > + * If we have 2 | tokens at the head of core_pattern, it > + * indicates we are a pipe and the reader should inherit the > + * namespaces of the crashing process > + */ > + cprm->switch_ns = (*(pat_ptr+1) == '|') ? true : false; > + if (cprm->switch_ns) > + /* Advance pat_ptr so as not to mess up corename */ > + pat_ptr++; > + } > + That was, umm, intricate. How's about this? if (ispipe && pat_ptr[1] == '|') { /* * We have 2 | tokens at the head of core_pattern which * indicates we are a pipe and the reader should inherit the * namespaces of the crashing process */ cprm->switch_ns = true; pat_ptr++; } --- a/Documentation/sysctl/kernel.txt~core_pattern-set-core-helpers-root-and-namespace-to-crashing-process-fix +++ a/Documentation/sysctl/kernel.txt @@ -194,7 +194,7 @@ core_pattern is used to specify a core d written to the standard input of that program instead of to a file. Note that when using |, the core pipe reader that is executed will be run in the global namespace and root filesystem. If two | tokens (i.e. ||) are supplied as the - first two characters of the core_pattern sysctl, the kernel will preform the + first two characters of the core_pattern sysctl, the kernel will perform the same pipe operation, but the core pipe reader will be executed using the namespace and root fs of the crashing process. --- a/fs/coredump.c~core_pattern-set-core-helpers-root-and-namespace-to-crashing-process-fix +++ a/fs/coredump.c @@ -164,16 +164,14 @@ static int format_corename(struct core_n if (!cn->corename) return -ENOMEM; - if (ispipe) { + if (ispipe && pat_ptr[1] == '|') { /* - * If we have 2 | tokens at the head of core_pattern, it + * We have 2 | tokens at the head of core_pattern which * indicates we are a pipe and the reader should inherit the * namespaces of the crashing process */ - cprm->switch_ns = (*(pat_ptr+1) == '|') ? true : false; - if (cprm->switch_ns) - /* Advance pat_ptr so as not to mess up corename */ - pat_ptr++; + cprm->switch_ns = true; + pat_ptr++; } /* Repeat as long as we have more pattern to process and more output _ ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2] core_pattern: set core helpers root and namespace to crashing process @ 2012-12-14 21:49 ` Andrew Morton 0 siblings, 0 replies; 20+ messages in thread From: Andrew Morton @ 2012-12-14 21:49 UTC (permalink / raw) To: Neil Horman Cc: linux-kernel, Daniel Berrange, Alexander Viro, Serge Hallyn, containers On Fri, 14 Dec 2012 16:04:08 -0500 Neil Horman <nhorman@tuxdriver.com> wrote: > As its currently implemented, redirection of core dumps to a pipe reader should > be executed such that the reader runs in the namespace of the crashing process, > and it currently does not. This is the only sane way to deal with namespaces > properly it seems to me, and this patch implements that functionality. > > Theres one problem I currently see with it, and that is that I'm not sure we can > change the current behavior of how the root fs is set for the pipe reader, lest > we break some user space expectations. As such I've made the switching of > namespaces configurable based on the addition of an extra '|' token in the > core_pattern sysctl. 1 '|' indicates a pipe using the global namespace, while > 2 '|' indicates that the namespace and root of the crashing process should be > used. > > I've tested this using both settings for the new sysctl, and it works well. > > For the sake of history, this was discussed before: > http://www.spinics.net/lists/linux-containers/msg21531.html > > It seems there was some modicum of consensus as to how this should work, but > there was no action taken on it. > > --- a/fs/coredump.c > +++ b/fs/coredump.c > @@ -164,6 +164,18 @@ static int format_corename(struct core_name *cn, struct coredump_params *cprm) > if (!cn->corename) > return -ENOMEM; > > + if (ispipe) { > + /* > + * If we have 2 | tokens at the head of core_pattern, it > + * indicates we are a pipe and the reader should inherit the > + * namespaces of the crashing process > + */ > + cprm->switch_ns = (*(pat_ptr+1) == '|') ? true : false; > + if (cprm->switch_ns) > + /* Advance pat_ptr so as not to mess up corename */ > + pat_ptr++; > + } > + That was, umm, intricate. How's about this? if (ispipe && pat_ptr[1] == '|') { /* * We have 2 | tokens at the head of core_pattern which * indicates we are a pipe and the reader should inherit the * namespaces of the crashing process */ cprm->switch_ns = true; pat_ptr++; } --- a/Documentation/sysctl/kernel.txt~core_pattern-set-core-helpers-root-and-namespace-to-crashing-process-fix +++ a/Documentation/sysctl/kernel.txt @@ -194,7 +194,7 @@ core_pattern is used to specify a core d written to the standard input of that program instead of to a file. Note that when using |, the core pipe reader that is executed will be run in the global namespace and root filesystem. If two | tokens (i.e. ||) are supplied as the - first two characters of the core_pattern sysctl, the kernel will preform the + first two characters of the core_pattern sysctl, the kernel will perform the same pipe operation, but the core pipe reader will be executed using the namespace and root fs of the crashing process. --- a/fs/coredump.c~core_pattern-set-core-helpers-root-and-namespace-to-crashing-process-fix +++ a/fs/coredump.c @@ -164,16 +164,14 @@ static int format_corename(struct core_n if (!cn->corename) return -ENOMEM; - if (ispipe) { + if (ispipe && pat_ptr[1] == '|') { /* - * If we have 2 | tokens at the head of core_pattern, it + * We have 2 | tokens at the head of core_pattern which * indicates we are a pipe and the reader should inherit the * namespaces of the crashing process */ - cprm->switch_ns = (*(pat_ptr+1) == '|') ? true : false; - if (cprm->switch_ns) - /* Advance pat_ptr so as not to mess up corename */ - pat_ptr++; + cprm->switch_ns = true; + pat_ptr++; } /* Repeat as long as we have more pattern to process and more output _ ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2] core_pattern: set core helpers root and namespace to crashing process 2012-12-14 21:04 ` [PATCH v2] " Neil Horman @ 2012-12-14 23:10 ` Eric W. Biederman 0 siblings, 0 replies; 20+ messages in thread From: Eric W. Biederman @ 2012-12-14 23:10 UTC (permalink / raw) To: Neil Horman Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Andrew Morton, Alexander Viro Neil Horman <nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org> writes: > As its currently implemented, redirection of core dumps to a pipe reader should > be executed such that the reader runs in the namespace of the crashing process, > and it currently does not. This is the only sane way to deal with namespaces > properly it seems to me, and this patch implements that functionality. I actually rather strongly disagree. While we have a global core dump pattern core dumps to a a pipe reader should be executed such that the reader runs in the namespace of the process that set the pattern. We can easily restrict that to the initial namespaces to make the implementation simpler. If you want to play namespace games you can implement all of those in user space once my tree merges for v3.8. I am really not a fan of the trigger process being able to control the environment of a privileged process. It makes writing the privileged process much trickier. Eric ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2] core_pattern: set core helpers root and namespace to crashing process @ 2012-12-14 23:10 ` Eric W. Biederman 0 siblings, 0 replies; 20+ messages in thread From: Eric W. Biederman @ 2012-12-14 23:10 UTC (permalink / raw) To: Neil Horman; +Cc: linux-kernel, containers, Alexander Viro, Andrew Morton Neil Horman <nhorman@tuxdriver.com> writes: > As its currently implemented, redirection of core dumps to a pipe reader should > be executed such that the reader runs in the namespace of the crashing process, > and it currently does not. This is the only sane way to deal with namespaces > properly it seems to me, and this patch implements that functionality. I actually rather strongly disagree. While we have a global core dump pattern core dumps to a a pipe reader should be executed such that the reader runs in the namespace of the process that set the pattern. We can easily restrict that to the initial namespaces to make the implementation simpler. If you want to play namespace games you can implement all of those in user space once my tree merges for v3.8. I am really not a fan of the trigger process being able to control the environment of a privileged process. It makes writing the privileged process much trickier. Eric ^ permalink raw reply [flat|nested] 20+ messages in thread
[parent not found: <87d2ycut5l.fsf-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>]
* Re: [PATCH v2] core_pattern: set core helpers root and namespace to crashing process 2012-12-14 23:10 ` Eric W. Biederman @ 2012-12-15 0:50 ` Neil Horman -1 siblings, 0 replies; 20+ messages in thread From: Neil Horman @ 2012-12-15 0:50 UTC (permalink / raw) To: Eric W. Biederman Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Andrew Morton, Alexander Viro On Fri, Dec 14, 2012 at 03:10:30PM -0800, Eric W. Biederman wrote: > Neil Horman <nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org> writes: > > > As its currently implemented, redirection of core dumps to a pipe reader should > > be executed such that the reader runs in the namespace of the crashing process, > > and it currently does not. This is the only sane way to deal with namespaces > > properly it seems to me, and this patch implements that functionality. > > I actually rather strongly disagree. > > While we have a global core dump pattern core dumps to a a pipe reader > should be executed such that the reader runs in the namespace of the > process that set the pattern. We can easily restrict that to the > initial namespaces to make the implementation simpler. > > If you want to play namespace games you can implement all of those in > user space once my tree merges for v3.8. > > I am really not a fan of the trigger process being able to control the > environment of a privileged process. It makes writing the privileged > process much trickier. > Why? What specific problem do you see with allowing a privlidged process to execute within a specific namespace, that doesn't also exist with having the pipe reader execute in the init namespace? Note I'm not saying that a poorly constructed pipe reader application doesn't have security problems if it doesn't validate the environment that its running in, but thats something that the pipe reader needs to be sure about. Note also, that if the token in core_pattern is set such that the core_pattern is namespace/root relative, that container needs to install the application relative its root as well (e.g. positive action still needs to be taken on the part of the container admin to make this work). For example, if you set core_pattern="||/usr/bin/foo" Then a process running in a chroot based at /sub/root/ still needs to install a file /sub/root/usr/bin/foo, or the core dump will fail. So its not like a container can just have a core reader execute without first making an administrative decision to do so. Neil ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2] core_pattern: set core helpers root and namespace to crashing process @ 2012-12-15 0:50 ` Neil Horman 0 siblings, 0 replies; 20+ messages in thread From: Neil Horman @ 2012-12-15 0:50 UTC (permalink / raw) To: Eric W. Biederman; +Cc: linux-kernel, containers, Alexander Viro, Andrew Morton On Fri, Dec 14, 2012 at 03:10:30PM -0800, Eric W. Biederman wrote: > Neil Horman <nhorman@tuxdriver.com> writes: > > > As its currently implemented, redirection of core dumps to a pipe reader should > > be executed such that the reader runs in the namespace of the crashing process, > > and it currently does not. This is the only sane way to deal with namespaces > > properly it seems to me, and this patch implements that functionality. > > I actually rather strongly disagree. > > While we have a global core dump pattern core dumps to a a pipe reader > should be executed such that the reader runs in the namespace of the > process that set the pattern. We can easily restrict that to the > initial namespaces to make the implementation simpler. > > If you want to play namespace games you can implement all of those in > user space once my tree merges for v3.8. > > I am really not a fan of the trigger process being able to control the > environment of a privileged process. It makes writing the privileged > process much trickier. > Why? What specific problem do you see with allowing a privlidged process to execute within a specific namespace, that doesn't also exist with having the pipe reader execute in the init namespace? Note I'm not saying that a poorly constructed pipe reader application doesn't have security problems if it doesn't validate the environment that its running in, but thats something that the pipe reader needs to be sure about. Note also, that if the token in core_pattern is set such that the core_pattern is namespace/root relative, that container needs to install the application relative its root as well (e.g. positive action still needs to be taken on the part of the container admin to make this work). For example, if you set core_pattern="||/usr/bin/foo" Then a process running in a chroot based at /sub/root/ still needs to install a file /sub/root/usr/bin/foo, or the core dump will fail. So its not like a container can just have a core reader execute without first making an administrative decision to do so. Neil ^ permalink raw reply [flat|nested] 20+ messages in thread
[parent not found: <1355255996-25953-1-git-send-email-nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>]
* Re: [PATCH] core_pattern: set core helpers root and namespace to crashing process 2012-12-11 19:59 [PATCH] core_pattern: set core helpers root and namespace to crashing process Neil Horman @ 2012-12-13 12:20 ` Serge Hallyn [not found] ` <1355255996-25953-1-git-send-email-nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org> 1 sibling, 0 replies; 20+ messages in thread From: Serge Hallyn @ 2012-12-13 12:20 UTC (permalink / raw) To: Neil Horman Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, Andrew Morton, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Alexander Viro Quoting Neil Horman (nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org): > Theres one problem I currently see with it, and that is that I'm not sure we can > change the current behavior of how the root fs is set for the pipe reader, lest > we break some user space expectations. As such, I've added a sysctl in this > patch to allow administrators to globally select if a core reader specified via > /proc/sys/kernel/core_pattern should use the global rootfs, or the (possibly) > chrooted fs of the crashing process. Practical question: How is the admin to make an educated decision on how to set the sysctl? -serge ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] core_pattern: set core helpers root and namespace to crashing process @ 2012-12-13 12:20 ` Serge Hallyn 0 siblings, 0 replies; 20+ messages in thread From: Serge Hallyn @ 2012-12-13 12:20 UTC (permalink / raw) To: Neil Horman Cc: linux-kernel, Daniel Berrange, Alexander Viro, Andrew Morton, containers Quoting Neil Horman (nhorman@tuxdriver.com): > Theres one problem I currently see with it, and that is that I'm not sure we can > change the current behavior of how the root fs is set for the pipe reader, lest > we break some user space expectations. As such, I've added a sysctl in this > patch to allow administrators to globally select if a core reader specified via > /proc/sys/kernel/core_pattern should use the global rootfs, or the (possibly) > chrooted fs of the crashing process. Practical question: How is the admin to make an educated decision on how to set the sysctl? -serge ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] core_pattern: set core helpers root and namespace to crashing process 2012-12-13 12:20 ` Serge Hallyn (?) @ 2012-12-13 18:12 ` Neil Horman [not found] ` <20121213181220.GB14796-B26myB8xz7F8NnZeBjwnZQMhkBWG/bsMQH7oEaQurus@public.gmane.org> -1 siblings, 1 reply; 20+ messages in thread From: Neil Horman @ 2012-12-13 18:12 UTC (permalink / raw) To: Serge Hallyn Cc: linux-kernel, Daniel Berrange, Alexander Viro, Andrew Morton, containers On Thu, Dec 13, 2012 at 06:20:48AM -0600, Serge Hallyn wrote: > Quoting Neil Horman (nhorman@tuxdriver.com): > > Theres one problem I currently see with it, and that is that I'm not sure we can > > change the current behavior of how the root fs is set for the pipe reader, lest > > we break some user space expectations. As such, I've added a sysctl in this > > patch to allow administrators to globally select if a core reader specified via > > /proc/sys/kernel/core_pattern should use the global rootfs, or the (possibly) > > chrooted fs of the crashing process. > > Practical question: How is the admin to make an educated decision on > how to set the sysctl? > My thought was that the admin typically wouldn't touch this at all. I really added it as a backwards compatibility option only. Setting the user space helper task to the root of the crashing parent has the possibility of breaking existing installs because the core_pattern helper might be expecting global file system access. Moving forward, my expectation would be that core_pattern helpers would be written with the default setting in mind, and we could eventually deprecate the control entirely. If you have a better mechanism in mind however (or if you think that removing the control is a resaonable approach), I'm certainly open to that. Neil > -serge > ^ permalink raw reply [flat|nested] 20+ messages in thread
[parent not found: <20121213181220.GB14796-B26myB8xz7F8NnZeBjwnZQMhkBWG/bsMQH7oEaQurus@public.gmane.org>]
* Re: [PATCH] core_pattern: set core helpers root and namespace to crashing process 2012-12-13 18:12 ` Neil Horman @ 2012-12-13 22:25 ` Andrew Morton 0 siblings, 0 replies; 20+ messages in thread From: Andrew Morton @ 2012-12-13 22:25 UTC (permalink / raw) To: Neil Horman Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Alexander Viro On Thu, 13 Dec 2012 13:12:20 -0500 Neil Horman <nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org> wrote: > On Thu, Dec 13, 2012 at 06:20:48AM -0600, Serge Hallyn wrote: > > Quoting Neil Horman (nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org): > > > Theres one problem I currently see with it, and that is that I'm not sure we can > > > change the current behavior of how the root fs is set for the pipe reader, lest > > > we break some user space expectations. As such, I've added a sysctl in this > > > patch to allow administrators to globally select if a core reader specified via > > > /proc/sys/kernel/core_pattern should use the global rootfs, or the (possibly) > > > chrooted fs of the crashing process. > > > > Practical question: How is the admin to make an educated decision on > > how to set the sysctl? By reading the documentation which Neil didn't include? > My thought was that the admin typically wouldn't touch this at all. I really > added it as a backwards compatibility option only. Setting the user space > helper task to the root of the crashing parent has the possibility of breaking > existing installs because the core_pattern helper might be expecting global file > system access. Moving forward, my expectation would be that core_pattern > helpers would be written with the default setting in mind, and we could > eventually deprecate the control entirely. > > If you have a better mechanism in mind however (or if you think that removing > the control is a resaonable approach), I'm certainly open to that. Yeah, this is a tiresome patch but I can't think of a better way. Except, perhaps, adding a new token to the core_pattern which says "switch namespaces"? Is there any propect that the core_pattern itself will later become a per-namespace containerised thing? I guess that if the per-container core_pattern has been configured, we can implicitly do the namespace switch as well. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] core_pattern: set core helpers root and namespace to crashing process @ 2012-12-13 22:25 ` Andrew Morton 0 siblings, 0 replies; 20+ messages in thread From: Andrew Morton @ 2012-12-13 22:25 UTC (permalink / raw) To: Neil Horman Cc: Serge Hallyn, linux-kernel, Daniel Berrange, Alexander Viro, containers On Thu, 13 Dec 2012 13:12:20 -0500 Neil Horman <nhorman@tuxdriver.com> wrote: > On Thu, Dec 13, 2012 at 06:20:48AM -0600, Serge Hallyn wrote: > > Quoting Neil Horman (nhorman@tuxdriver.com): > > > Theres one problem I currently see with it, and that is that I'm not sure we can > > > change the current behavior of how the root fs is set for the pipe reader, lest > > > we break some user space expectations. As such, I've added a sysctl in this > > > patch to allow administrators to globally select if a core reader specified via > > > /proc/sys/kernel/core_pattern should use the global rootfs, or the (possibly) > > > chrooted fs of the crashing process. > > > > Practical question: How is the admin to make an educated decision on > > how to set the sysctl? By reading the documentation which Neil didn't include? > My thought was that the admin typically wouldn't touch this at all. I really > added it as a backwards compatibility option only. Setting the user space > helper task to the root of the crashing parent has the possibility of breaking > existing installs because the core_pattern helper might be expecting global file > system access. Moving forward, my expectation would be that core_pattern > helpers would be written with the default setting in mind, and we could > eventually deprecate the control entirely. > > If you have a better mechanism in mind however (or if you think that removing > the control is a resaonable approach), I'm certainly open to that. Yeah, this is a tiresome patch but I can't think of a better way. Except, perhaps, adding a new token to the core_pattern which says "switch namespaces"? Is there any propect that the core_pattern itself will later become a per-namespace containerised thing? I guess that if the per-container core_pattern has been configured, we can implicitly do the namespace switch as well. ^ permalink raw reply [flat|nested] 20+ messages in thread
[parent not found: <20121213142534.ea07a3c8.akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>]
* Re: [PATCH] core_pattern: set core helpers root and namespace to crashing process 2012-12-13 22:25 ` Andrew Morton @ 2012-12-14 2:49 ` Neil Horman -1 siblings, 0 replies; 20+ messages in thread From: Neil Horman @ 2012-12-14 2:49 UTC (permalink / raw) To: Andrew Morton Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Alexander Viro On Thu, Dec 13, 2012 at 02:25:34PM -0800, Andrew Morton wrote: > On Thu, 13 Dec 2012 13:12:20 -0500 > Neil Horman <nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org> wrote: > > > On Thu, Dec 13, 2012 at 06:20:48AM -0600, Serge Hallyn wrote: > > > Quoting Neil Horman (nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org): > > > > Theres one problem I currently see with it, and that is that I'm not sure we can > > > > change the current behavior of how the root fs is set for the pipe reader, lest > > > > we break some user space expectations. As such, I've added a sysctl in this > > > > patch to allow administrators to globally select if a core reader specified via > > > > /proc/sys/kernel/core_pattern should use the global rootfs, or the (possibly) > > > > chrooted fs of the crashing process. > > > > > > Practical question: How is the admin to make an educated decision on > > > how to set the sysctl? > > By reading the documentation which Neil didn't include? > Yeah, that was stupid of me, I'll respin this with docs. > > My thought was that the admin typically wouldn't touch this at all. I really > > added it as a backwards compatibility option only. Setting the user space > > helper task to the root of the crashing parent has the possibility of breaking > > existing installs because the core_pattern helper might be expecting global file > > system access. Moving forward, my expectation would be that core_pattern > > helpers would be written with the default setting in mind, and we could > > eventually deprecate the control entirely. > > > > If you have a better mechanism in mind however (or if you think that removing > > the control is a resaonable approach), I'm certainly open to that. > > Yeah, this is a tiresome patch but I can't think of a better way. > > Except, perhaps, adding a new token to the core_pattern which says > "switch namespaces"? > I like that idea, perhaps '||' instead of '|' as the leading token can indicate "use the namespace root" vs. "use the global root". Thoughts? > Is there any propect that the core_pattern itself will later become a > per-namespace containerised thing? I guess that if the per-container > core_pattern has been configured, we can implicitly do the namespace > switch as well. Yes, that makes sense. Unfortunately, I don't see proc containerization happening any time soon. I suppose if we do the above tokenization, that can be used despite any future containerization that takes place. I'll respin the patch with documentation, and replace the extra sysctl with the above tokenization in the AM. Best Neil > > ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] core_pattern: set core helpers root and namespace to crashing process @ 2012-12-14 2:49 ` Neil Horman 0 siblings, 0 replies; 20+ messages in thread From: Neil Horman @ 2012-12-14 2:49 UTC (permalink / raw) To: Andrew Morton Cc: Serge Hallyn, linux-kernel, Daniel Berrange, Alexander Viro, containers On Thu, Dec 13, 2012 at 02:25:34PM -0800, Andrew Morton wrote: > On Thu, 13 Dec 2012 13:12:20 -0500 > Neil Horman <nhorman@tuxdriver.com> wrote: > > > On Thu, Dec 13, 2012 at 06:20:48AM -0600, Serge Hallyn wrote: > > > Quoting Neil Horman (nhorman@tuxdriver.com): > > > > Theres one problem I currently see with it, and that is that I'm not sure we can > > > > change the current behavior of how the root fs is set for the pipe reader, lest > > > > we break some user space expectations. As such, I've added a sysctl in this > > > > patch to allow administrators to globally select if a core reader specified via > > > > /proc/sys/kernel/core_pattern should use the global rootfs, or the (possibly) > > > > chrooted fs of the crashing process. > > > > > > Practical question: How is the admin to make an educated decision on > > > how to set the sysctl? > > By reading the documentation which Neil didn't include? > Yeah, that was stupid of me, I'll respin this with docs. > > My thought was that the admin typically wouldn't touch this at all. I really > > added it as a backwards compatibility option only. Setting the user space > > helper task to the root of the crashing parent has the possibility of breaking > > existing installs because the core_pattern helper might be expecting global file > > system access. Moving forward, my expectation would be that core_pattern > > helpers would be written with the default setting in mind, and we could > > eventually deprecate the control entirely. > > > > If you have a better mechanism in mind however (or if you think that removing > > the control is a resaonable approach), I'm certainly open to that. > > Yeah, this is a tiresome patch but I can't think of a better way. > > Except, perhaps, adding a new token to the core_pattern which says > "switch namespaces"? > I like that idea, perhaps '||' instead of '|' as the leading token can indicate "use the namespace root" vs. "use the global root". Thoughts? > Is there any propect that the core_pattern itself will later become a > per-namespace containerised thing? I guess that if the per-container > core_pattern has been configured, we can implicitly do the namespace > switch as well. Yes, that makes sense. Unfortunately, I don't see proc containerization happening any time soon. I suppose if we do the above tokenization, that can be used despite any future containerization that takes place. I'll respin the patch with documentation, and replace the extra sysctl with the above tokenization in the AM. Best Neil > > ^ permalink raw reply [flat|nested] 20+ messages in thread
[parent not found: <20121214024938.GA6591-0o1r3XBGOEbbgkc5XkKeNuvMHUBZFtU3YPYVAmT7z5s@public.gmane.org>]
* Re: [PATCH] core_pattern: set core helpers root and namespace to crashing process 2012-12-14 2:49 ` Neil Horman @ 2012-12-14 9:04 ` Daniel P. Berrange -1 siblings, 0 replies; 20+ messages in thread From: Daniel P. Berrange @ 2012-12-14 9:04 UTC (permalink / raw) To: Neil Horman Cc: Andrew Morton, containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Alexander Viro On Thu, Dec 13, 2012 at 09:49:39PM -0500, Neil Horman wrote: > On Thu, Dec 13, 2012 at 02:25:34PM -0800, Andrew Morton wrote: > > On Thu, 13 Dec 2012 13:12:20 -0500 > > Neil Horman <nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org> wrote: > > > > > On Thu, Dec 13, 2012 at 06:20:48AM -0600, Serge Hallyn wrote: > > > > Quoting Neil Horman (nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org): > > > > > Theres one problem I currently see with it, and that is that I'm not sure we can > > > > > change the current behavior of how the root fs is set for the pipe reader, lest > > > > > we break some user space expectations. As such, I've added a sysctl in this > > > > > patch to allow administrators to globally select if a core reader specified via > > > > > /proc/sys/kernel/core_pattern should use the global rootfs, or the (possibly) > > > > > chrooted fs of the crashing process. > > > > > > > > Practical question: How is the admin to make an educated decision on > > > > how to set the sysctl? > > > > By reading the documentation which Neil didn't include? > > > Yeah, that was stupid of me, I'll respin this with docs. > > > > My thought was that the admin typically wouldn't touch this at all. I really > > > added it as a backwards compatibility option only. Setting the user space > > > helper task to the root of the crashing parent has the possibility of breaking > > > existing installs because the core_pattern helper might be expecting global file > > > system access. Moving forward, my expectation would be that core_pattern > > > helpers would be written with the default setting in mind, and we could > > > eventually deprecate the control entirely. > > > > > > If you have a better mechanism in mind however (or if you think that removing > > > the control is a resaonable approach), I'm certainly open to that. > > > > Yeah, this is a tiresome patch but I can't think of a better way. > > > > Except, perhaps, adding a new token to the core_pattern which says > > "switch namespaces"? > > > I like that idea, perhaps '||' instead of '|' as the leading token can indicate > "use the namespace root" vs. "use the global root". Thoughts? That seems like a nicer idea to me, than the extra sysctl knob. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] core_pattern: set core helpers root and namespace to crashing process @ 2012-12-14 9:04 ` Daniel P. Berrange 0 siblings, 0 replies; 20+ messages in thread From: Daniel P. Berrange @ 2012-12-14 9:04 UTC (permalink / raw) To: Neil Horman Cc: Andrew Morton, Serge Hallyn, linux-kernel, Alexander Viro, containers On Thu, Dec 13, 2012 at 09:49:39PM -0500, Neil Horman wrote: > On Thu, Dec 13, 2012 at 02:25:34PM -0800, Andrew Morton wrote: > > On Thu, 13 Dec 2012 13:12:20 -0500 > > Neil Horman <nhorman@tuxdriver.com> wrote: > > > > > On Thu, Dec 13, 2012 at 06:20:48AM -0600, Serge Hallyn wrote: > > > > Quoting Neil Horman (nhorman@tuxdriver.com): > > > > > Theres one problem I currently see with it, and that is that I'm not sure we can > > > > > change the current behavior of how the root fs is set for the pipe reader, lest > > > > > we break some user space expectations. As such, I've added a sysctl in this > > > > > patch to allow administrators to globally select if a core reader specified via > > > > > /proc/sys/kernel/core_pattern should use the global rootfs, or the (possibly) > > > > > chrooted fs of the crashing process. > > > > > > > > Practical question: How is the admin to make an educated decision on > > > > how to set the sysctl? > > > > By reading the documentation which Neil didn't include? > > > Yeah, that was stupid of me, I'll respin this with docs. > > > > My thought was that the admin typically wouldn't touch this at all. I really > > > added it as a backwards compatibility option only. Setting the user space > > > helper task to the root of the crashing parent has the possibility of breaking > > > existing installs because the core_pattern helper might be expecting global file > > > system access. Moving forward, my expectation would be that core_pattern > > > helpers would be written with the default setting in mind, and we could > > > eventually deprecate the control entirely. > > > > > > If you have a better mechanism in mind however (or if you think that removing > > > the control is a resaonable approach), I'm certainly open to that. > > > > Yeah, this is a tiresome patch but I can't think of a better way. > > > > Except, perhaps, adding a new token to the core_pattern which says > > "switch namespaces"? > > > I like that idea, perhaps '||' instead of '|' as the leading token can indicate > "use the namespace root" vs. "use the global root". Thoughts? That seems like a nicer idea to me, than the extra sysctl knob. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] core_pattern: set core helpers root and namespace to crashing process 2012-12-13 12:20 ` Serge Hallyn (?) (?) @ 2012-12-13 18:12 ` Neil Horman -1 siblings, 0 replies; 20+ messages in thread From: Neil Horman @ 2012-12-13 18:12 UTC (permalink / raw) To: Serge Hallyn Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, Andrew Morton, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Alexander Viro On Thu, Dec 13, 2012 at 06:20:48AM -0600, Serge Hallyn wrote: > Quoting Neil Horman (nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org): > > Theres one problem I currently see with it, and that is that I'm not sure we can > > change the current behavior of how the root fs is set for the pipe reader, lest > > we break some user space expectations. As such, I've added a sysctl in this > > patch to allow administrators to globally select if a core reader specified via > > /proc/sys/kernel/core_pattern should use the global rootfs, or the (possibly) > > chrooted fs of the crashing process. > > Practical question: How is the admin to make an educated decision on > how to set the sysctl? > My thought was that the admin typically wouldn't touch this at all. I really added it as a backwards compatibility option only. Setting the user space helper task to the root of the crashing parent has the possibility of breaking existing installs because the core_pattern helper might be expecting global file system access. Moving forward, my expectation would be that core_pattern helpers would be written with the default setting in mind, and we could eventually deprecate the control entirely. If you have a better mechanism in mind however (or if you think that removing the control is a resaonable approach), I'm certainly open to that. Neil > -serge > ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2] core_pattern: set core helpers root and namespace to crashing process [not found] ` <1355255996-25953-1-git-send-email-nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org> 2012-12-13 12:20 ` Serge Hallyn @ 2012-12-14 21:04 ` Neil Horman 1 sibling, 0 replies; 20+ messages in thread From: Neil Horman @ 2012-12-14 21:04 UTC (permalink / raw) To: linux-kernel-u79uwXL29TY76Z2rM5mHXA Cc: Neil Horman, containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, Alexander Viro, Andrew Morton As its currently implemented, redirection of core dumps to a pipe reader should be executed such that the reader runs in the namespace of the crashing process, and it currently does not. This is the only sane way to deal with namespaces properly it seems to me, and this patch implements that functionality. Theres one problem I currently see with it, and that is that I'm not sure we can change the current behavior of how the root fs is set for the pipe reader, lest we break some user space expectations. As such I've made the switching of namespaces configurable based on the addition of an extra '|' token in the core_pattern sysctl. 1 '|' indicates a pipe using the global namespace, while 2 '|' indicates that the namespace and root of the crashing process should be used. I've tested this using both settings for the new sysctl, and it works well. For the sake of history, this was discussed before: http://www.spinics.net/lists/linux-containers/msg21531.html It seems there was some modicum of consensus as to how this should work, but there was no action taken on it. Signed-off-by: Neil Horman <nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org> Reported-by: Daniel Berrange <berrange-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> CC: Daniel Berrange <berrange-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> CC: Alexander Viro <viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn@public.gmane.org> CC: Andrew Morton <akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org> CC: Serge Hallyn <serge.hallyn-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org> CC: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org --- Documentation/sysctl/kernel.txt | 7 ++++++- fs/coredump.c | 23 +++++++++++++++++++++++ include/linux/binfmts.h | 2 ++ 3 files changed, 31 insertions(+), 1 deletion(-) diff --git a/Documentation/sysctl/kernel.txt b/Documentation/sysctl/kernel.txt index 2907ba6..f853fca 100644 --- a/Documentation/sysctl/kernel.txt +++ b/Documentation/sysctl/kernel.txt @@ -191,7 +191,12 @@ core_pattern is used to specify a core dumpfile pattern name. %<OTHER> both are dropped . If the first character of the pattern is a '|', the kernel will treat the rest of the pattern as a command to run. The core dump will be - written to the standard input of that program instead of to a file. + written to the standard input of that program instead of to a file. Note that + when using |, the core pipe reader that is executed will be run in the global + namespace and root filesystem. If two | tokens (i.e. ||) are supplied as the + first two characters of the core_pattern sysctl, the kernel will preform the + same pipe operation, but the core pipe reader will be executed using the + namespace and root fs of the crashing process. ============================================================== diff --git a/fs/coredump.c b/fs/coredump.c index ce47379..d4b71c0 100644 --- a/fs/coredump.c +++ b/fs/coredump.c @@ -164,6 +164,18 @@ static int format_corename(struct core_name *cn, struct coredump_params *cprm) if (!cn->corename) return -ENOMEM; + if (ispipe) { + /* + * If we have 2 | tokens at the head of core_pattern, it + * indicates we are a pipe and the reader should inherit the + * namespaces of the crashing process + */ + cprm->switch_ns = (*(pat_ptr+1) == '|') ? true : false; + if (cprm->switch_ns) + /* Advance pat_ptr so as not to mess up corename */ + pat_ptr++; + } + /* Repeat as long as we have more pattern to process and more output space */ while (*pat_ptr) { @@ -443,6 +455,7 @@ static void wait_for_dump_helpers(struct file *file) static int umh_pipe_setup(struct subprocess_info *info, struct cred *new) { struct file *files[2]; + struct path root; struct coredump_params *cp = (struct coredump_params *)info->data; int err = create_pipe_files(files, 0); if (err) @@ -455,6 +468,14 @@ static int umh_pipe_setup(struct subprocess_info *info, struct cred *new) /* and disallow core files too */ current->signal->rlim[RLIMIT_CORE] = (struct rlimit){1, 1}; + + if (cp->switch_ns) { + get_fs_root(cp->cprocess->fs, &root); + set_fs_root(current->fs, &root); + switch_task_namespaces(current, cp->cprocess->nsproxy); + } + + return err; } @@ -476,6 +497,8 @@ void do_coredump(siginfo_t *siginfo, struct pt_regs *regs) .siginfo = siginfo, .regs = regs, .limit = rlimit(RLIMIT_CORE), + .cprocess = current, + .switch_ns = false, /* * We must use the same mm->flags while dumping core to avoid * inconsistency of bit flags, since this flag is not protected diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h index cfcc6bf..3f06261 100644 --- a/include/linux/binfmts.h +++ b/include/linux/binfmts.h @@ -61,6 +61,8 @@ struct coredump_params { siginfo_t *siginfo; struct pt_regs *regs; struct file *file; + struct task_struct *cprocess; + bool switch_ns; unsigned long limit; unsigned long mm_flags; }; -- 1.7.11.7 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH] core_pattern: set core helpers root and namespace to crashing process @ 2012-12-11 19:59 Neil Horman 0 siblings, 0 replies; 20+ messages in thread From: Neil Horman @ 2012-12-11 19:59 UTC (permalink / raw) To: linux-kernel-u79uwXL29TY76Z2rM5mHXA Cc: Neil Horman, containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, Alexander Viro, Andrew Morton As its currently implemented, redirection of core dumps to a pipe reader should be executed such that the reader runs in the namespace of the crashing process, and it currently does not. This is the only sane way to deal with namespaces properly it seems to me, and this patch implements that functionality. Theres one problem I currently see with it, and that is that I'm not sure we can change the current behavior of how the root fs is set for the pipe reader, lest we break some user space expectations. As such, I've added a sysctl in this patch to allow administrators to globally select if a core reader specified via /proc/sys/kernel/core_pattern should use the global rootfs, or the (possibly) chrooted fs of the crashing process. I've tested this using both settings for the new sysctl, and it works well. For the sake of history, this was discussed before: http://www.spinics.net/lists/linux-containers/msg21531.html It seems there was some modicum of consensus as to how this should work, but there was no action taken on it. Signed-off-by: Neil Horman <nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org> Reported-by: Daniel Berrange <berrange-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> CC: Daniel Berrange <berrange-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> CC: Alexander Viro <viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn@public.gmane.org> CC: Andrew Morton <akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org> CC: Serge Hallyn <serge.hallyn-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org> CC: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org --- fs/coredump.c | 11 +++++++++++ include/linux/binfmts.h | 1 + kernel/sysctl.c | 8 ++++++++ 3 files changed, 20 insertions(+) diff --git a/fs/coredump.c b/fs/coredump.c index ce47379..e7d8ca2 100644 --- a/fs/coredump.c +++ b/fs/coredump.c @@ -47,6 +47,7 @@ int core_uses_pid; char core_pattern[CORENAME_MAX_SIZE] = "core"; unsigned int core_pipe_limit; +unsigned int core_pipe_global_root; struct core_name { char *corename; @@ -443,6 +444,7 @@ static void wait_for_dump_helpers(struct file *file) static int umh_pipe_setup(struct subprocess_info *info, struct cred *new) { struct file *files[2]; + struct path root; struct coredump_params *cp = (struct coredump_params *)info->data; int err = create_pipe_files(files, 0); if (err) @@ -455,6 +457,14 @@ static int umh_pipe_setup(struct subprocess_info *info, struct cred *new) /* and disallow core files too */ current->signal->rlim[RLIMIT_CORE] = (struct rlimit){1, 1}; + + if (!core_pipe_global_root) { + get_fs_root(cp->cprocess->fs, &root); + set_fs_root(current->fs, &root); + } + + switch_task_namespaces(current, cp->cprocess->nsproxy); + return err; } @@ -476,6 +486,7 @@ void do_coredump(siginfo_t *siginfo, struct pt_regs *regs) .siginfo = siginfo, .regs = regs, .limit = rlimit(RLIMIT_CORE), + .cprocess = current, /* * We must use the same mm->flags while dumping core to avoid * inconsistency of bit flags, since this flag is not protected diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h index cfcc6bf..08e7bcb 100644 --- a/include/linux/binfmts.h +++ b/include/linux/binfmts.h @@ -61,6 +61,7 @@ struct coredump_params { siginfo_t *siginfo; struct pt_regs *regs; struct file *file; + struct task_struct *cprocess; unsigned long limit; unsigned long mm_flags; }; diff --git a/kernel/sysctl.c b/kernel/sysctl.c index 26f65ea..be7b69d 100644 --- a/kernel/sysctl.c +++ b/kernel/sysctl.c @@ -102,6 +102,7 @@ extern int suid_dumpable; extern int core_uses_pid; extern char core_pattern[]; extern unsigned int core_pipe_limit; +extern unsigned int core_pipe_global_root; #endif extern int pid_max; extern int min_free_kbytes; @@ -430,6 +431,13 @@ static struct ctl_table kern_table[] = { .mode = 0644, .proc_handler = proc_dointvec, }, + { + .procname = "core_pipe_global_root", + .data = &core_pipe_global_root, + .maxlen = sizeof(unsigned int), + .mode = 0644, + .proc_handler = proc_dointvec, + }, #endif #ifdef CONFIG_PROC_SYSCTL { -- 1.7.11.7 ^ permalink raw reply related [flat|nested] 20+ messages in thread
end of thread, other threads:[~2012-12-15 0:50 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-12-11 19:59 [PATCH] core_pattern: set core helpers root and namespace to crashing process Neil Horman
2012-12-14 21:04 ` [PATCH v2] " Neil Horman
[not found] ` <1355519048-28473-1-git-send-email-nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
2012-12-14 21:49 ` Andrew Morton
2012-12-14 21:49 ` Andrew Morton
2012-12-14 23:10 ` Eric W. Biederman
2012-12-14 23:10 ` Eric W. Biederman
[not found] ` <87d2ycut5l.fsf-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
2012-12-15 0:50 ` Neil Horman
2012-12-15 0:50 ` Neil Horman
[not found] ` <1355255996-25953-1-git-send-email-nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
2012-12-13 12:20 ` [PATCH] " Serge Hallyn
2012-12-13 12:20 ` Serge Hallyn
2012-12-13 18:12 ` Neil Horman
[not found] ` <20121213181220.GB14796-B26myB8xz7F8NnZeBjwnZQMhkBWG/bsMQH7oEaQurus@public.gmane.org>
2012-12-13 22:25 ` Andrew Morton
2012-12-13 22:25 ` Andrew Morton
[not found] ` <20121213142534.ea07a3c8.akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
2012-12-14 2:49 ` Neil Horman
2012-12-14 2:49 ` Neil Horman
[not found] ` <20121214024938.GA6591-0o1r3XBGOEbbgkc5XkKeNuvMHUBZFtU3YPYVAmT7z5s@public.gmane.org>
2012-12-14 9:04 ` Daniel P. Berrange
2012-12-14 9:04 ` Daniel P. Berrange
2012-12-13 18:12 ` Neil Horman
2012-12-14 21:04 ` [PATCH v2] " Neil Horman
-- strict thread matches above, loose matches on Subject: below --
2012-12-11 19:59 [PATCH] " Neil Horman
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.