2008-12-11 07:00:33

by Hidehiro Kawai

[permalink] [raw]
Subject: [PATCH] coredump_filter: enable to change the default filter

This patch introduces new kernel parameter `coredump_filter'.
Setting a value to this parameter causes the default bitmask
of coredump_filter to be changed.

It is useful for users to change coredump_filter settings for
the whole system at boot time. Without this parameter, users
have to change coredump_filter settings for each /proc/<pid>/
in an initializing script.

Signed-off-by: Hidehiro Kawai <[email protected]>
CC: Roland McGrath <[email protected]>
CC: Motohiro Kosaki <[email protected]>
---
Documentation/kernel-parameters.txt | 5 +++++
kernel/fork.c | 15 +++++++++++++--
2 files changed, 18 insertions(+), 2 deletions(-)

Index: linux-2.6.28-rc8/Documentation/kernel-parameters.txt
===================================================================
--- linux-2.6.28-rc8.orig/Documentation/kernel-parameters.txt
+++ linux-2.6.28-rc8/Documentation/kernel-parameters.txt
@@ -547,6 +547,11 @@ and is between 256 and 4096 characters.
not work reliably with all consoles, but is known
to work with serial and VGA consoles.

+ coredump_filter=
+ [KNL] Change the default value for
+ /proc/<pid>/coredump_filter.
+ See also Documentation/filesystems/proc.txt.
+
cpcihp_generic= [HW,PCI] Generic port I/O CompactPCI driver
Format:
<first_slot>,<last_slot>,<port>,<enum_bit>[,<debug>]
Index: linux-2.6.28-rc8/kernel/fork.c
===================================================================
--- linux-2.6.28-rc8.orig/kernel/fork.c
+++ linux-2.6.28-rc8/kernel/fork.c
@@ -397,6 +397,18 @@ __cacheline_aligned_in_smp DEFINE_SPINLO
#define allocate_mm() (kmem_cache_alloc(mm_cachep, GFP_KERNEL))
#define free_mm(mm) (kmem_cache_free(mm_cachep, (mm)))

+static unsigned long default_dump_filter = MMF_DUMP_FILTER_DEFAULT;
+
+static int __init coredump_filter_setup(char *s)
+{
+ default_dump_filter =
+ (simple_strtoul(s, NULL, 0) << MMF_DUMP_FILTER_SHIFT) &
+ MMF_DUMP_FILTER_MASK;
+ return 1;
+}
+
+__setup("coredump_filter=", coredump_filter_setup);
+
#include <linux/init_task.h>

static struct mm_struct * mm_init(struct mm_struct * mm, struct task_struct *p)
@@ -405,8 +417,7 @@ static struct mm_struct * mm_init(struct
atomic_set(&mm->mm_count, 1);
init_rwsem(&mm->mmap_sem);
INIT_LIST_HEAD(&mm->mmlist);
- mm->flags = (current->mm) ? current->mm->flags
- : MMF_DUMP_FILTER_DEFAULT;
+ mm->flags = (current->mm) ? current->mm->flags : default_dump_filter;
mm->core_state = NULL;
mm->nr_ptes = 0;
set_mm_counter(mm, file_rss, 0);



2008-12-11 06:47:05

by KOSAKI Motohiro

[permalink] [raw]
Subject: Re: [PATCH] coredump_filter: enable to change the default filter

Hi

> This patch introduces new kernel parameter `coredump_filter'.
> Setting a value to this parameter causes the default bitmask
> of coredump_filter to be changed.
>
> It is useful for users to change coredump_filter settings for
> the whole system at boot time. Without this parameter, users
> have to change coredump_filter settings for each /proc/<pid>/
> in an initializing script.

I think this patch only useful on following situation, right?

- Administrator can't change rc script.
- Administrator can change boot parameter.


When happen above situation?
or, you intent to init process debugging?

2008-12-11 23:11:04

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] coredump_filter: enable to change the default filter

On Thu, 11 Dec 2008 15:46:49 +0900 (JST)
KOSAKI Motohiro <[email protected]> wrote:

> Hi
>
> > This patch introduces new kernel parameter `coredump_filter'.
> > Setting a value to this parameter causes the default bitmask
> > of coredump_filter to be changed.
> >
> > It is useful for users to change coredump_filter settings for
> > the whole system at boot time. Without this parameter, users
> > have to change coredump_filter settings for each /proc/<pid>/
> > in an initializing script.
>
> I think this patch only useful on following situation, right?
>
> - Administrator can't change rc script.
> - Administrator can change boot parameter.
>
>
> When happen above situation?
> or, you intent to init process debugging?

Yes, this can be implemented in userspace by setting init's filter
before init forks any other processes.

I can understand the practical problems with that form of implementation
however :(


Or does the patch change other behaviour? Say, when mm_init() is
called by a kernel thread (current->mm==NULL)? call_usermodehelper(),
for example?

If so, then setting init's filter doesn't cover that case.

2008-12-11 23:22:36

by Roland McGrath

[permalink] [raw]
Subject: Re: [PATCH] coredump_filter: enable to change the default filter

> Or does the patch change other behaviour? Say, when mm_init() is
> called by a kernel thread (current->mm==NULL)? call_usermodehelper(),
> for example?
>
> If so, then setting init's filter doesn't cover that case.

Hmm, probably for the kernel thread case mm_init should use
task_active_pid_ns(current)->child_reaper->mm->flags.


Thanks,
Roland

2008-12-19 10:05:30

by Hidehiro Kawai

[permalink] [raw]
Subject: Re: [PATCH] coredump_filter: enable to change the default filter

Hi,

Roland McGrath wrote:

>>Or does the patch change other behaviour? Say, when mm_init() is
>>called by a kernel thread (current->mm==NULL)? call_usermodehelper(),
>>for example?
>>
>>If so, then setting init's filter doesn't cover that case.
>
> Hmm, probably for the kernel thread case mm_init should use
> task_active_pid_ns(current)->child_reaper->mm->flags.

It makes sense. It also enables us to change coredump_filter for
user mode helpers after the system was booted.

However, we would need a special care for PID namespace if we use
init's coredump_filter as default. For a process with new PID
namespace and new /proc, writing to /proc/1/coredump_filter doesn't
mean changing default, although it's not a usual operation.

The reason why I suggested the boot parameter way is most users
can change the default easily. Some users have difficulty to
change init scripts for various reasons.

So I think the boot parameter way is simpler for both users and
kernel developers.

Thanks,
--
Hidehiro Kawai
Hitachi, Systems Development Laboratory
Linux Technology Center

2008-12-19 19:54:18

by Roland McGrath

[permalink] [raw]
Subject: Re: [PATCH] coredump_filter: enable to change the default filter

> It makes sense. It also enables us to change coredump_filter for
> user mode helpers after the system was booted.

Right.

> However, we would need a special care for PID namespace if we use
> init's coredump_filter as default. For a process with new PID
> namespace and new /proc, writing to /proc/1/coredump_filter doesn't
> mean changing default, although it's not a usual operation.

I'd figured that private PID namespaces with their own init would want
their own private settings for this default too.

> The reason why I suggested the boot parameter way is most users
> can change the default easily. Some users have difficulty to
> change init scripts for various reasons.

I don't object to it deeply or anything.
I just pointed out that we do already have an alternative.


Thanks,
Roland

2008-12-26 06:23:18

by Hidehiro Kawai

[permalink] [raw]
Subject: Re: [PATCH] coredump_filter: enable to change the default filter

Hi,

I'm sorry for late reply.

Roland McGrath wrote:

>>However, we would need a special care for PID namespace if we use
>>init's coredump_filter as default. For a process with new PID
>>namespace and new /proc, writing to /proc/1/coredump_filter doesn't
>>mean changing default, although it's not a usual operation.
>
> I'd figured that private PID namespaces with their own init would want
> their own private settings for this default too.

Probably it's true, but I'm not sure if having coredump_filter have
two means is acceptable. init in a private PID namespace can die
and dump a core file. So its coredump_filter has two means; default
setting for the namespace and setting for itself.

Thanks,
--
Hidehiro Kawai
Hitachi, Systems Development Laboratory
Linux Technology Center

2008-12-26 06:50:06

by KOSAKI Motohiro

[permalink] [raw]
Subject: Re: [PATCH] coredump_filter: enable to change the default filter

> >>However, we would need a special care for PID namespace if we use
> >>init's coredump_filter as default. For a process with new PID
> >>namespace and new /proc, writing to /proc/1/coredump_filter doesn't
> >>mean changing default, although it's not a usual operation.
> >
> > I'd figured that private PID namespaces with their own init would want
> > their own private settings for this default too.
>
> Probably it's true, but I'm not sure if having coredump_filter have
> two means is acceptable. init in a private PID namespace can die
> and dump a core file. So its coredump_filter has two means; default
> setting for the namespace and setting for itself.

FWIW, I think private namespace should have private settings framework.
I don't like to make special private settings code in this patch.

Yeah, defenitely we need to private settings for various reason, I agreed.
but we also need consistent private settings rule and framework.
otherwise, we will make various non consistent crap. maybe..


2008-12-26 09:45:51

by Hidehiro Kawai

[permalink] [raw]
Subject: Re: [PATCH] coredump_filter: enable to change the default filter

KOSAKI Motohiro wrote:

>>>>However, we would need a special care for PID namespace if we use
>>>>init's coredump_filter as default. For a process with new PID
>>>>namespace and new /proc, writing to /proc/1/coredump_filter doesn't
>>>>mean changing default, although it's not a usual operation.
>>>
>>>I'd figured that private PID namespaces with their own init would want
>>>their own private settings for this default too.
>>
>>Probably it's true, but I'm not sure if having coredump_filter have
>>two means is acceptable. init in a private PID namespace can die
>>and dump a core file. So its coredump_filter has two means; default
>>setting for the namespace and setting for itself.
>
> FWIW, I think private namespace should have private settings framework.
> I don't like to make special private settings code in this patch.
>
> Yeah, defenitely we need to private settings for various reason, I agreed.
> but we also need consistent private settings rule and framework.
> otherwise, we will make various non consistent crap. maybe..

Anyway, as Andrew pointed out, current coredump filter doesn't
care for user mode helpers. So we need some sort of changes in
coredump filter so that users can change user mode helpers'
settings; use child_reapers setting as default (Roland's
suggestion) or use a boot parameter.

Thanks,
--
Hidehiro Kawai
Hitachi, Systems Development Laboratory
Linux Technology Center

2008-12-26 11:46:15

by KOSAKI Motohiro

[permalink] [raw]
Subject: Re: [PATCH] coredump_filter: enable to change the default filter

>>>>>However, we would need a special care for PID namespace if we use
>>>>>init's coredump_filter as default. For a process with new PID
>>>>>namespace and new /proc, writing to /proc/1/coredump_filter doesn't
>>>>>mean changing default, although it's not a usual operation.
>>>>
>>>>I'd figured that private PID namespaces with their own init would want
>>>>their own private settings for this default too.
>>>
>>>Probably it's true, but I'm not sure if having coredump_filter have
>>>two means is acceptable. init in a private PID namespace can die
>>>and dump a core file. So its coredump_filter has two means; default
>>>setting for the namespace and setting for itself.
>>
>> FWIW, I think private namespace should have private settings framework.
>> I don't like to make special private settings code in this patch.
>>
>> Yeah, defenitely we need to private settings for various reason, I agreed.
>> but we also need consistent private settings rule and framework.
>> otherwise, we will make various non consistent crap. maybe..
>
> Anyway, as Andrew pointed out, current coredump filter doesn't
> care for user mode helpers. So we need some sort of changes in
> coredump filter so that users can change user mode helpers'
> settings; use child_reapers setting as default (Roland's
> suggestion) or use a boot parameter.

hm, ok.
I agreed your patch is nice.

Acked-by: KOSAKI Motohiro <[email protected]>