Moved core dump functionality into its own file
Subsystems (4)
PATCH 1/2 Moved core dump functionality into its own file
From: Alex Kelly <[email protected]>
This was done in preparation for making core dump functionality optional.
The variable "suid_dumpable" and associated functions are left in fs/exec.c because they're used elsewhere, such as in ptrace.
Signed-off-by Alex Kelly <[email protected]>
Reviewed-by Josh Triplett <[email protected]>
--- fs/Makefile | 2 +- fs/coredump.c | 689 ++++++++++++++++++++++++++++++++++++++++++++++++++ fs/exec.c | 647 +---------------------------------------------- include/linux/sched.h | 1 + 4 files changed, 692 insertions(+), 647 deletions(-) create mode 100644 fs/coredump.c
fs/Makefile +1 -1
@@ -11,7 +11,7 @@ obj-y := open.o read_write.o file_table.o super.o \ attr.o bad_inode.o file.o filesystems.o namespace.o \ seq_file.o xattr.o libfs.o fs-writeback.o \ pnode.o drop_caches.o splice.o sync.o utimes.o \ - stack.o fs_struct.o statfs.o + stack.o fs_struct.o statfs.o coredump.o ifeq ($(CONFIG_BLOCK),y) obj-y += buffer.o bio.o block_dev.o direct-io.o mpage.o ioprio.o
fs/coredump.c new file +689 -0
689 lines not on this page · raw message
fs/exec.c +1 -646
670 lines not on this page · raw message
include/linux/sched.h +1 -0
@@ -413,6 +413,7 @@ static inline void arch_pick_mmap_layout(struct mm_struct *mm) {}extern void set_dumpable (struct mm_struct *mm,int value);extern int get_dumpable (struct mm_struct *mm); +extern int __get_dumpable (unsigned long mm_flags);/* get/set_dumpable() values */ #define SUID_DUMPABLE_DISABLED 0
Signature · 1 line
1.7.11.2
PATCH 2/2 Made core dump functionality optional
From: Alex <[email protected]>
Adds an expert Kconfig option, CONFIG_COREDUMP, which allows disabling of core dump. This saves approximately 2.6k in the compiled kernel, and complements CONFIG_ELF_CORE, which now depends on it.
CONFIG_COREDUMP also disables coredump-related sysctls, except for suid_dumpable and related functions, which are necessary for ptrace.
Signed-off-by Alex Kelly <[email protected]>
Reviewed-by Josh Triplett <[email protected]>
--- fs/Kconfig.binfmt | 8 ++++++++ fs/Makefile | 3 ++- include/linux/binfmts.h | 4 ++++ init/Kconfig | 1 + kernel/sysctl.c | 6 +++++- 5 files changed, 20 insertions(+), 2 deletions(-)
fs/Kconfig.binfmt +8 -0
@@ -164,3 +164,11 @@ config BINFMT_MISC You may say M here for module support and later load the module when you have use for it; the module is called binfmt_misc. If you don't know what to answer at this point, say Y. + +config COREDUMP + bool "Enable core dump support" if EXPERT + default y + help + This option enables support for performing core dumps. You almost + certainly want to say Y here. Not necessary on systems that never + need debugging or only ever run flawless code.
fs/Makefile +2 -1
@@ -11,7 +11,7 @@ obj-y := open.o read_write.o file_table.o super.o \ attr.o bad_inode.o file.o filesystems.o namespace.o \ seq_file.o xattr.o libfs.o fs-writeback.o \ pnode.o drop_caches.o splice.o sync.o utimes.o \ - stack.o fs_struct.o statfs.o coredump.o + stack.o fs_struct.o statfs.o ifeq ($(CONFIG_BLOCK),y) obj-y += buffer.o bio.o block_dev.o direct-io.o mpage.o ioprio.o
@@ -48,6 +48,7 @@ obj-$(CONFIG_FS_MBCACHE) += mbcache.o obj-$(CONFIG_FS_POSIX_ACL) += posix_acl.o xattr_acl.o obj-$(CONFIG_NFS_COMMON) += nfs_common/ obj-$(CONFIG_GENERIC_ACL) += generic_acl.o +obj-$(CONFIG_COREDUMP) += coredump.o obj-$(CONFIG_FHANDLE) += fhandle.o
include/linux/binfmts.h +4 -0
@@ -132,7 +132,11 @@ extern int copy_strings_kernel(int argc, const char *const *argv, struct linux_binprm *bprm);extern int prepare_bprm_creds (struct linux_binprm *bprm);extern void install_exec_creds (struct linux_binprm *bprm); +#ifdef CONFIG_COREDUMP extern void do_coredump (long signr,int exit_code,struct pt_regs *regs); +#else +static inline void do_coredump (long signr,int exit_code,struct pt_regs *regs) {} +#endif extern void set_binfmt (struct linux_binfmt *new);extern void free_bprm (struct linux_binprm *);
init/Kconfig +1 -0
@@ -1230,6 +1230,7 @@ config BUG Just say Y. config ELF_CORE + depends on COREDUMP default y bool "Enable ELF core dumps" if EXPERT help
kernel/sysctl.c +5 -1
@@ -97,10 +97,12 @@extern int sysctl_overcommit_memory;extern int sysctl_overcommit_ratio;extern int max_threads; -extern int core_uses_pid;extern int suid_dumpable; +#ifdef CONFIG_COREDUMP +extern int core_uses_pid;extern char core_pattern[];extern unsigned int core_pipe_limit; +#endif extern int pid_max;extern int min_free_kbytes;extern int pid_max_min, pid_max_max;
@@ -404,6 +406,7 @@ static struct ctl_table kern_table[] = { .mode = 0644, .proc_handler = proc_dointvec, }, +#ifdef CONFIG_COREDUMP { .procname = "core_uses_pid", .data = &core_uses_pid,
@@ -425,6 +428,7 @@ static struct ctl_table kern_table[] = { .mode = 0644, .proc_handler = proc_dointvec, }, +#endif #ifdef CONFIG_PROC_SYSCTL { .procname = "tainted",
Signature · 1 line
1.7.11.2
Quoted 3 lines · Alex Kelly
From: Alex Kelly <[email protected]>
This was done in preparation for making core dump functionality optional.
Please use present tense and a sane title, something like:
fs: Move core dump functionality into its own file
fs: Make core dump functionality optional
While looking at that code, there's also a few uglies in it, like:
Quoted 18 lines
fs/coredump.c new file +12
-0+return nr; +} + + +/* + +/* Repeat as long as we have more pattern to process and more output +space */ +} + + +void do_coredump (long signr,int exit_code,struct pt_regs *regs)
include/linux/sched.h static inline void arch_pick_mmap_layout(struct mm_struct *mm) {} · line 416
Quoted 4 lines
include/linux/sched.h +1
-0@@ -413,6 +413,7 @@ static inline void arch_pick_mmap_layout(struct mm_struct *mm) {}extern void set_dumpable (struct mm_struct *mm,int value);extern int get_dumpable (struct mm_struct *mm); +extern int __get_dumpable (unsigned long mm_flags);
These prototypes should move out of sched.h and into a coredump.h header or so.
If we are touching this code lets use the opportunity and do this right.
Note, I'd suggest to put any such further changes into separate, additional patches at the end: a cleanup patch, a header file introduction patch, etc. - and keep the "dumb code movement" chnage in the initial patch. This will make it all much easier to review.
Please also review the code for more uglies, the above was just a very quick stylistic scan.
Thanks,
Ingo
PATCH 1/4 fs: Move core dump functionality into its own file
In reply to Alex KellyFrom: Alex Kelly <[email protected]>
This prepares for making core dump functionality optional.
The variable "suid_dumpable" and associated functions are left in fs/exec.c because they're used elsewhere, such as in ptrace.
Signed-off-by Alex Kelly <[email protected]>
Reviewed-by Josh Triplett <[email protected]>
--- fs/Makefile | 2 +- fs/coredump.c | 689 ++++++++++++++++++++++++++++++++++++++++++++++++++ fs/exec.c | 647 +---------------------------------------------- include/linux/sched.h | 1 + 4 files changed, 692 insertions(+), 647 deletions(-) create mode 100644 fs/coredump.c
fs/Makefile +1 -1
@@ -11,7 +11,7 @@ obj-y := open.o read_write.o file_table.o super.o \ attr.o bad_inode.o file.o filesystems.o namespace.o \ seq_file.o xattr.o libfs.o fs-writeback.o \ pnode.o drop_caches.o splice.o sync.o utimes.o \ - stack.o fs_struct.o statfs.o + stack.o fs_struct.o statfs.o coredump.o ifeq ($(CONFIG_BLOCK),y) obj-y += buffer.o bio.o block_dev.o direct-io.o mpage.o ioprio.o
fs/coredump.c new file +689 -0
689 lines not on this page · raw message
fs/exec.c +1 -646
670 lines not on this page · raw message
include/linux/sched.h +1 -0
@@ -413,6 +413,7 @@ static inline void arch_pick_mmap_layout(struct mm_struct *mm) {}extern void set_dumpable (struct mm_struct *mm,int value);extern int get_dumpable (struct mm_struct *mm); +extern int __get_dumpable (unsigned long mm_flags);/* get/set_dumpable() values */ #define SUID_DUMPABLE_DISABLED 0
Signature · 1 line
1.7.11.2
PATCH 2/4 fs: Make core dump functionality optional
From: Alex <[email protected]>
Adds an expert Kconfig option, CONFIG_COREDUMP, which allows disabling of core dump. This saves approximately 2.6k in the compiled kernel, and complements CONFIG_ELF_CORE, which now depends on it.
CONFIG_COREDUMP also disables coredump-related sysctls, except for suid_dumpable and related functions, which are necessary for ptrace.
Signed-off-by Alex Kelly <[email protected]>
Reviewed-by Josh Triplett <[email protected]>
--- fs/Kconfig.binfmt | 8 ++++++++ fs/Makefile | 3 ++- include/linux/binfmts.h | 4 ++++ init/Kconfig | 1 + kernel/sysctl.c | 6 +++++- 5 files changed, 20 insertions(+), 2 deletions(-)
fs/Kconfig.binfmt +8 -0
@@ -164,3 +164,11 @@ config BINFMT_MISC You may say M here for module support and later load the module when you have use for it; the module is called binfmt_misc. If you don't know what to answer at this point, say Y. + +config COREDUMP + bool "Enable core dump support" if EXPERT + default y + help + This option enables support for performing core dumps. You almost + certainly want to say Y here. Not necessary on systems that never + need debugging or only ever run flawless code.
fs/Makefile +2 -1
@@ -11,7 +11,7 @@ obj-y := open.o read_write.o file_table.o super.o \ attr.o bad_inode.o file.o filesystems.o namespace.o \ seq_file.o xattr.o libfs.o fs-writeback.o \ pnode.o drop_caches.o splice.o sync.o utimes.o \ - stack.o fs_struct.o statfs.o coredump.o + stack.o fs_struct.o statfs.o ifeq ($(CONFIG_BLOCK),y) obj-y += buffer.o bio.o block_dev.o direct-io.o mpage.o ioprio.o
@@ -48,6 +48,7 @@ obj-$(CONFIG_FS_MBCACHE) += mbcache.o obj-$(CONFIG_FS_POSIX_ACL) += posix_acl.o xattr_acl.o obj-$(CONFIG_NFS_COMMON) += nfs_common/ obj-$(CONFIG_GENERIC_ACL) += generic_acl.o +obj-$(CONFIG_COREDUMP) += coredump.o obj-$(CONFIG_FHANDLE) += fhandle.o
include/linux/binfmts.h +4 -0
@@ -132,7 +132,11 @@ extern int copy_strings_kernel(int argc, const char *const *argv, struct linux_binprm *bprm);extern int prepare_bprm_creds (struct linux_binprm *bprm);extern void install_exec_creds (struct linux_binprm *bprm); +#ifdef CONFIG_COREDUMP extern void do_coredump (long signr,int exit_code,struct pt_regs *regs); +#else +static inline void do_coredump (long signr,int exit_code,struct pt_regs *regs) {} +#endif extern void set_binfmt (struct linux_binfmt *new);extern void free_bprm (struct linux_binprm *);
init/Kconfig +1 -0
@@ -1230,6 +1230,7 @@ config BUG Just say Y. config ELF_CORE + depends on COREDUMP default y bool "Enable ELF core dumps" if EXPERT help
kernel/sysctl.c +5 -1
@@ -97,10 +97,12 @@extern int sysctl_overcommit_memory;extern int sysctl_overcommit_ratio;extern int max_threads; -extern int core_uses_pid;extern int suid_dumpable; +#ifdef CONFIG_COREDUMP +extern int core_uses_pid;extern char core_pattern[];extern unsigned int core_pipe_limit; +#endif extern int pid_max;extern int min_free_kbytes;extern int pid_max_min, pid_max_max;
@@ -404,6 +406,7 @@ static struct ctl_table kern_table[] = { .mode = 0644, .proc_handler = proc_dointvec, }, +#ifdef CONFIG_COREDUMP { .procname = "core_uses_pid", .data = &core_uses_pid,
@@ -425,6 +428,7 @@ static struct ctl_table kern_table[] = { .mode = 0644, .proc_handler = proc_dointvec, }, +#endif #ifdef CONFIG_PROC_SYSCTL { .procname = "tainted",
Signature · 1 line
1.7.11.2
PATCH 3/4 fs: Clean up some artifacts in coredump.c
In reply to [email protected]From: Alex Kelly <[email protected]>
Specifically, some whitespace got carried over from the move that shouldn't have, and there were some comment style issues in the original code that are now fixed
Signed-off-by Alex Kelly <[email protected]>
Reviewed-by Josh Triplett <[email protected]>
--- fs/coredump.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
fs/coredump.c +4 -4
@@ -70,7 +70,6 @@ static int zap_process(struct task_struct *start, int exit_code)return nr; } -/* * Core dumping helper functions. These are the only things you should * do on a core-file: use only these functions to write out all the
@@ -195,8 +194,10 @@ static int format_corename(struct core_name *cn, long signr)if (!cn->corename)return -ENOMEM ; -/* Repeat as long as we have more pattern to process and more output -space */ +/* +* Repeat as long as we have more pattern to process and more output +* space. +*/ while (*pat_ptr) {if (*pat_ptr !='%' ) {if (*pat_ptr ==0 )
@@ -471,7 +472,6 @@ static int umh_pipe_setup(struct subprocess_info *info, struct cred *new)return 0 ; } -void do_coredump (long signr,int exit_code,struct pt_regs *regs) {struct core_state core_state;
Signature · 1 line
1.7.11.2
PATCH 4/4 fs: Update coredump-related headers
In reply to [email protected]From: Alex Kelly <[email protected]>
This patch creates a new header file, fs/coredump.h, which contains functions only used by the new coredump.c. It also moves do_coredump to the /include/coredump.h header file, for consistency.
Signed-off-by Alex Kelly <[email protected]>
Reviewed-by Josh Triplett <[email protected]>
--- fs/coredump.c | 2 ++ fs/coredump.h | 6 ++++++ fs/exec.c | 1 + include/linux/binfmts.h | 5 ----- include/linux/coredump.h | 5 +++++ include/linux/sched.h | 2 -- kernel/signal.c | 1 + 7 files changed, 15 insertions(+), 7 deletions(-) create mode 100644 fs/coredump.h
fs/coredump.c +2 -0
@@ -14,6 +14,7 @@#include <linux/key.h> #include <linux/personality.h> #include <linux/binfmts.h> +#include <linux/coredump.h> #include <linux/utsname.h> #include <linux/pid_namespace.h> #include <linux/module.h>
@@ -39,6 +40,7 @@#include <trace/events/task.h> #include "internal.h" +#include "coredump.h" #include <trace/events/sched.h>
fs/coredump.h new file +6 -0
@@ -0,0 +1,6 @@ +#ifndef _FS_COREDUMP_H +#define _FS_COREDUMP_H + +extern int __get_dumpable (unsigned long mm_flags); + +#endif
fs/exec.c +1 -0
@@ -63,6 +63,7 @@#include <trace/events/task.h> #include "internal.h" +#include "coredump.h" #include <trace/events/sched.h>
include/linux/binfmts.h +0 -5
@@ -132,11 +132,6 @@ extern int copy_strings_kernel(int argc, const char *const *argv, struct linux_binprm *bprm);extern int prepare_bprm_creds (struct linux_binprm *bprm);extern void install_exec_creds (struct linux_binprm *bprm); -#ifdef CONFIG_COREDUMP -extern void do_coredump (long signr,int exit_code,struct pt_regs *regs); -#else -static inline void do_coredump (long signr,int exit_code,struct pt_regs *regs) {} -#endif extern void set_binfmt (struct linux_binfmt *new);extern void free_bprm (struct linux_binprm *);
include/linux/coredump.h +5 -0
@@ -11,5 +11,10 @@*/ extern int dump_write (struct file *file,const void *addr,int nr);extern int dump_seek (struct file *file,loff_t off); +#ifdef CONFIG_COREDUMP +extern void do_coredump (long signr,int exit_code,struct pt_regs *regs); +#else +static inline void do_coredump (long signr,int exit_code,struct pt_regs *regs) {} +#endif #endif /* _LINUX_COREDUMP_H */
include/linux/sched.h +0 -2
@@ -410,10 +410,8 @@ extern void arch_unmap_area_topdown(struct mm_struct *, unsigned long);static inline void arch_pick_mmap_layout (struct mm_struct *mm) {}#endif -extern void set_dumpable (struct mm_struct *mm,int value);extern int get_dumpable (struct mm_struct *mm); -extern int __get_dumpable (unsigned long mm_flags);/* get/set_dumpable() values */ #define SUID_DUMPABLE_DISABLED 0
kernel/signal.c +1 -0
@@ -17,6 +17,7 @@#include <linux/fs.h> #include <linux/tty.h> #include <linux/binfmts.h> +#include <linux/coredump.h> #include <linux/security.h> #include <linux/syscalls.h> #include <linux/ptrace.h>
Signature · 1 line
1.7.11.2
PATCH v3 1/4 fs: Move core dump functionality into its own file
In reply to Alex KellyThis prepares for making core dump functionality optional.
The variable "suid_dumpable" and associated functions are left in fs/exec.c because they're used elsewhere, such as in ptrace.
Signed-off-by Alex Kelly <[email protected]>
Reviewed-by Josh Triplett <[email protected]>
---
v2: This patch set is a second revision that follows some suggestions from Ingo Molnar and Josh Triplett. Specifically, authorship of commits is revised for consistency, and an additional two patches cleaning up artifacts and making headers more sane are added.
v3: This version fixes a few more authorship issues and some problems caused by a bad git send-email config. Sorry about the extra mails
fs/Makefile | 2 +- fs/coredump.c | 689 ++++++++++++++++++++++++++++++++++++++++++++++++++ fs/exec.c | 647 +---------------------------------------------- include/linux/sched.h | 1 + 4 files changed, 692 insertions(+), 647 deletions(-) create mode 100644 fs/coredump.c
fs/Makefile +1 -1
@@ -11,7 +11,7 @@ obj-y := open.o read_write.o file_table.o super.o \ attr.o bad_inode.o file.o filesystems.o namespace.o \ seq_file.o xattr.o libfs.o fs-writeback.o \ pnode.o drop_caches.o splice.o sync.o utimes.o \ - stack.o fs_struct.o statfs.o + stack.o fs_struct.o statfs.o coredump.o ifeq ($(CONFIG_BLOCK),y) obj-y += buffer.o bio.o block_dev.o direct-io.o mpage.o ioprio.o
fs/coredump.c new file +689 -0
689 lines not on this page · raw message
fs/exec.c +1 -646
670 lines not on this page · raw message
include/linux/sched.h +1 -0
@@ -413,6 +413,7 @@ static inline void arch_pick_mmap_layout(struct mm_struct *mm) {}extern void set_dumpable (struct mm_struct *mm,int value);extern int get_dumpable (struct mm_struct *mm); +extern int __get_dumpable (unsigned long mm_flags);/* get/set_dumpable() values */ #define SUID_DUMPABLE_DISABLED 0
Signature · 1 line
1.7.11.2
PATCH v3 2/4 fs: Make core dump functionality optional
Adds an expert Kconfig option, CONFIG_COREDUMP, which allows disabling of core dump. This saves approximately 2.6k in the compiled kernel, and complements CONFIG_ELF_CORE, which now depends on it.
CONFIG_COREDUMP also disables coredump-related sysctls, except for suid_dumpable and related functions, which are necessary for ptrace.
Signed-off-by Alex Kelly <[email protected]>
Reviewed-by Josh Triplett <[email protected]>
--- fs/Kconfig.binfmt | 8 ++++++++ fs/Makefile | 3 ++- include/linux/binfmts.h | 4 ++++ init/Kconfig | 1 + kernel/sysctl.c | 6 +++++- 5 files changed, 20 insertions(+), 2 deletions(-)
fs/Kconfig.binfmt +8 -0
@@ -164,3 +164,11 @@ config BINFMT_MISC You may say M here for module support and later load the module when you have use for it; the module is called binfmt_misc. If you don't know what to answer at this point, say Y. + +config COREDUMP + bool "Enable core dump support" if EXPERT + default y + help + This option enables support for performing core dumps. You almost + certainly want to say Y here. Not necessary on systems that never + need debugging or only ever run flawless code.
fs/Makefile +2 -1
@@ -11,7 +11,7 @@ obj-y := open.o read_write.o file_table.o super.o \ attr.o bad_inode.o file.o filesystems.o namespace.o \ seq_file.o xattr.o libfs.o fs-writeback.o \ pnode.o drop_caches.o splice.o sync.o utimes.o \ - stack.o fs_struct.o statfs.o coredump.o + stack.o fs_struct.o statfs.o ifeq ($(CONFIG_BLOCK),y) obj-y += buffer.o bio.o block_dev.o direct-io.o mpage.o ioprio.o
@@ -48,6 +48,7 @@ obj-$(CONFIG_FS_MBCACHE) += mbcache.o obj-$(CONFIG_FS_POSIX_ACL) += posix_acl.o xattr_acl.o obj-$(CONFIG_NFS_COMMON) += nfs_common/ obj-$(CONFIG_GENERIC_ACL) += generic_acl.o +obj-$(CONFIG_COREDUMP) += coredump.o obj-$(CONFIG_FHANDLE) += fhandle.o
include/linux/binfmts.h +4 -0
@@ -132,7 +132,11 @@ extern int copy_strings_kernel(int argc, const char *const *argv, struct linux_binprm *bprm);extern int prepare_bprm_creds (struct linux_binprm *bprm);extern void install_exec_creds (struct linux_binprm *bprm); +#ifdef CONFIG_COREDUMP extern void do_coredump (long signr,int exit_code,struct pt_regs *regs); +#else +static inline void do_coredump (long signr,int exit_code,struct pt_regs *regs) {} +#endif extern void set_binfmt (struct linux_binfmt *new);extern void free_bprm (struct linux_binprm *);
init/Kconfig +1 -0
@@ -1230,6 +1230,7 @@ config BUG Just say Y. config ELF_CORE + depends on COREDUMP default y bool "Enable ELF core dumps" if EXPERT help
kernel/sysctl.c +5 -1
@@ -97,10 +97,12 @@extern int sysctl_overcommit_memory;extern int sysctl_overcommit_ratio;extern int max_threads; -extern int core_uses_pid;extern int suid_dumpable; +#ifdef CONFIG_COREDUMP +extern int core_uses_pid;extern char core_pattern[];extern unsigned int core_pipe_limit; +#endif extern int pid_max;extern int min_free_kbytes;extern int pid_max_min, pid_max_max;
@@ -404,6 +406,7 @@ static struct ctl_table kern_table[] = { .mode = 0644, .proc_handler = proc_dointvec, }, +#ifdef CONFIG_COREDUMP { .procname = "core_uses_pid", .data = &core_uses_pid,
@@ -425,6 +428,7 @@ static struct ctl_table kern_table[] = { .mode = 0644, .proc_handler = proc_dointvec, }, +#endif #ifdef CONFIG_PROC_SYSCTL { .procname = "tainted",
Signature · 1 line
1.7.11.2
PATCH v3 3/4 fs: Clean up some artifacts in coredump.c
In reply to Alex KellySpecifically, some whitespace got carried over from the move that shouldn't have, and there were some comment style issues in the original code that are now fixed
Signed-off-by Alex Kelly <[email protected]>
Reviewed-by Josh Triplett <[email protected]>
--- fs/coredump.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
fs/coredump.c +4 -4
@@ -70,7 +70,6 @@ static int zap_process(struct task_struct *start, int exit_code)return nr; } -/* * Core dumping helper functions. These are the only things you should * do on a core-file: use only these functions to write out all the
@@ -195,8 +194,10 @@ static int format_corename(struct core_name *cn, long signr)if (!cn->corename)return -ENOMEM ; -/* Repeat as long as we have more pattern to process and more output -space */ +/* +* Repeat as long as we have more pattern to process and more output +* space. +*/ while (*pat_ptr) {if (*pat_ptr !='%' ) {if (*pat_ptr ==0 )
@@ -471,7 +472,6 @@ static int umh_pipe_setup(struct subprocess_info *info, struct cred *new)return 0 ; } -void do_coredump (long signr,int exit_code,struct pt_regs *regs) {struct core_state core_state;
Signature · 1 line
1.7.11.2
PATCH v3 4/4 fs: Update coredump-related headers
In reply to Alex KellyThis patch creates a new header file, fs/coredump.h, which contains functions only used by the new coredump.c. It also moves do_coredump to the include/linux/coredump.h header file, for consistency.
Signed-off-by Alex Kelly <[email protected]>
Reviewed-by Josh Triplett <[email protected]>
--- fs/coredump.c | 2 ++ fs/coredump.h | 6 ++++++ fs/exec.c | 1 + include/linux/binfmts.h | 5 ----- include/linux/coredump.h | 5 +++++ include/linux/sched.h | 2 -- kernel/signal.c | 1 + 7 files changed, 15 insertions(+), 7 deletions(-) create mode 100644 fs/coredump.h
fs/coredump.c +2 -0
@@ -14,6 +14,7 @@#include <linux/key.h> #include <linux/personality.h> #include <linux/binfmts.h> +#include <linux/coredump.h> #include <linux/utsname.h> #include <linux/pid_namespace.h> #include <linux/module.h>
@@ -39,6 +40,7 @@#include <trace/events/task.h> #include "internal.h" +#include "coredump.h" #include <trace/events/sched.h>
fs/coredump.h new file +6 -0
@@ -0,0 +1,6 @@ +#ifndef _FS_COREDUMP_H +#define _FS_COREDUMP_H + +extern int __get_dumpable (unsigned long mm_flags); + +#endif
fs/exec.c +1 -0
@@ -63,6 +63,7 @@#include <trace/events/task.h> #include "internal.h" +#include "coredump.h" #include <trace/events/sched.h>
include/linux/binfmts.h +0 -5
@@ -132,11 +132,6 @@ extern int copy_strings_kernel(int argc, const char *const *argv, struct linux_binprm *bprm);extern int prepare_bprm_creds (struct linux_binprm *bprm);extern void install_exec_creds (struct linux_binprm *bprm); -#ifdef CONFIG_COREDUMP -extern void do_coredump (long signr,int exit_code,struct pt_regs *regs); -#else -static inline void do_coredump (long signr,int exit_code,struct pt_regs *regs) {} -#endif extern void set_binfmt (struct linux_binfmt *new);extern void free_bprm (struct linux_binprm *);
include/linux/coredump.h +5 -0
@@ -11,5 +11,10 @@*/ extern int dump_write (struct file *file,const void *addr,int nr);extern int dump_seek (struct file *file,loff_t off); +#ifdef CONFIG_COREDUMP +extern void do_coredump (long signr,int exit_code,struct pt_regs *regs); +#else +static inline void do_coredump (long signr,int exit_code,struct pt_regs *regs) {} +#endif #endif /* _LINUX_COREDUMP_H */
include/linux/sched.h +0 -2
@@ -410,10 +410,8 @@ extern void arch_unmap_area_topdown(struct mm_struct *, unsigned long);static inline void arch_pick_mmap_layout (struct mm_struct *mm) {}#endif -extern void set_dumpable (struct mm_struct *mm,int value);extern int get_dumpable (struct mm_struct *mm); -extern int __get_dumpable (unsigned long mm_flags);/* get/set_dumpable() values */ #define SUID_DUMPABLE_DISABLED 0
kernel/signal.c +1 -0
@@ -17,6 +17,7 @@#include <linux/fs.h> #include <linux/tty.h> #include <linux/binfmts.h> +#include <linux/coredump.h> #include <linux/security.h> #include <linux/syscalls.h> #include <linux/ptrace.h>
Signature · 1 line
1.7.11.2
[NAK] Re: [PATCHv3 1/4] fs: Move core dump functionality into its own file
In reply to Alex KellyHi Alex,
Overall, this seems like a fine idea. However, I think the code move has gone very badly. Comments are mismatched, there are typos added, formatting changed, case changed, trailing white space added, and punctuation dropped.
NAKed-by Kees Cook <[email protected]>
Review
Quoted 2 lines · Alex Kelly
(quoted diff) +2
-0+/* The maximal length of core_pattern is also specified in sysctl.c */ +static int zap_process(struct task_struct *start, int exit_code)
These two lines have nothing to do with each other. This comment belongs with "char core_pattern[CORENAME_MAX_SIZE] = "core";", IIUC.
Why is zap_process not with the zap_threads helpers?
Review
Quoted 3 lines
(quoted diff) +3
-0+void do_coredump(long signr, int exit_code, struct pt_regs *regs) +[...] + * so we dump it as root in mode 2, and onlt into a controlled
"onlt"? The removed code didn't have this typo. Was the moved code not cut/pasted? That got my attention. In fact, I went and compared all the code that was moved, and this was not the only change -- what happened here?
tmp/add.code renamed from tmp/del.code +6 -6
@@ -327,7 +328,7 @@ core_waiters = zap_threads(tsk, mm, core_state, exit_code); up_write(&mm->mmap_sem); - if (core_waiters > 0) { + if (core_waiters > 0){ struct core_thread *ptr; wait_for_completion(&core_state->startup);
@@ -466,8 +467,8 @@ /* * We cannot trust fsuid as being the "true" uid of the process * nor do we know its entire history. We only know it was tainted - * so we dump it as root in mode 2, and only into a controlled - * environment (pipe handler or fully qualified path). + * so we dump it as root in mode 2, and onlt into a controlled + * environment (pipe handler or fully qualified path) */ if (__get_dumpable(cprm.mm_flags) == SUID_DUMPABLE_SAFE) { /* Setuid core dump mode */
@@ -505,11 +506,11 @@ * * Normally core limits are irrelevant to pipes, since * we're not writing to the file system, but we use - * cprm.limit of 1 here as a speacial value, this is a + * cprm.limit of 1 here as a speacial value, this is a * consistent way to catch recursive crashes. * We can still crash if the core_pattern binary sets * RLIM_CORE = !1, but it runs as root, and can do - * lots of stupid things. + * lots of stupid things * * Note that we use task_tgid_vnr here to grab the pid * of the process group leader. That way we get the
@@ -556,7 +557,7 @@ if (need_nonrelative && cn.corename[0] != '/') { printk(KERN_WARNING "Pid %d(%s) can only dump core "\ - "to fully qualified path!\n", + "To fully qualified path!\n", task_tgid_vnr(current), current->comm); printk(KERN_WARNING "Skipping core dump\n"); goto fail_unlock;
-Kees
Signature · 1 line
Kees Cook @outflux.net