Re: [PATCH 0/4] coredump: core dump masking support v4

[Andrew Morton <akpm@linux-foundation.org>]

On Fri, 02 Mar 2007 13:41:30 +0900
"Kawai, Hidehiro" <hidehiro.kawai.ez@hitachi.com> wrote:

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

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.