coredump: core dump masking support v4

Hi,

This patch series is version 4 of the core dump masking feature, which provides a per-process flag not to dump anonymous shared memory segments.

In the previous version, the flag value was passed around the core dump functions as an argument to use the same setting while dumping. In this version, instead of doing that, a r/w semaphore prevents the setting from being changed while dumping.

This patch series can be applied against 2.6.20-mm2.
The supported core file formats are ELF and ELF-FDPIC. ELF has been tested, but ELF-FDPIC has not been built and tested because I don't have the test environment.

Background:
Some software programs share huge memory among hundreds of processes. If a failure occurs on one of these processes, they can be signaled by a monitoring process to generate core files and restart the service. However, it can develop into a system-wide failure such as system slow down for a long time and disk space shortage because the total size of the core files is very huge!

To avoid the above situation we can limit the core file size by setrlimit(2) or ulimit(1). But this method can lose important data such as stack because core dumping is terminated halfway. So I suggest keeping shared memory segments from being dumped for particular processes. Because the shared memory attached to processes is common in them, we don't need to dump the shared memory every time.

Usage:
Get all shared memory segments of pid 1234 not to dump:

  $ echo 1 > /proc/1234/coredump_omit_anonymous_shared

When a new process is created, the process inherits the flag status from its parent. It is useful to set the core dump flags before the program runs. For example:

  $ echo 1 > /proc/self/coredump_omit_anonymous_shared
  $ ./some_program

ChangeLog:
v4:
- in maydump(), retrieve the core dump setting from mm_struct directly, instead of its additional argument
- writing to /proc/<pid>/coredump_omit_anonymous_shared returns EBUSY while core dumping.

v3:
http://groups.google.com/group/linux.kernel/browse_frm/thread/706d2ae41c1cb2de/
- remove `/proc/<pid>/core_flags' proc entry
- add `/proc/<pid>/coredump_anonymous_shared' as a named flag
- remove kernel.core_flags_enable sysctl parameter

v2:
http://groups.google.com/group/linux.kernel/browse_frm/thread/cb254465971d4a42/
http://groups.google.com/group/linux.kernel/browse_frm/thread/da78f2702e06fa11/
- rename coremask' to core_flags'
- change core_flags' member in mm_struct to a bit field next to dumpable'
- introduce a global spin lock to protect adjacent two bit fields (core_flags and dumpable) from race condition
- fix a bug that the generated core file can be corrupted when core dumping and updating core_flags occur concurrently
- add kernel.core_flags_enable sysctl parameter to enable/disable flags in /proc/<pid>/core_flags
- support ELF-FDPIC binary format, but not tested

v1:
http://groups.google.com/group/linux.kernel/browse_frm/thread/1381fc54d716e3e6/

Hidehiro Kawai
Hitachi, Ltd., Systems Development Laboratory
E-mail: [email protected]

This patch adds an interface to set/reset a flag which determines anonymous shared memory segments should be dumped or not when a core file is generated.

/proc/<pid>/coredump_omit_anonymous_shared file is provided to access the flag. You can change the flag status for a particular process by writing to or reading from the file.

The flag status is inherited to the child process when it is created.

The flag is stored into coredump_omit_anon_shared member of mm_struct, which shares bytes with dumpable member because these two are adjacent bit fields. In order to avoid write-write race between the two, we use a global spin lock.

smp_wmb() at updating dumpable is removed because set_dumpable() includes a pair of spin lock and unlock which has the effect of memory barrier.

---
 fs/exec.c               |   12 ++--
 fs/proc/base.c          |  103 ++++++++++++++++++++++++++++++++++++++
 include/linux/binfmts.h |    4 +
 include/linux/sched.h   |   33 ++++++++++++
 kernel/fork.c           |    3 +
 kernel/sys.c            |   62 +++++++---------------
 security/commoncap.c    |    2 
 security/dummy.c        |    2 
 8 files changed, 174 insertions(+), 47 deletions(-)

Index: linux-2.6.20-mm2/fs/proc/base.c
===================================================================
@@ -74,6 +74,7 @@
 #include <linux/poll.h>
 #include <linux/nsproxy.h>
 #include <linux/oom.h>
+#include <linux/elf.h>
 #include "internal.h"
 
 /* NOTE:

@@ -1753,6 +1754,104 @@ static const struct inode_operations pro
 
 #endif
 
+#if defined(USE_ELF_CORE_DUMP) && defined(CONFIG_ELF_CORE)
+static ssize_t proc_coredump_omit_anon_shared_read(struct file *file,
+						   char __user *buf,
+						   size_t count,
+						   loff_t *ppos)
+{
+	struct task_struct *task = get_proc_task(file->f_dentry->d_inode);
+	struct mm_struct *mm;
+	char buffer[PROC_NUMBUF];
+	size_t len;
+	loff_t __ppos = *ppos;
+	int ret;
+
+	ret = -ESRCH;
+	if (!task)
+		goto out_no_task;
+
+	ret = 0;
+	mm = get_task_mm(task);
+	if (!mm)
+		goto out_no_mm;
+
+	len = snprintf(buffer, sizeof(buffer), "%u\n",
+		       mm->coredump_omit_anon_shared);
+	if (__ppos >= len)
+		goto out;
+	if (count > len - __ppos)
+		count = len - __ppos;
+
+	ret = -EFAULT;
+	if (copy_to_user(buf, buffer + __ppos, count))
+		goto out;
+
+	ret = count;
+	*ppos = __ppos + count;
+
+ out:
+	mmput(mm);
+ out_no_mm:
+	put_task_struct(task);
+ out_no_task:
+	return ret;
+}
+
+static ssize_t proc_coredump_omit_anon_shared_write(struct file *file,
+						    const char __user *buf,
+						    size_t count,
+						    loff_t *ppos)
+{
+	struct task_struct *task;
+	struct mm_struct *mm;
+	char buffer[PROC_NUMBUF], *end;
+	unsigned int val;
+	int ret;
+
+	ret = -EFAULT;
+	memset(buffer, 0, sizeof(buffer));
+	if (count > sizeof(buffer) - 1)
+		count = sizeof(buffer) - 1;
+	if (copy_from_user(buffer, buf, count))
+		goto out_no_task;
+
+	ret = -EINVAL;
+	val = (unsigned int)simple_strtoul(buffer, &end, 0);
+	if (*end == '\n')
+		end++;
+	if (end - buffer == 0)
+		goto out_no_task;
+
+	ret = -ESRCH;
+	task = get_proc_task(file->f_dentry->d_inode);
+	if (!task)
+		goto out_no_task;
+
+	ret = end - buffer;
+	mm = get_task_mm(task);
+	if (!mm)
+		goto out_no_mm;
+
+	if (down_write_trylock(&coredump_settings_sem)) {
+		set_coredump_omit_anon_shared(mm, (val != 0));
+		up_write(&coredump_settings_sem);
+	} else
+		ret = -EBUSY;
+
+	mmput(mm);
+ out_no_mm:
+	put_task_struct(task);
+ out_no_task:
+	return ret;
+}
+
+static struct file_operations proc_coredump_omit_anon_shared_operations = {
+	.read		= proc_coredump_omit_anon_shared_read,
+	.write		= proc_coredump_omit_anon_shared_write,
+};
+#endif
+
 /*
  * /proc/self:
  */

@@ -1972,6 +2071,10 @@ static struct pid_entry tgid_base_stuff[
 #ifdef CONFIG_FAULT_INJECTION
 	REG("make-it-fail", S_IRUGO|S_IWUSR, fault_inject),
 #endif
+#if defined(USE_ELF_CORE_DUMP) && defined(CONFIG_ELF_CORE)
+	REG("coredump_omit_anonymous_shared", S_IRUGO|S_IWUSR,
+	    coredump_omit_anon_shared),
+#endif
 #ifdef CONFIG_TASK_IO_ACCOUNTING
 	INF("io",	S_IRUGO, pid_io_accounting),
 #endif

Index: linux-2.6.20-mm2/include/linux/sched.h
===================================================================
@@ -366,7 +366,13 @@ struct mm_struct {
 	unsigned int token_priority;
 	unsigned int last_interval;
 
+	/*
+	 * Writing to these bit fields can cause race condition.  To avoid
+	 * the race, use dump_bits_lock.  You may also use set_dumpable() or
+	 * set_coredump_*() macros to set a value.
+	 */
 	unsigned char dumpable:2;
+	unsigned char coredump_omit_anon_shared:1;  /* don't dump anon shm */
 
 	/* coredumping support */
 	int core_waiters;

@@ -1727,6 +1733,33 @@ static inline void inc_syscw(struct task
 }
 #endif
 
+#include <linux/elf.h>
+/*
+ * These macros are used to protect dumpable and coredump_omit_anon_shared bit
+ * fields in mm_struct from write race between the two.
+ */
+extern spinlock_t dump_bits_lock;
+#define __set_dump_bits(dest, val) \
+	do {					\
+		spin_lock(&dump_bits_lock);	\
+		(dest) = (val);			\
+		spin_unlock(&dump_bits_lock);	\
+	} while (0)
+
+#if defined(USE_ELF_CORE_DUMP) && defined(CONFIG_ELF_CORE)
+# define set_dumpable(mm, val) \
+	__set_dump_bits((mm)->dumpable, val)
+# define set_coredump_omit_anon_shared(mm, val) \
+	__set_dump_bits((mm)->coredump_omit_anon_shared, val)
+#else
+# define set_dumpable(mm, val) \
+	do {				\
+		(mm)->dumpable = (val);	\
+		smp_wmb();		\
+	} while (0)
+# define set_coredump_omit_anon_shared(mm, val)
+#endif
+
 #endif /* __KERNEL__ */
 
 #endif

Index: linux-2.6.20-mm2/fs/exec.c
===================================================================
@@ -61,6 +61,8 @@
 int core_uses_pid;
 char core_pattern[128] = "core";
 int suid_dumpable = 0;
+DEFINE_SPINLOCK(dump_bits_lock);
+DECLARE_RWSEM(coredump_settings_sem);
 
 EXPORT_SYMBOL(suid_dumpable);
 /* The maximal length of core_pattern is also specified in sysctl.c */

@@ -853,9 +855,9 @@ int flush_old_exec(struct linux_binprm *
 	current->sas_ss_sp = current->sas_ss_size = 0;
 
 	if (current->euid == current->uid && current->egid == current->gid)
-		current->mm->dumpable = 1;
+		set_dumpable(current->mm, 1);
 	else
-		current->mm->dumpable = suid_dumpable;
+		set_dumpable(current->mm, suid_dumpable);
 
 	name = bprm->filename;
 

@@ -883,7 +885,7 @@ int flush_old_exec(struct linux_binprm *
 	    file_permission(bprm->file, MAY_READ) ||
 	    (bprm->interp_flags & BINPRM_FLAGS_ENFORCE_NONDUMP)) {
 		suid_keys(current);
-		current->mm->dumpable = suid_dumpable;
+		set_dumpable(current->mm, suid_dumpable);
 	}
 
 	/* An exec changes our domain. We are no longer part of the thread

@@ -1477,7 +1479,7 @@ int do_coredump(long signr, int exit_cod
 		flag = O_EXCL;		/* Stop rewrite attacks */
 		current->fsuid = 0;	/* Dump root private */
 	}
-	mm->dumpable = 0;
+	set_dumpable(mm, 0);
 
 	retval = coredump_wait(exit_code);
 	if (retval < 0)

@@ -1530,7 +1532,9 @@ int do_coredump(long signr, int exit_cod
 	if (!ispipe && do_truncate(file->f_path.dentry, 0, 0, file) != 0)
 		goto close_fail;
 
+	down_read(&coredump_settings_sem);
 	retval = binfmt->core_dump(signr, regs, file);
+	up_read(&coredump_settings_sem);
 
 	if (retval)
 		current->signal->group_exit_code |= 0x80;

Index: linux-2.6.20-mm2/kernel/fork.c
===================================================================
@@ -334,6 +334,9 @@ static struct mm_struct * mm_init(struct
 	atomic_set(&mm->mm_count, 1);
 	init_rwsem(&mm->mmap_sem);
 	INIT_LIST_HEAD(&mm->mmlist);
+	/* don't need to use set_coredump_omit_anon_shared() */
+	mm->coredump_omit_anon_shared =
+		(current->mm) ?	current->mm->coredump_omit_anon_shared : 0;
 	mm->core_waiters = 0;
 	mm->nr_ptes = 0;
 	set_mm_counter(mm, file_rss, 0);

Index: linux-2.6.20-mm2/security/commoncap.c
===================================================================
@@ -244,7 +244,7 @@ void cap_bprm_apply_creds (struct linux_
 
 	if (bprm->e_uid != current->uid || bprm->e_gid != current->gid ||
 	    !cap_issubset (new_permitted, current->cap_permitted)) {
-		current->mm->dumpable = suid_dumpable;
+		set_dumpable(current->mm, suid_dumpable);
 
 		if (unsafe & ~LSM_UNSAFE_PTRACE_CAP) {
 			if (!capable(CAP_SETUID)) {

Index: linux-2.6.20-mm2/security/dummy.c
===================================================================
@@ -130,7 +130,7 @@ static void dummy_bprm_free_security (st
 static void dummy_bprm_apply_creds (struct linux_binprm *bprm, int unsafe)
 {
 	if (bprm->e_uid != current->uid || bprm->e_gid != current->gid) {
-		current->mm->dumpable = suid_dumpable;
+		set_dumpable(current->mm, suid_dumpable);
 
 		if ((unsafe & ~LSM_UNSAFE_PTRACE_CAP) && !capable(CAP_SETUID)) {
 			bprm->e_uid = current->uid;

Index: linux-2.6.20-mm2/include/linux/binfmts.h
===================================================================
@@ -17,6 +17,8 @@ struct pt_regs;
 
 #ifdef __KERNEL__
 
+#include <linux/rwsem.h>
+
 /*
  * This structure is used to hold the arguments that are used when loading binaries.
  */

@@ -75,6 +77,8 @@ extern int suid_dumpable;
 #define SUID_DUMP_USER		1	/* Dump as user of process */
 #define SUID_DUMP_ROOT		2	/* Dump as root */
 
+extern struct rw_semaphore coredump_settings_sem;
+
 /* Stack area protections */
 #define EXSTACK_DEFAULT   0	/* Whatever the arch defaults to */
 #define EXSTACK_DISABLE_X 1	/* Disable executable stacks */

This patch enables to omit anonymous shared memory from an ELF formatted core file when it is generated.

---
 fs/binfmt_elf.c |   12 +++++++++---
 1 files changed, 9 insertions(+), 3 deletions(-)

Index: linux-2.6.20-mm2/fs/binfmt_elf.c
===================================================================
@@ -1191,9 +1191,15 @@ static int maydump(struct vm_area_struct
 	if (vma->vm_flags & (VM_IO | VM_RESERVED))
 		return 0;
 
-	/* Dump shared memory only if mapped from an anonymous file. */
-	if (vma->vm_flags & VM_SHARED)
-		return vma->vm_file->f_path.dentry->d_inode->i_nlink == 0;
+	/*
+	 * Dump shared memory only if mapped from an anonymous file and
+	 * /proc/<pid>/coredump_omit_anonymous_shared flag is not set.
+	 */
+	if (vma->vm_flags & VM_SHARED) {
+		if (vma->vm_file->f_path.dentry->d_inode->i_nlink)
+			return 0;
+		return vma->vm_mm->coredump_omit_anon_shared == 0;
+	}
 
 	/* If it hasn't been written to, don't write it out */
 	if (!vma->anon_vma)

This patch enables to omit anonymous shared memory from an ELF-FDPIC formatted core file when it is generated.

The debug messages from maydump() in fs/binfmt_elf_fdpic.c are changed appropriately so that we can know what kind of memory segments are dumped or not.

---
 fs/binfmt_elf_fdpic.c |   25 ++++++++++++++++---------
 1 files changed, 16 insertions(+), 9 deletions(-)

Index: linux-2.6.20-mm2/fs/binfmt_elf_fdpic.c
===================================================================
@@ -1168,7 +1168,7 @@ static int dump_seek(struct file *file, 
  *
  * I think we should skip something. But I am not sure how. H.J.
  */
-static int maydump(struct vm_area_struct *vma)
+static int maydump(struct vm_area_struct *vma, struct mm_struct *mm)
 {
 	/* Do not dump I/O mapped devices or special mappings */
 	if (vma->vm_flags & (VM_IO | VM_RESERVED)) {

@@ -1184,15 +1184,22 @@ static int maydump(struct vm_area_struct
 		return 0;
 	}
 
-	/* Dump shared memory only if mapped from an anonymous file. */
+	/*
+	 * Dump shared memory only if mapped from an anonymous file and
+	 * /proc/<pid>/coredump_omit_anonymous_shared flag is not set.
+	 */
 	if (vma->vm_flags & VM_SHARED) {
-		if (vma->vm_file->f_path.dentry->d_inode->i_nlink == 0) {
+		if (vma->vm_file->f_path.dentry->d_inode->i_nlink) {
 			kdcore("%08lx: %08lx: no (share)", vma->vm_start, vma->vm_flags);
+			return 0;
+		}
+		if (mm->coredump_omit_anon_shared) {
+			kdcore("%08lx: %08lx: no (anon-share)", vma->vm_start, vma->vm_flags);
+			return 0;
+		} else {
+			kdcore("%08lx: %08lx: yes (anon-share)", vma->vm_start, vma->vm_flags);
 			return 1;
 		}
-
-		kdcore("%08lx: %08lx: no (share)", vma->vm_start, vma->vm_flags);
-		return 0;
 	}
 
 #ifdef CONFIG_MMU

@@ -1451,7 +1458,7 @@ static int elf_fdpic_dump_segments(struc
 	for (vma = current->mm->mmap; vma; vma = vma->vm_next) {
 		unsigned long addr;
 
-		if (!maydump(vma))
+		if (!maydump(vma, mm))
 			continue;
 
 		for (addr = vma->vm_start;

@@ -1506,7 +1513,7 @@ static int elf_fdpic_dump_segments(struc
 	for (vml = current->mm->context.vmlist; vml; vml = vml->next) {
 	struct vm_area_struct *vma = vml->vma;
 
-		if (!maydump(vma))
+		if (!maydump(vma, mm))
 			continue;
 
 		if ((*size += PAGE_SIZE) > *limit)

@@ -1715,7 +1722,7 @@ static int elf_fdpic_core_dump(long sign
 		phdr.p_offset = offset;
 		phdr.p_vaddr = vma->vm_start;
 		phdr.p_paddr = 0;
-		phdr.p_filesz = maydump(vma) ? sz : 0;
+		phdr.p_filesz = maydump(vma, current->mm) ? sz : 0;
 		phdr.p_memsz = sz;
 		offset += phdr.p_filesz;
 		phdr.p_flags = vma->vm_flags & VM_READ ? PF_R : 0;

This patch adds the documentation for
/proc/<pid>/coredump_omit_anonymous_shared.

---
 Documentation/filesystems/proc.txt |   38 +++++++++++++++++++++++++++
 1 files changed, 38 insertions(+)

Index: linux-2.6.20-mm2/Documentation/filesystems/proc.txt
===================================================================
@@ -41,6 +41,7 @@ Table of Contents
   2.11	/proc/sys/fs/mqueue - POSIX message queues filesystem
   2.12	/proc/<pid>/oom_adj - Adjust the oom-killer score
   2.13	/proc/<pid>/oom_score - Display current oom-killer score
+  2.14	/proc/<pid>/coredump_omit_anonymous_shared - Core dump coordinator
 
 ------------------------------------------------------------------------------
 Preface

@@ -1982,6 +1983,43 @@ This file can be used to check the curre
 any given <pid>. Use it together with /proc/<pid>/oom_adj to tune which
 process should be killed in an out-of-memory situation.
 
+2.14 /proc/<pid>/coredump_omit_anonymous_shared - Core dump coordinator
+---------------------------------------------------------------------
+When a process is dumped, all anonymous memory is written to a core file as
+long as the size of the core file isn't limited. But sometimes we don't want
+to dump some memory segments, for example, huge shared memory.
+
+The /proc/<pid>/coredump_omit_anonymous_shared is a flag which enables you to
+omit anonymous shared memory segments from a core file when it is generated.
+When the <pid> process is dumped, the core dump routine decides whether a
+given memory segment should be dumped into a core file or not based on the
+type of the memory segment and the flag.
+
+If you have written a non-zero value to this proc file, anonymous shared
+memory segments are not dumped. There are three types of anonymous shared
+memory:
+
+  - IPC shared memory
+  - the memory segments created by mmap(2) with MAP_ANONYMOUS and MAP_SHARED
+    flags
+  - the memory segments created by mmap(2) with MAP_SHARED flag, and the
+    mapped file has already been unlinked
+
+Because current core dump routine doesn't distinguish these segments, you can
+only choose either dumping all anonymous shared memory segments or not.
+
+If you don't want to dump all shared memory segments attached to pid 1234,
+write 0 to the process's proc file.
+
+  $ echo 1 > /proc/1234/coredump_omit_anonymous_shared
+
+When a new process is created, the process inherits the flag status from its
+parent. It is useful to set the flag before the program runs.
+For example:
+
+  $ echo 1 > /proc/self/coredump_omit_anonymous_shared
+  $ ./some_program
+
 ------------------------------------------------------------------------------
 Summary
 ------------------------------------------------------------------------------

Hi!

Yes. So, you used very different interface interface from ulimit, which means locking is hard.

Plus, what you are doing can be done in userspace using google coredumper.

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Hi!

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

First up, please convince us that this problem cannot be solved in userspace. Note that we now support dumping core over a pipe to a userspace application which can perform filtering such as this (see Documentation/sysctl/kernel.txt:core_pattern).

Assuming that your argument is successful...

- The unpleasing trylock in proc_coredump_omit_anon_shared_write() is there, I believe, to handle the case where a coredump is presently in progress. We don't want to change the filtering rule while the dump is happening.

What I suggest you do instead is to take a copy of mm->coredump_omit_anon_shared into a local variable with one single read per coredump. Pass that local down into all the callees which need to see it. That way, no locking is needed.

- These games we're playing with the atomicity of the bitfields in the mm_struct need to go away.

First up, please prepare a standalone patch which removes mm_struct.dumpable and adds `unsigned long mm_struct.flags'. Include a comment telling people that they must use atomic bitops (set_bit, clear_bit) on mm_struct.flags.

Reimplement the current three-value dumpable silliness using two or three separate flags in mm_struct.flags. Of course, this design means that there will be tiny timing windows where the value of these two or three flags have intermediate, invalid states. Please take care of those little windows and document how you did so. I expect a suitable approach would be to set and clear the flags in a suitable order, so that if a race _does_ happen, the results are benign.

- Once that is done, you're ready to think about your new functionality. Start out with

	#define MM_FLAG_COREDUMP_OMIT_ANON_SHARED	(1 << 3)

or whatever, and it all becomes easy.

- Finally, the code as you have it here is very specific to your specific
requirement: don't dump shared memory segments.

But if we're going to implement in-kernel core-dump filtering of this nature, we should design it extensibly, even if we don't actually implement those extensions at this time.

Because other people might (reasonably) wish to omit anonymous memory, or private mappings, or file-backed VMAs, or whatever.

So maybe /proc/pid/coredump_omit_anon_shared should become /proc/pid/core_dumpfilter, which is a carefully documented bitmask.

Hi Pavel,
I'm sorry for my late reply.

Thank you for pointing out.
It seems I mistook when I changed the documents.
`write 1' is correct.

I'll fix the patchset so that kernel doesn't produce the spurious error.

For answers to your another mail, please wait a few days. I'm still considering the answer partly.

Thanks,

Hidehiro Kawai
Hitachi, Ltd., Systems Development Laboratory

Hi,
Thank you for your kind comments.

I'm still discussing the answer with my senior colleagues, so please wait a few days. I think I can reply at the beginning of next week.

Best regards,

Hidehiro Kawai
Hitachi, Ltd., Systems Development Laboratory

Hi Pavel,
Thank you for your reply.
I'm sorry for my late reply.

I have discussed with my colleagues why you say "ugly" against my procfs interface, then I noticed I may have misunderstood what you said. Is the reason for saying "ugly" two interfaces, i.e. preexisting ulimit (get/setrlimit) and my proc entry, exist to control core file size? If so, I'm sorry for taking your precious time by proceeding to the discussion without enough understanding.

Assuming my presumption is true, I don't think it's so ugly because there are other parameters to control core dumping such as dumpable (controled by prctl(2)) and suid_dumpable (controled via fs.suid_dumpable sysctl).
What would you think about that?

Anyway, here are the answers to what you pointed out.

I'd like to allow users to change the flag from other process. So I have to do locking even if it is hard. You said this flexibility was not an advantage before, but in some cases, it is needed.

Please assume the case where a process forks many children and they share a huge shared memory. Sometimes an end user wants to set coredump_omit_anon_shared flag to those processes except for a particular child process. With ulimit (setrlimit) interface, the user can't do such setup without modifying the application program. But normal end user will not be able to modify the program.

I think that the needs differ between userland core dumper user and in-kernel core dumper user. Pros and cons also differ.

Some of people (such as system admins, distro vendors, etc) need highly reliable core dumper because they don't want to experience same failures again and they don't hope that another failure is caused by core dumping. Userland core dumper is useful because it is relatively easy to be customized, but its reliability highly depends on the application programs.

If the stack for signal handlers is not set up carefully, if the data used by userland core dumper has been destroyed, if coredump_omit_anon_shared flag has been overwritten by bad data, or if the address of functions have been destroyed, the userland core dumper may fail to dump. So in-kernel solutoin is required by enterprise users.

I'm considering using the previous argument passing approach (preserves the setting value into a local variable and then passes it to core dump routines) or another approach which introduce a per-process flag to indicate that core dump is ongoing. Both of these approach never produce spurious errors.

Using the above-stated approach, this semaphore becomes unnecessary.

Thanks,

Hidehiro Kawai
Hitachi, Ltd., Systems Development Laboratory

Hi,
Thank you for your kind comments.
I'm sorry for my late reply.

I understand. Thank you for your suggestion. I'll reply about it in another mail, but it may take a few days.

Previous v3 patchset does what you suggest, and here are links to the patches:

[PATCH 2/4] coredump: ELF: enable to omit anonymous shared memory
http://lkml.org/lkml/2007/2/16/156

[PATCH 3/4] coredump: ELF-FDPIC: enable to omit anonymous shared memory
http://lkml.org/lkml/2007/2/16/157

However, there was an opposite opinion. To pass the flag status, I added omit_anon_shared argument to elf_fdpic_dump_segments(). Then, David pointed that the argument was unncecessary, because the function also receives mm_struct *mm which includes coredump_omit_anon_shared. But mm->coredump_omit_anon_shared can be changed while core dumping, and it may causes the core file to be corrupted. So in v4 patchset I used r/w semaphore to prevent mm->coredump_omit_anon_shared from being changed.

If I add an addtional argument to elf_fdpic_dump_segments() again, I have to explain it to David. I'll tell him that removing mm argument from the function will be a solution since it refers current->mm directly and the mm argument is never used.

OK. I'll do it in the next version.

I understood. Since I had done so initially, I'll revert it to.

There are people who wish to dump VMAs which are not dumped by default. Taking this into account, some bits of core_dumpfilter will be set by default. This means users have to be aware of the default bitmask when they change the bitmask. Perhaps changing the bitmask requires 3 steps:

1. read the default bitmask
2. change bits of the mask
3. write it to the proc entry

So I think it is better if we provide /proc/pid/core_flags (default:
all bits are 0) instead of core_dumpfilter. With this interface, users who use only one bit of the bitmask (this will be a common case) just have to write 2^n to the proc entry. It takes only one step:

1. write a value to the proc entry

If we can implement at the same cost, core_flags will be better because it is useful for users. What would you think about that?

By the way, Robin Holt wrote as follows:

Do you think Robin's suggestion is acceptable?

Best regards,

Hidehiro Kawai
Hitachi, Ltd., Systems Development Laboratory

It sounds unnecessarily complex, and unnecessarily different from our normal expectations of /proc files. And the value we read differs from the value we wrote... I think having a non-zero default will be fine.

Marginal, I think. This is not likely to be a field which a lot of people modify a lot of times. Those few people who need to work with this can afford to look the values up in the documentation while writing their script.

And it requires a distressingly large amount of code to implement a /proc file. Perhaps in this situation the code can be shared.

otoh, why is it a /proc thing at all?

unsigned long sys_set_corefile_filter(unsigned long enable_mask); unsigned long sys_clear_corefile_filter(unsigned long enable_mask);

would be better?

Hi!

Yes.

Fix userland core dumper to be reliable, then.

It should be possible to dump from separate process.

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

I don't think it's that easy. The userland core dumper, as I understand it, has to work *within* an application program (it's a library), thus the application program my scotch the core dumper in a couple of ways:

(1) by trying to claim the same services (such as signal handlers).

(2) by destroying the coredumper should the application run amok and splat it (by munmapping it, mmapping over it, writing over it, etc.).

Plus there are cases that an in-application userspace coredumper can't catch - namely double/triple faults.

David

That's no longer necessarily true with the recently-added dump-to-an-application feature:

core_pattern:
...
. 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.

That's new in 2.6.20 (maybe .19?) so people probably don't know about it.

Hi,

I think dumping core over a pipe is almost good. Filtering and writing out a core by a separete userspace program can be reliable because it is independent of the failed user process. But I have one concern; data transfer over a pipe is slow. It took 7 seconds to transfer 2GB anonymous shared memory (detailed at the last of this mail).

In the case of dumping hundreds processes which share giga bytes memory, it will take a few minutes even if the huge shared memory is not written to a disk. If a user wants to restart the failed application as soon as possible to keep downtime to a minimum, this extra transfer time will be a barrier. So I think in-kernel filtering is still needed.

I performed a simple experiment to confirm the transfer speed:
1. run a program which allocates 2GB anonymous shared memory
2. the program receives SIGSEGV
3. core image is generated and passed to tp_pipe(*) over a pipe
4. tp_pipe reads the pipe and revokes the received data while data arrives
5. tp_pipe logs time of completing the data transfer
6. repeat 1 to 5 ten times and calculate the average and standard deviation

(*) test program which I prepared for this experiment

Here is the result:

             Average: 7.041 sec

Standard deviation: 0.903

And cpuinfo and meminfo of my box:

$ cat /proc/cpuinfo
processor	: 0
vendor_id	: GenuineIntel
cpu family	: 15
model		: 4
model name	: Intel(R) Xeon(TM) CPU 3.20GHz
stepping	: 1
cpu MHz		: 3200.343
cache size	: 1024 KB
fdiv_bug	: no
hlt_bug		: no
f00f_bug	: no
coma_bug	: no
fpu		: yes
fpu_exception	: yes
cpuid level	: 5
wp		: yes
flags		: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe nx lm constant_tsc pni monitor ds_cpl cid cx16 xtpr
bogomips	: 6404.82
clflush size	: 64

processor : 1
vendor_id : GenuineIntel
cpu family : 15
model : 4
model name : Intel(R) Xeon(TM) CPU 3.20GHz
stepping : 1
cpu MHz : 3200.343
cache size : 1024 KB
fdiv_bug : no
hlt_bug : no
f00f_bug : no
coma_bug : no
fpu : yes
fpu_exception : yes
cpuid level : 5
wp : yes
flags : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe nx lm constant_tsc pni monitor ds_cpl cid cx16 xtpr
bogomips : 6400.75
clflush size : 64

$ cat /proc/meminfo
MemTotal:      8309240 kB
MemFree:       8216036 kB
Buffers:         11888 kB
Cached:          45200 kB
SwapCached:          0 kB
Active:          42324 kB
Inactive:        26008 kB
HighTotal:     7470896 kB
HighFree:      7405504 kB
LowTotal:       838344 kB
LowFree:        810532 kB
SwapTotal:    16386260 kB
SwapFree:     16386260 kB
Dirty:               4 kB
Writeback:           0 kB
AnonPages:       11232 kB
Mapped:           5828 kB
Slab:            11916 kB
SReclaimable:     5604 kB
SUnreclaim:       6312 kB
PageTables:        896 kB
NFS_Unstable:        0 kB
Bounce:              0 kB
CommitLimit:  20540880 kB
Committed_AS:    33540 kB
VmallocTotal:   118776 kB
VmallocUsed:      8324 kB
VmallocChunk:   110408 kB
HugePages_Total:     0
HugePages_Free:      0
HugePages_Rsvd:      0
Hugepagesize:     2048 kB

Thanks,

Hidehiro Kawai
Hitachi, Ltd., Systems Development Laboratory

Yes, I agree - I don't think we presently have a way of avoiding having to send all of that uninteresting data down the pipe.

One may, however, be able to play tricks with /proc/<pid>/mem from within the corefile-generating program: select the vmas which are to be dumped, read only those ones. I don't know how practical that would be.

Have the kernel fork and exec a debug program "just in time" with the dying process ptrace-attached in advance. That program could then use ptrace() or whatever to do the coredump. I think Windows does something like that.

David