log out-of-virtual-memory events

I'm looking for a way to keep track of the processes that fail to allocate new virtual memory. What do you think about the following approach (untested)?

Quoted 3 lines · Andrea Righi

I'm looking for a way to keep track of the processes that fail to allocate new
virtual memory. What do you think about the following approach (untested)?

Looks like an easy way for users to spam syslogd over and over and over again.

At the very least, shouldn't this be dependant on print_fatal_signals?

Quoted 4 lines · Andrea Righi

I'm looking for a way to keep track of the processes that fail to allocate new
virtual memory. What do you think about the following approach (untested)?

Looks like an easy way for users to spam syslogd over and over and over again.

At the very least, shouldn't this be dependant on print_fatal_signals?

Quoted 11 lines · Rik van Riel

Quoted 4 lines · Andrea Righi

I'm looking for a way to keep track of the processes that fail to allocate new
virtual memory. What do you think about the following approach (untested)?

Looks like an easy way for users to spam syslogd over and over and over again.

At the very least, shouldn't this be dependant on print_fatal_signals?

Anyway, with print-fatal-signals enabled a user could spam syslogd too, simply with a (char *)0 = 0 program, but we could always identify the spam attempts logging the process uid...

In any case, I agree, it should depend on that patch...

What about adding a simple msleep_interruptible(SOME_MSECS) at the end of log_vm_enomem() or, at least, a might_sleep() to limit the potential spam/second rate?

I do not see such on i386, so why for x86_64?

Quoted 17 lines · Andrea Righi

Quoted 10 lines · Rik van Riel

Quoted 4 lines · Andrea Righi

I'm looking for a way to keep track of the processes that fail to allocate new
virtual memory. What do you think about the following approach (untested)?

Looks like an easy way for users to spam syslogd over and over and over again.

At the very least, shouldn't this be dependant on print_fatal_signals?

Anyway, with print-fatal-signals enabled a user could spam syslogd too, simply with a (char *)0 = 0 program, but we could always identify the spam attempts logging the process uid...

In any case, I agree, it should depend on that patch...

Quoted 3 lines

What about adding a simple msleep_interruptible(SOME_MSECS) at the end of log_vm_enomem() or, at least, a might_sleep() to limit the potential spam/second rate?

An msleep will slow down this process, but do nothing about slowing down the amount of logging. Simply fork a few more processes and all you are doing with msleep is polluting the pid space.

What about a throttling similar to what ia64 does for floating point assist faults (handle_fpu_swa()). There is a thread flag to not log the events at all. It is rate throttled globally, but uses per cpu variables for early exits. This algorithm scaled well to a thousand cpus.

Quoted 16 lines · Robin Holt

Quoted 15 lines · Andrea Righi

Quoted 10 lines · Rik van Riel

Quoted 4 lines · Andrea Righi

I'm looking for a way to keep track of the processes that fail to allocate new
virtual memory. What do you think about the following approach (untested)?

Looks like an easy way for users to spam syslogd over and over and over again.

At the very least, shouldn't this be dependant on print_fatal_signals?

Anyway, with print-fatal-signals enabled a user could spam syslogd too, simply with a (char *)0 = 0 program, but we could always identify the spam attempts logging the process uid...

Quoted 9 lines

Quoted 5 lines

In any case, I agree, it should depend on that patch...

What about adding a simple msleep_interruptible(SOME_MSECS) at the end of log_vm_enomem() or, at least, a might_sleep() to limit the potential spam/second rate?

An msleep will slow down this process, but do nothing about slowing down the amount of logging. Simply fork a few more processes and all you are doing with msleep is polluting the pid space.

Very true.

Quoted 5 lines

What about a throttling similar to what ia64 does for floating point assist faults (handle_fpu_swa()). There is a thread flag to not log the events at all. It is rate throttled globally, but uses per cpu variables for early exits. This algorithm scaled well to a thousand cpus.

Actually using printk_ratelimit() should be enough... BTW print_fatal_signals() should use it too.

printk("%s/%d: potentially unexpected fatal signal %d.\n",
current->comm, current->pid, signr);

Quoted 2 lines

I do not see such on i386, so why for x86_64?

So that you know that one of your programs crashed. That's a feature.

Quoted 2 lines · Andi Kleen

Quoted 1 line

I do not see such on i386, so why for x86_64?

So that you know that one of your programs crashed. That's a feature.

This feature could be handy for i386 too.

Quoted 6 lines · Jan Engelhardt

Quoted 2 lines · Andi Kleen

Quoted 1 line

I do not see such on i386, so why for x86_64?

So that you know that one of your programs crashed. That's a feature.

This feature could be handy for i386 too.

What about your /proc/sys/kernel/print-fatal-signals? it must be set to 1 to enable that feature.

Quoted 3 lines · Andrea Righi

Quoted 2 lines · Jan Engelhardt

Quoted 1 line · Andi Kleen

Quoted 1 line

I do not see such on i386, so why for x86_64?

Quoted 5 lines

Quoted 3 lines

Quoted 1 line

So that you know that one of your programs crashed. That's a feature.

This feature could be handy for i386 too.

What about your /proc/sys/kernel/print-fatal-signals? it must be set to 1 to enable that feature.

That file does not exist on versions
2.6.18 <= version <= 2.6.20

Quoted 2 lines · In article <[email protected]> you wrote:

printk("%s/%d: potentially unexpected fatal signal %d.\n",
current->comm, current->pid, signr);

can we have both KERN_WARNING please?

Gruss
Bernd

Review

Quoted 3 lines · In article <[email protected]> you wrote:

(quoted diff) +3 -0

+       printk(KERN_INFO
+              "out of virtual memory for process %d (%s): total_vm=%lu, uid=%d\n",
+               current->pid, current->comm, total_vm, current->uid);

And align this one with the print_fatal layout:

printk(KERN_WARNING
"%s/%d process cannot request more virtual memory: total_vm=%lu, uid=%d\n",
current->comm, current->pid, total_vm, current->uid);

Quoted 2 lines

Quoted 1 line

I do not see such on i386, so why for x86_64?

So that you know that one of your programs crashed. That's a feature.

This feature could be handy for i386 too.

Quoted 7 lines

Quoted 4 lines

I'm looking for a way to keep track of the processes that fail to allocate new
virtual memory. What do you think about the following approach (untested)?

Looks like an easy way for users to spam syslogd over and over and over again.
At the very least, shouldn't this be dependant on print_fatal_signals?

Anyway, with print-fatal-signals enabled a user could spam syslogd too, simply with a (char *)0 = 0 program, but we could always identify the spam attempts logging the process uid...

Quoted 3 lines

Quoted 2 lines

Quoted 1 line

I do not see such on i386, so why for x86_64?

So that you know that one of your programs crashed. That's a feature.

This feature could be handy for i386 too.

Since 2.6.18.2 I use this patch. With 2.6.21.1 it still applies altough with a small offsets. Works like a charm.

Signed-off by: Folkert van Heusden <[email protected]>

kernel/signal.c 2006-11-17 15:59:13.000000000 +0100 renamed from kernel/signal.c 2006-11-04 02:33:58.000000000 +0100 +0 -0

@@ -706,6 +706,15 @@
        struct sigqueue * q = NULL;
        int ret = 0;

Quoted 9 lines

Quoted 3 lines

Quoted 2 lines

Quoted 1 line

I do not see such on i386, so why for x86_64?

So that you know that one of your programs crashed. That's a feature.

This feature could be handy for i386 too.

Since 2.6.18.2 I use this patch. With 2.6.21.1 it still applies altough with a small offsets. Works like a charm.

Signed-off by: Folkert van Heusden <[email protected]>

kernel/signal.c 2006-11-17 15:59:13.000000000 +0100 renamed from kernel/signal.c 2006-11-04 02:33:58.000000000 +0100 +0 -0

Quoted 1 line

+ sig, t -> pid, t -> uid, t -> gid, t -> comm);

Please check line 219 of Documentation/CodingStyle, Section 3.1: Spaces

	and no space around the '.' and "->" structure member operators.

Quoted 9 lines

Quoted 9 lines

Quoted 3 lines

Quoted 2 lines

Quoted 1 line

I do not see such on i386, so why for x86_64?

So that you know that one of your programs crashed. That's a feature.

This feature could be handy for i386 too.

Since 2.6.18.2 I use this patch. With 2.6.21.1 it still applies altough with a small offsets. Works like a charm.

Signed-off by: Folkert van Heusden <[email protected]>

kernel/signal.c 2006-11-17 15:59:13.000000000 +0100 renamed from kernel/signal.c 2006-11-04 02:33:58.000000000 +0100 +0 -0

...

Quoted 4 lines

Quoted 1 line

+ sig, t -> pid, t -> uid, t -> gid, t -> comm);

Please check line 219 of Documentation/CodingStyle, Section 3.1: Spaces

	and no space around the '.' and "->" structure member operators.

New version without the spaces around '->' and a nice 'unlikely' added.

Signed-off by: Folkert van Heusden <[email protected]>

kernel/signal.c +9 -0

@@ -706,6 +706,15 @@
 	struct sigqueue * q = NULL;
 	int ret = 0;
 
+	if (unlikely(sig == SIGQUIT || sig == SIGILL  || sig == SIGTRAP ||
+	    sig == SIGABRT || sig == SIGBUS  || sig == SIGFPE  ||
+	    sig == SIGSEGV || sig == SIGXCPU || sig == SIGXFSZ ||
+	    sig == SIGSYS  || sig == SIGSTKFLT))
+	{
+		printk(KERN_WARNING "Sig %d send to %d owned by %d.%d (%s)\n",
+			sig, t->pid, t->uid, t->gid, t->comm);
+	}
+
 	/*
 	 * fast-pathed signals for kernel-internal things like SIGSTOP
 	 * or SIGKILL.

Folkert van Heusden

Quoted 22 lines

Quoted 9 lines

Quoted 9 lines

Quoted 3 lines

Quoted 2 lines

Quoted 1 line

I do not see such on i386, so why for x86_64?

So that you know that one of your programs crashed. That's a feature.

This feature could be handy for i386 too.

Since 2.6.18.2 I use this patch. With 2.6.21.1 it still applies altough with a small offsets. Works like a charm.

Signed-off by: Folkert van Heusden <[email protected]>

kernel/signal.c 2006-11-17 15:59:13.000000000 +0100 renamed from kernel/signal.c 2006-11-04 02:33:58.000000000 +0100 +0 -0

...

Quoted 4 lines

Quoted 1 line

+ sig, t -> pid, t -> uid, t -> gid, t -> comm);

Please check line 219 of Documentation/CodingStyle, Section 3.1: Spaces

	and no space around the '.' and "->" structure member operators.

New version without the spaces around '->' and a nice 'unlikely' added. Signed-off by: Folkert van Heusden <[email protected]>

kernel/signal.c +0 -0

@@ -706,6 +706,15 @@
 	struct sigqueue * q = NULL;
 	int ret = 0;
 

kernel/signal.c line 720

Quoted 12 lines

kernel/signal.c +9 -0

+	if (unlikely(sig == SIGQUIT || sig == SIGILL  || sig == SIGTRAP ||
+	    sig == SIGABRT || sig == SIGBUS  || sig == SIGFPE  ||
+	    sig == SIGSEGV || sig == SIGXCPU || sig == SIGXFSZ ||
+	    sig == SIGSYS  || sig == SIGSTKFLT))
+	{
+		printk(KERN_WARNING "Sig %d send to %d owned by %d.%d (%s)\n",
+			sig, t->pid, t->uid, t->gid, t->comm);
+	}
+
 	/*
 	 * fast-pathed signals for kernel-internal things like SIGSTOP
 	 * or SIGKILL.

Would turning that into a switch() generate better code.

Quoted 15 lines

kernel/signal.c line 720

Quoted 13 lines

kernel/signal.c +9 -0

+	if (unlikely(sig == SIGQUIT || sig == SIGILL  || sig == SIGTRAP ||
+	    sig == SIGABRT || sig == SIGBUS  || sig == SIGFPE  ||
+	    sig == SIGSEGV || sig == SIGXCPU || sig == SIGXFSZ ||
+	    sig == SIGSYS  || sig == SIGSTKFLT))
+	{
+		printk(KERN_WARNING "Sig %d send to %d owned by %d.%d (%s)\n",
+			sig, t->pid, t->uid, t->gid, t->comm);
+	}
+
 	/*
 	 * fast-pathed signals for kernel-internal things like SIGSTOP
 	 * or SIGKILL.

Would turning that into a switch() generate better code.

Doubt it: in the worst case you still nee to check for each possibility. Furthermore a.f.a.i.k. with switch you cannot do 'unlinkely()'.

Quoted 14 lines

Quoted 14 lines

Review

Quoted 13 lines

(quoted diff) +9 -0

+	if (unlikely(sig == SIGQUIT || sig == SIGILL  || sig == SIGTRAP ||
+	    sig == SIGABRT || sig == SIGBUS  || sig == SIGFPE  ||
+	    sig == SIGSEGV || sig == SIGXCPU || sig == SIGXFSZ ||
+	    sig == SIGSYS  || sig == SIGSTKFLT))
+	{
+		printk(KERN_WARNING "Sig %d send to %d owned by %d.%d (%s)\n",
+			sig, t->pid, t->uid, t->gid, t->comm);
+	}
+
 	/*
 	 * fast-pathed signals for kernel-internal things like SIGSTOP
 	 * or SIGKILL.

Would turning that into a switch() generate better code.

Yes, this time.

Quoted 2 lines

Doubt it: in the worst case you still nee to check for each possibility. Furthermore a.f.a.i.k. with switch you cannot do 'unlinkely()'.

With if(), it generates a ton of "CMP, JE" instructions. With switch(), I would assume gcc transforms it into using a jump table (aka "JMP [table+sig]")
I tried it: with switch(), gcc transforms this into a bitmap comparison ("MOV eax, 1; SHL eax, sig; TEST eax, 0x830109f8"), which seems even cheaper than a jump table.

Quoted 7 lines

signal.c renamed from signal.c.org +7 -0

+	switch(sig) {
+	case SIGQUIT: 
+	case SIGILL: 
+	case SIGTRAP:
+	case SIGABRT: 
+	case SIGBUS: 
+	case SIGFPE:

signal.c line 756

Quoted 5 lines

signal.c renamed from signal.c.org +5 -0

+	case SIGSEGV: 
+	case SIGXCPU: 
+	case SIGXFSZ:
+	case SIGSYS: 
+	case SIGSTKFLT:

Unconditional? That's definitely a very bad idea. If anything only unhandled signals should be printed this way because some programs use them internally.

But I think your list is far too long anyways.

Quoted 6 lines

Quoted 6 lines

signal.c renamed from signal.c.org +6 -0

+	switch(sig) {
+	case SIGQUIT: 
+	case SIGILL: 
+	case SIGTRAP:
+	case SIGABRT: 
+	case SIGBUS: 

Quoted 8 lines

signal.c line 756

Quoted 6 lines

signal.c renamed from signal.c.org +6 -0

+	case SIGFPE:
+	case SIGSEGV: 
+	case SIGXCPU: 
+	case SIGXFSZ:
+	case SIGSYS: 
+	case SIGSTKFLT:

Unconditional? That's definitely a very bad idea. If anything only unhandled signals should be printed this way because some programs use them internally.

Use these signals internally? Afaik these are fatal, stopping the process. So using them internally would be a little tricky.

Quoted 1 line

But I think your list is far too long anyways.

So, which ones would you like to have removed then?

Quoted 7 lines

Quoted 7 lines

signal.c renamed from signal.c.org +7 -0

+	switch(sig) {
+	case SIGQUIT: 
+	case SIGILL: 
+	case SIGTRAP:
+	case SIGABRT: 
+	case SIGBUS: 
+	case SIGFPE:

Quoted 8 lines

signal.c line 756

Quoted 5 lines

signal.c renamed from signal.c.org +5 -0

+	case SIGSEGV: 
+	case SIGXCPU: 
+	case SIGXFSZ:
+	case SIGSYS: 
+	case SIGSTKFLT:

Unconditional? That's definitely a very bad idea. If anything only unhandled signals should be printed this way because some programs use them internally.

Use these signals internally? Afaik these are fatal, stopping the process. So using them internally would be a little tricky.

Quoted 2 lines

But I think your list is far too long anyways.

So, which ones would you like to have removed then?

Quoted 3 lines

Quoted 1 line

But I think your list is far too long anyways.

So, which ones would you like to have removed then?

SIGFPE at least and the accounting signals are dubious too. SIGQUIT can be also relatively common.

Quoted 8 lines · Bernd Eckenfels

Quoted 2 lines · In article <[email protected]> you wrote:

printk("%s/%d: potentially unexpected fatal signal %d.\n",
current->comm, current->pid, signr);

can we have both KERN_WARNING please?

Gruss
Bernd

Depends on print_fatal_signals patch.

---

Limit the rate of print_fatal_signal() to avoid potential denial-of-service attacks.

Signed-off-by Andrea Righi <[email protected]>

diff -urpN linux-2.6.22-rc1-mm1/kernel/signal.c linux-2.6.22-rc1-mm1-vm-log-enomem/kernel/signal.c

kernel/signal.c +0 -0

@@ -790,7 +790,10 @@ static void print_vmas(void)
 

Print informations about userspace processes that fail to allocate new virtual memory.

Well OK. But vdso-print-fatal-signals.patch is designated not-for-mainline anyway.

I think the DoS which you identify has been available for a very long time on ia64, x86_64 and perhaps others.

signal.c line 756

Quoted 12 lines

signal.c renamed from signal.c.org +12 -0

+	switch(sig) {
+	case SIGQUIT: 
+	case SIGILL: 
+	case SIGTRAP:
+	case SIGABRT: 
+	case SIGBUS: 
+	case SIGFPE:
+	case SIGSEGV: 
+	case SIGXCPU: 
+	case SIGXFSZ:
+	case SIGSYS: 
+	case SIGSTKFLT:

Unconditional? That's definitely a very bad idea. If anything only unhandled signals should be printed this way because some programs use them internally. But I think your list is far too long anyways.

-Andi

Quoted 2 lines · On Sat, 19 May 2007 12:34:01 +0200 (MEST) Andrea Righi…

Print informations about userspace processes that fail to allocate new virtual memory.

Why is this useful?

Quoted 2 lines

Quoted 2 lines

(quoted diff) +2 -0

+	switch(sig) {
+	case SIGQUIT: 

Quoted 5 lines

Review

Quoted 1 line

(quoted diff) +1 -0

+	case SIGSTKFLT:

Unconditional? That's definitely a very bad idea. If anything only unhandled signals should be printed this way because some programs use them internally. But I think your list is far too long anyways.

Maybe you could use somthing similar to unhandled_signal() in arch/x86_64/mm/fault.c, but I agree that the list seems a bit too long...

Quoted 7 lines

Quoted 7 lines

signal.c renamed from signal.c.org +7 -0

+	switch(sig) {
+	case SIGQUIT: 
+	case SIGILL: 
+	case SIGTRAP:
+	case SIGABRT: 
+	case SIGBUS: 
+	case SIGFPE:

Quoted 8 lines

signal.c line 756

Quoted 5 lines

signal.c renamed from signal.c.org +5 -0

+	case SIGSEGV: 
+	case SIGXCPU: 
+	case SIGXFSZ:
+	case SIGSYS: 
+	case SIGSTKFLT:

Unconditional? That's definitely a very bad idea. If anything only unhandled signals should be printed this way because some programs use them internally.

Use these signals internally? Afaik these are fatal, stopping the process. So using them internally would be a little tricky.

(quoted diff) +2 -0

+	}
+

/*
* fast-pathed signals for kernel-internal things like SIGSTOP
* or SIGKILL.

of course, this can also be limited to only the interesting signals:

Signed-off by: Folkert van Heusden <[email protected]>

signal.c renamed from signal.c.org +15 -0

@@ -739,6 +739,28 @@
 	struct sigqueue * q = NULL;
 	int ret = 0;
 
+	/* emit some logging for nasty signals
+	 * especially SIGSEGV and friends aught to be looked at when happening
+	 */
+	switch(sig) {
+	case SIGQUIT: 
+	case SIGILL: 
+	case SIGTRAP:
+	case SIGABRT: 
+	case SIGBUS: 
+	case SIGFPE:
+	case SIGSEGV: 
+	case SIGXCPU: 
+	case SIGXFSZ:
+	case SIGSYS: 
+	case SIGSTKFLT:

Review

Quoted 6 lines

What about the following enhancement: I check with sig_fatal if it would kill the process and only then emit a message. So when an application takes care itself of handling it nothing is printed.

(quoted diff) +3 -0

+	/* emit some logging for unhandled signals
+	 */
+	if (sig_fatal(t, sig))

Not unhandled_signal()?

Review

Quoted 1 line

(quoted diff) +1 -0

+	{

if (sig_fatal(t, sig)) {

Review

Quoted 1 line

(quoted diff) +1 -0

+		printk(KERN_WARNING "Sig %d send to %d owned by %d.%d (%s)\n",

s/send/sent/;

Review

Quoted 1 line

(quoted diff) +1 -0

+		sig, t -> pid, t -> uid, t -> gid, t -> comm);

t->pid, t->uid, t->gid, t->comm);

Quoted 7 lines

Review

Quoted 6 lines

What about the following enhancement: I check with sig_fatal if it would kill the process and only then emit a message. So when an application takes care itself of handling it nothing is printed.

(quoted diff) +3 -0

+	/* emit some logging for unhandled signals
+	 */
+	if (sig_fatal(t, sig))

Not unhandled_signal()?

Can we already use that one in send_signal? As the signal needs to be send first I think before we know if it was handled or not? sig_fatal checks if the handler is set to default - which is it is not taken care of.

Quoted 7 lines

Quoted 7 lines

Review

Quoted 6 lines

What about the following enhancement: I check with sig_fatal if it would kill the process and only then emit a message. So when an application takes care itself of handling it nothing is printed.

(quoted diff) +3 -0

+	/* emit some logging for unhandled signals
+	 */
+	if (sig_fatal(t, sig))

Not unhandled_signal()?

Quoted 4 lines

Can we already use that one in send_signal? As the signal needs to be send first I think before we know if it was handled or not? sig_fatal checks if the handler is set to default - which is it is not taken care of.

What about ptrace()'d processes? I don't think we should log signals for them...

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

Review

Quoted 6 lines

What about the following enhancement: I check with sig_fatal if it would kill the process and only then emit a message. So when an application takes care itself of handling it nothing is printed.

(quoted diff) +3 -0

+	/* emit some logging for unhandled signals
+	 */
+	if (sig_fatal(t, sig))

Not unhandled_signal()?

Quoted 5 lines

Quoted 4 lines

Can we already use that one in send_signal? As the signal needs to be send first I think before we know if it was handled or not? sig_fatal checks if the handler is set to default - which is it is not taken care of.

What about ptrace()'d processes? I don't think we should log signals for them...

Why not?

Quoted 7 lines

Review

Quoted 6 lines

What about the following enhancement: I check with sig_fatal if it would kill the process and only then emit a message. So when an application takes care itself of handling it nothing is printed.

(quoted diff) +3 -0

+    /* emit some logging for unhandled signals
+     */
+    if (sig_fatal(t, sig))

Not unhandled_signal()?

Can we already use that one in send_signal? As the signal needs to be send first I think before we know if it was handled or not? sig_fatal checks if the handler is set to default - which is it is not taken care of.

Quoted 3 lines

Quoted 1 line

+ {

if (sig_fatal(t, sig)) {

Quoted 1 line

+ printk(KERN_WARNING "Sig %d send to %d owned by %d.%d (%s)\n",

Quoted 3 lines

s/send/sent/;

Quoted 1 line

+ sig, t -> pid, t -> uid, t -> gid, t -> comm);

t->pid, t->uid, t->gid, t->comm);

Description:
This patch adds code to the signal-sender making it log a message when an unhandled fatal signal will be delivered.

Quoted 8 lines

Quoted 3 lines

Quoted 1 line

+ {

if (sig_fatal(t, sig)) {

Quoted 1 line

+ printk(KERN_WARNING "Sig %d send to %d owned by %d.%d

(%s)\n",

Quoted 3 lines

s/send/sent/;

Quoted 1 line

+ sig, t -> pid, t -> uid, t -> gid, t -> comm);

t->pid, t->uid, t->gid, t->comm);

Gargh ... why does this want to be in the *kernel*'s logs? In any case, can you please make this KERN_INFO (or lower) instead of KERN_WARNING.

Well OK. But vdso-print-fatal-signals.patch is designated not-for-mainline anyway.

Quoted 2 lines · Andrew Morton

Well OK. But vdso-print-fatal-signals.patch is designated not-for-mainline anyway.

btw., why?

It's very, very useful to distro, early-boot-userspace and glibc development. The only add-on change should be to not print SIGKILL events. Otherwise it's very much a keeper. Hm?

Quoted 2 lines · Andrew Morton

Well OK. But vdso-print-fatal-signals.patch is designated not-for-mainline anyway.

btw., why? It's very, very useful to distro, early-boot-userspace and glibc development. The only add-on change should be to not print SIGKILL events. Otherwise it's very much a keeper. Hm?

Quoted 6 lines · On Thu, 24 May 2007 09:58:35 +0200 Ingo Molnar <[email protected]> wrote:

Quoted 2 lines · Andrew Morton

Well OK. But vdso-print-fatal-signals.patch is designated not-for-mainline anyway.

Quoted 1 line

btw., why?

err, because that's what I decided a year ago. I wonder why ;)

Perhaps because of the DoS thing, but it has a /proc knob and defaults to off, so it should be OK.

Quoted 3 lines

It's very, very useful to distro, early-boot-userspace and glibc development. The only add-on change should be to not print SIGKILL events. Otherwise it's very much a keeper. Hm?

<promotes it>

[...] The only add-on change should be to not print SIGKILL events.

Quoted 10 lines · Andrew Morton

Quoted 7 lines · On Thu, 24 May 2007 09:58:35 +0200 Ingo Molnar <[email protected]> wrote:

Quoted 2 lines · Andrew Morton

Well OK. But vdso-print-fatal-signals.patch is designated not-for-mainline anyway.

btw., why?

Quoted 4 lines

err, because that's what I decided a year ago. I wonder why ;)

Perhaps because of the DoS thing, but it has a /proc knob and defaults to off, so it should be OK.

yeah. There's also a boot option. To address the DoS angle, should i make it optionally printk_ratelimit() perhaps? (although often the messages come in streams and skipping a message can be annoying)

Quoted 6 lines

Quoted 6 lines

Quoted 3 lines

Quoted 1 line

+ {

if (sig_fatal(t, sig)) {

Quoted 1 line

+ printk(KERN_WARNING "Sig %d send to %d owned by %d.%d

(%s)\n",

Quoted 2 lines

s/send/sent/;

Quoted 1 line

+ sig, t -> pid, t -> uid, t -> gid, t -> comm);

Quoted 5 lines

Quoted 2 lines

Quoted 1 line

t->pid, t->uid, t->gid, t->comm);

Gargh ... why does this want to be in the *kernel*'s logs? In any case, can you please make this KERN_INFO (or lower) instead of KERN_WARNING.

Description:
This patch adds code to the signal-sender making it log a message when an unhandled fatal signal will be delivered.

Signed-of by: Folkert van Heusden <[email protected]

Quoted 1 line · Folkert van Heusden

Signed-of by: Folkert van Heusden <[email protected]

This looks broken BTW.

kernel/signal.c line 751

Quoted 4 lines

kernel/signal.c renamed from kernel/signal.c.org +4 -0

+			printk(KERN_INFO "Sig %d sent to %d owned by %d.%d (%s), sent by pid %d, uid %d\n",
+				sig, t->pid, t->uid, t->gid, t->comm,
+				info -> _sifields._kill._pid,
+				info -> _sifields._kill._uid);

Am I the only one whose eyes are hurt by these spaces?