i386-show-unhandled-signals-v3

i386-show-unhandled-signals-v3

[Masoud Asgharifard Sharbiani]

Hello,
This patch makes the i386 behave the same way that x86_64 does when a
segfault happens.  A line gets printed to the kernel log so that tools
that
need to check for failures can behave more uniformly between
debug.show_unhandled_signals sysctl variable to 0 (or by doing echo 0 > 
/proc/sys/debug/exception-trace)

Also, all of the lines being printed are now using printk_ratelimit() to
deny the ability of DoS from a local user with a program like the
following:

main()
{
       while (1)
               if (!fork()) *(int *)0 = 0;
}

This new revision also includes the fix that Andrew did which got rid of
new sysctl that was added to the system in earlier versions of this.
Also, 'show-unhandled-signals' sysctl has been renamed back to the old
'exception-trace' to avoid breakage of people's scripts.

cheers,
Masoud Sharbiani

Signed-off-by: Masoud Sharbiani <masouds@google.com>
Cc: Andi Kleen <ak@suse.de>

---
 arch/i386/kernel/signal.c   |    7 +++++++
 arch/i386/kernel/traps.c    |    7 +++++++
 arch/i386/mm/fault.c        |   10 ++++++++++
 arch/x86_64/kernel/signal.c |    2 +-
 arch/x86_64/kernel/traps.c  |    6 ++++--
 arch/x86_64/mm/fault.c      |   15 +++------------
 arch/x86_64/mm/init.c       |   35 -----------------------------------
 include/asm-x86_64/proto.h  |    2 --
 include/linux/signal.h      |    3 +++
 kernel/signal.c             |   10 ++++++++++
 kernel/sysctl.c             |   10 ++++++++++
 11 files changed, 55 insertions(+), 52 deletions(-)

diff --git a/arch/i386/kernel/signal.c b/arch/i386/kernel/signal.c
index d574e38..f5dd856 100644
--- a/arch/i386/kernel/signal.c
+++ b/arch/i386/kernel/signal.c
@@ -199,6 +199,13 @@ asmlinkage int sys_sigreturn(unsigned long __unused)
 	return eax;
 
 badframe:
+	if (show_unhandled_signals && printk_ratelimit())
+		printk("%s%s[%d] bad frame in sigreturn frame:%p eip:%lx"
+		       " esp:%lx oeax:%lx\n",
+		    current->pid > 1 ? KERN_INFO : KERN_EMERG,
+		    current->comm, current->pid, frame, regs->eip,
+		    regs->esp, regs->orig_eax);
+
 	force_sig(SIGSEGV, current);
 	return 0;
 }	
diff --git a/arch/i386/kernel/traps.c b/arch/i386/kernel/traps.c
index 18c1c28..c20283c 100644
--- a/arch/i386/kernel/traps.c
+++ b/arch/i386/kernel/traps.c
@@ -611,6 +611,13 @@ fastcall void __kprobes do_general_protection(struct pt_regs * regs,
 
 	current->thread.error_code = error_code;
 	current->thread.trap_no = 13;
+	if (show_unhandled_signals && unhandled_signal(current, SIGSEGV) &&
+	    printk_ratelimit())
+		printk(KERN_INFO
+		    "%s[%d] general protection eip:%lx esp:%lx error:%lx\n",
+		    current->comm, current->pid,
+		    regs->eip, regs->esp, error_code);
+
 	force_sig(SIGSEGV, current);
 	return;
 
diff --git a/arch/i386/mm/fault.c b/arch/i386/mm/fault.c
index 1ecb3e4..52c940b 100644
--- a/arch/i386/mm/fault.c
+++ b/arch/i386/mm/fault.c
@@ -283,6 +283,8 @@ static inline int vmalloc_fault(unsigned long address)
 	return 0;
 }
 
+int show_unhandled_signals = 1;
+
 /*
  * This routine handles page faults.  It determines the address,
  * and the problem, and then passes it off to one of the appropriate
@@ -470,6 +472,14 @@ bad_area_nosemaphore:
 		if (is_prefetch(regs, address, error_code))
 			return;
 
+		if (show_unhandled_signals && unhandled_signal(tsk, SIGSEGV) &&
+		    printk_ratelimit()) {
+			printk("%s%s[%d]: segfault at %08lx eip %08lx "
+			    "esp %08lx error %lx\n",
+			    tsk->pid > 1 ? KERN_INFO : KERN_EMERG,
+			    tsk->comm, tsk->pid, address, regs->eip,
+			    regs->esp, error_code);
+		}
 		tsk->thread.cr2 = address;
 		/* Kernel addresses are always protection faults */
 		tsk->thread.error_code = error_code | (address >= TASK_SIZE);
diff --git a/arch/x86_64/kernel/signal.c b/arch/x86_64/kernel/signal.c
index 290f5d8..f9506f6 100644
--- a/arch/x86_64/kernel/signal.c
+++ b/arch/x86_64/kernel/signal.c
@@ -480,7 +480,7 @@ do_notify_resume(struct pt_regs *regs, void *unused, __u32 thread_info_flags)
 void signal_fault(struct pt_regs *regs, void __user *frame, char *where)
 { 
 	struct task_struct *me = current; 
-	if (exception_trace)
+	if (show_unhandled_signals && printk_ratelimit())
 		printk("%s[%d] bad frame in %s frame:%p rip:%lx rsp:%lx orax:%lx\n",
 	       me->comm,me->pid,where,frame,regs->rip,regs->rsp,regs->orig_rax); 
 
diff --git a/arch/x86_64/kernel/traps.c b/arch/x86_64/kernel/traps.c
index 74cbeb2..b9660c4 100644
--- a/arch/x86_64/kernel/traps.c
+++ b/arch/x86_64/kernel/traps.c
@@ -580,7 +580,8 @@ static void __kprobes do_trap(int trapnr, int signr, char *str,
 		tsk->thread.error_code = error_code;
 		tsk->thread.trap_no = trapnr;
 
-		if (exception_trace && unhandled_signal(tsk, signr))
+		if (show_unhandled_signals && unhandled_signal(tsk, signr) &&
+		    printk_ratelimit())
 			printk(KERN_INFO
 			       "%s[%d] trap %s rip:%lx rsp:%lx error:%lx\n",
 			       tsk->comm, tsk->pid, str,
@@ -684,7 +685,8 @@ asmlinkage void __kprobes do_general_protection(struct pt_regs * regs,
 		tsk->thread.error_code = error_code;
 		tsk->thread.trap_no = 13;
 
-		if (exception_trace && unhandled_signal(tsk, SIGSEGV))
+		if (show_unhandled_signals && unhandled_signal(tsk, SIGSEGV) &&
+		    printk_ratelimit())
 			printk(KERN_INFO
 		       "%s[%d] general protection rip:%lx rsp:%lx error:%lx\n",
 			       tsk->comm, tsk->pid,
diff --git a/arch/x86_64/mm/fault.c b/arch/x86_64/mm/fault.c
index 635e58d..0412824 100644
--- a/arch/x86_64/mm/fault.c
+++ b/arch/x86_64/mm/fault.c
@@ -221,16 +221,6 @@ static int is_errata93(struct pt_regs *regs, unsigned long address)
 	return 0;
 } 
 
-int unhandled_signal(struct task_struct *tsk, int sig)
-{
-	if (is_init(tsk))
-		return 1;
-	if (tsk->ptrace & PT_PTRACED)
-		return 0;
-	return (tsk->sighand->action[sig-1].sa.sa_handler == SIG_IGN) ||
-		(tsk->sighand->action[sig-1].sa.sa_handler == SIG_DFL);
-}
-
 static noinline void pgtable_bad(unsigned long address, struct pt_regs *regs,
 				 unsigned long error_code)
 {
@@ -302,7 +292,7 @@ static int vmalloc_fault(unsigned long address)
 }
 
 int page_fault_trace = 0;
-int exception_trace = 1;
+int show_unhandled_signals = 1;
 
 /*
  * This routine handles page faults.  It determines the address,
@@ -495,7 +485,8 @@ bad_area_nosemaphore:
 		    (address >> 32))
 			return;
 
-		if (exception_trace && unhandled_signal(tsk, SIGSEGV)) {
+		if (show_unhandled_signals && unhandled_signal(tsk, SIGSEGV) &&
+		    printk_ratelimit()) {
 			printk(
 		       "%s%s[%d]: segfault at %016lx rip %016lx rsp %016lx error %lx\n",
 					tsk->pid > 1 ? KERN_INFO : KERN_EMERG,
diff --git a/arch/x86_64/mm/init.c b/arch/x86_64/mm/init.c
index 9a0e98a..5096168 100644
--- a/arch/x86_64/mm/init.c
+++ b/arch/x86_64/mm/init.c
@@ -697,41 +697,6 @@ int kern_addr_valid(unsigned long addr)
 	return pfn_valid(pte_pfn(*pte));
 }
 
-#ifdef CONFIG_SYSCTL
-#include <linux/sysctl.h>
-
-extern int exception_trace, page_fault_trace;
-
-static ctl_table debug_table2[] = {
-	{
-		.ctl_name	= 99,
-		.procname	= "exception-trace",
-		.data		= &exception_trace,
-		.maxlen		= sizeof(int),
-		.mode		= 0644,
-		.proc_handler	= proc_dointvec
-	},
-	{}
-}; 
-
-static ctl_table debug_root_table2[] = { 
-	{
-		.ctl_name = CTL_DEBUG,
-		.procname = "debug",
-		.mode = 0555,
-		.child = debug_table2
-	},
-	{}
-}; 
-
-static __init int x8664_sysctl_init(void)
-{ 
-	register_sysctl_table(debug_root_table2);
-	return 0;
-}
-__initcall(x8664_sysctl_init);
-#endif
-
 /* A pseudo VMA to allow ptrace access for the vsyscall page.  This only
    covers the 64bit vsyscall page now. 32bit has a real VMA now and does
    not need special handling anymore. */
diff --git a/include/asm-x86_64/proto.h b/include/asm-x86_64/proto.h
index 85255db..4fad501 100644
--- a/include/asm-x86_64/proto.h
+++ b/include/asm-x86_64/proto.h
@@ -75,8 +75,6 @@ extern void setup_node_bootmem(int nodeid, unsigned long start, unsigned long en
 extern void early_quirks(void);
 extern void check_efer(void);
 
-extern int unhandled_signal(struct task_struct *tsk, int sig);
-
 extern void select_idle_routine(const struct cpuinfo_x86 *c);
 
 extern unsigned long table_start, table_end;
diff --git a/include/linux/signal.h b/include/linux/signal.h
index ea91abe..0ae3388 100644
--- a/include/linux/signal.h
+++ b/include/linux/signal.h
@@ -237,12 +237,15 @@ extern int group_send_sig_info(int sig, struct siginfo *info, struct task_struct
 extern int __group_send_sig_info(int, struct siginfo *, struct task_struct *);
 extern long do_sigpending(void __user *, unsigned long);
 extern int sigprocmask(int, sigset_t *, sigset_t *);
+extern int show_unhandled_signals;
 
 struct pt_regs;
 extern int get_signal_to_deliver(siginfo_t *info, struct k_sigaction *return_ka, struct pt_regs *regs, void *cookie);
 
 extern struct kmem_cache *sighand_cachep;
 
+int unhandled_signal(struct task_struct *tsk, int sig);
+
 /*
  * In POSIX a signal is sent either to a specific thread (Linux task)
  * or to the process as a whole (Linux thread group).  How the signal
diff --git a/kernel/signal.c b/kernel/signal.c
index 39d1227..ef8156a 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -255,6 +255,16 @@ flush_signal_handlers(struct task_struct *t, int force_default)
 	}
 }
 
+int unhandled_signal(struct task_struct *tsk, int sig)
+{
+	if (is_init(tsk))
+		return 1;
+	if (tsk->ptrace & PT_PTRACED)
+		return 0;
+	return (tsk->sighand->action[sig-1].sa.sa_handler == SIG_IGN) ||
+		(tsk->sighand->action[sig-1].sa.sa_handler == SIG_DFL);
+}
+
 
 /* Notify the system that a driver wants to block all signals for this
  * process, and wants to be notified if any signals at all were to be
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 7063ebc..af7002f 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -1153,6 +1153,16 @@ static ctl_table fs_table[] = {
 };
 
 static ctl_table debug_table[] = {
+#ifdef CONFIG_X86
+	{
+		.ctl_name	= CTL_UNNUMBERED,
+		.procname	= "exception-trace",
+		.data		= &show_unhandled_signals,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= proc_dointvec
+	},
+#endif
 	{ .ctl_name = 0 }
 };
 

Re: i386-show-unhandled-signals-v3

[Kirill Korotaev]

plz don't enable it by default... :/
any user can spam syslog with these messages and if syslog is run as root
can take the whole diskspace...

Thanks,
Kirill

Masoud Asgharifard Sharbiani wrote:
> Hello,
> This patch makes the i386 behave the same way that x86_64 does when a
> segfault happens.  A line gets printed to the kernel log so that tools
> that
> need to check for failures can behave more uniformly between
> debug.show_unhandled_signals sysctl variable to 0 (or by doing echo 0 > 
> /proc/sys/debug/exception-trace)
> 
> Also, all of the lines being printed are now using printk_ratelimit() to
> deny the ability of DoS from a local user with a program like the
> following:
> 
> main()
> {
>        while (1)
>                if (!fork()) *(int *)0 = 0;
> }
> 
> This new revision also includes the fix that Andrew did which got rid of
> new sysctl that was added to the system in earlier versions of this.
> Also, 'show-unhandled-signals' sysctl has been renamed back to the old
> 'exception-trace' to avoid breakage of people's scripts.
> 
> cheers,
> Masoud Sharbiani
> 
> Signed-off-by: Masoud Sharbiani <masouds@google.com>
> Cc: Andi Kleen <ak@suse.de>
> 
> ---
>  arch/i386/kernel/signal.c   |    7 +++++++
>  arch/i386/kernel/traps.c    |    7 +++++++
>  arch/i386/mm/fault.c        |   10 ++++++++++
>  arch/x86_64/kernel/signal.c |    2 +-
>  arch/x86_64/kernel/traps.c  |    6 ++++--
>  arch/x86_64/mm/fault.c      |   15 +++------------
>  arch/x86_64/mm/init.c       |   35 -----------------------------------
>  include/asm-x86_64/proto.h  |    2 --
>  include/linux/signal.h      |    3 +++
>  kernel/signal.c             |   10 ++++++++++
>  kernel/sysctl.c             |   10 ++++++++++
>  11 files changed, 55 insertions(+), 52 deletions(-)
> 
> diff --git a/arch/i386/kernel/signal.c b/arch/i386/kernel/signal.c
> index d574e38..f5dd856 100644
> --- a/arch/i386/kernel/signal.c
> +++ b/arch/i386/kernel/signal.c
> @@ -199,6 +199,13 @@ asmlinkage int sys_sigreturn(unsigned long __unused)
>  	return eax;
>  
>  badframe:
> +	if (show_unhandled_signals && printk_ratelimit())
> +		printk("%s%s[%d] bad frame in sigreturn frame:%p eip:%lx"
> +		       " esp:%lx oeax:%lx\n",
> +		    current->pid > 1 ? KERN_INFO : KERN_EMERG,
> +		    current->comm, current->pid, frame, regs->eip,
> +		    regs->esp, regs->orig_eax);
> +
>  	force_sig(SIGSEGV, current);
>  	return 0;
>  }	
> diff --git a/arch/i386/kernel/traps.c b/arch/i386/kernel/traps.c
> index 18c1c28..c20283c 100644
> --- a/arch/i386/kernel/traps.c
> +++ b/arch/i386/kernel/traps.c
> @@ -611,6 +611,13 @@ fastcall void __kprobes do_general_protection(struct pt_regs * regs,
>  
>  	current->thread.error_code = error_code;
>  	current->thread.trap_no = 13;
> +	if (show_unhandled_signals && unhandled_signal(current, SIGSEGV) &&
> +	    printk_ratelimit())
> +		printk(KERN_INFO
> +		    "%s[%d] general protection eip:%lx esp:%lx error:%lx\n",
> +		    current->comm, current->pid,
> +		    regs->eip, regs->esp, error_code);
> +
>  	force_sig(SIGSEGV, current);
>  	return;
>  
> diff --git a/arch/i386/mm/fault.c b/arch/i386/mm/fault.c
> index 1ecb3e4..52c940b 100644
> --- a/arch/i386/mm/fault.c
> +++ b/arch/i386/mm/fault.c
> @@ -283,6 +283,8 @@ static inline int vmalloc_fault(unsigned long address)
>  	return 0;
>  }
>  
> +int show_unhandled_signals = 1;
> +
>  /*
>   * This routine handles page faults.  It determines the address,
>   * and the problem, and then passes it off to one of the appropriate
> @@ -470,6 +472,14 @@ bad_area_nosemaphore:
>  		if (is_prefetch(regs, address, error_code))
>  			return;
>  
> +		if (show_unhandled_signals && unhandled_signal(tsk, SIGSEGV) &&
> +		    printk_ratelimit()) {
> +			printk("%s%s[%d]: segfault at %08lx eip %08lx "
> +			    "esp %08lx error %lx\n",
> +			    tsk->pid > 1 ? KERN_INFO : KERN_EMERG,
> +			    tsk->comm, tsk->pid, address, regs->eip,
> +			    regs->esp, error_code);
> +		}
>  		tsk->thread.cr2 = address;
>  		/* Kernel addresses are always protection faults */
>  		tsk->thread.error_code = error_code | (address >= TASK_SIZE);
> diff --git a/arch/x86_64/kernel/signal.c b/arch/x86_64/kernel/signal.c
> index 290f5d8..f9506f6 100644
> --- a/arch/x86_64/kernel/signal.c
> +++ b/arch/x86_64/kernel/signal.c
> @@ -480,7 +480,7 @@ do_notify_resume(struct pt_regs *regs, void *unused, __u32 thread_info_flags)
>  void signal_fault(struct pt_regs *regs, void __user *frame, char *where)
>  { 
>  	struct task_struct *me = current; 
> -	if (exception_trace)
> +	if (show_unhandled_signals && printk_ratelimit())
>  		printk("%s[%d] bad frame in %s frame:%p rip:%lx rsp:%lx orax:%lx\n",
>  	       me->comm,me->pid,where,frame,regs->rip,regs->rsp,regs->orig_rax); 
>  
> diff --git a/arch/x86_64/kernel/traps.c b/arch/x86_64/kernel/traps.c
> index 74cbeb2..b9660c4 100644
> --- a/arch/x86_64/kernel/traps.c
> +++ b/arch/x86_64/kernel/traps.c
> @@ -580,7 +580,8 @@ static void __kprobes do_trap(int trapnr, int signr, char *str,
>  		tsk->thread.error_code = error_code;
>  		tsk->thread.trap_no = trapnr;
>  
> -		if (exception_trace && unhandled_signal(tsk, signr))
> +		if (show_unhandled_signals && unhandled_signal(tsk, signr) &&
> +		    printk_ratelimit())
>  			printk(KERN_INFO
>  			       "%s[%d] trap %s rip:%lx rsp:%lx error:%lx\n",
>  			       tsk->comm, tsk->pid, str,
> @@ -684,7 +685,8 @@ asmlinkage void __kprobes do_general_protection(struct pt_regs * regs,
>  		tsk->thread.error_code = error_code;
>  		tsk->thread.trap_no = 13;
>  
> -		if (exception_trace && unhandled_signal(tsk, SIGSEGV))
> +		if (show_unhandled_signals && unhandled_signal(tsk, SIGSEGV) &&
> +		    printk_ratelimit())
>  			printk(KERN_INFO
>  		       "%s[%d] general protection rip:%lx rsp:%lx error:%lx\n",
>  			       tsk->comm, tsk->pid,
> diff --git a/arch/x86_64/mm/fault.c b/arch/x86_64/mm/fault.c
> index 635e58d..0412824 100644
> --- a/arch/x86_64/mm/fault.c
> +++ b/arch/x86_64/mm/fault.c
> @@ -221,16 +221,6 @@ static int is_errata93(struct pt_regs *regs, unsigned long address)
>  	return 0;
>  } 
>  
> -int unhandled_signal(struct task_struct *tsk, int sig)
> -{
> -	if (is_init(tsk))
> -		return 1;
> -	if (tsk->ptrace & PT_PTRACED)
> -		return 0;
> -	return (tsk->sighand->action[sig-1].sa.sa_handler == SIG_IGN) ||
> -		(tsk->sighand->action[sig-1].sa.sa_handler == SIG_DFL);
> -}
> -
>  static noinline void pgtable_bad(unsigned long address, struct pt_regs *regs,
>  				 unsigned long error_code)
>  {
> @@ -302,7 +292,7 @@ static int vmalloc_fault(unsigned long address)
>  }
>  
>  int page_fault_trace = 0;
> -int exception_trace = 1;
> +int show_unhandled_signals = 1;
>  
>  /*
>   * This routine handles page faults.  It determines the address,
> @@ -495,7 +485,8 @@ bad_area_nosemaphore:
>  		    (address >> 32))
>  			return;
>  
> -		if (exception_trace && unhandled_signal(tsk, SIGSEGV)) {
> +		if (show_unhandled_signals && unhandled_signal(tsk, SIGSEGV) &&
> +		    printk_ratelimit()) {
>  			printk(
>  		       "%s%s[%d]: segfault at %016lx rip %016lx rsp %016lx error %lx\n",
>  					tsk->pid > 1 ? KERN_INFO : KERN_EMERG,
> diff --git a/arch/x86_64/mm/init.c b/arch/x86_64/mm/init.c
> index 9a0e98a..5096168 100644
> --- a/arch/x86_64/mm/init.c
> +++ b/arch/x86_64/mm/init.c
> @@ -697,41 +697,6 @@ int kern_addr_valid(unsigned long addr)
>  	return pfn_valid(pte_pfn(*pte));
>  }
>  
> -#ifdef CONFIG_SYSCTL
> -#include <linux/sysctl.h>
> -
> -extern int exception_trace, page_fault_trace;
> -
> -static ctl_table debug_table2[] = {
> -	{
> -		.ctl_name	= 99,
> -		.procname	= "exception-trace",
> -		.data		= &exception_trace,
> -		.maxlen		= sizeof(int),
> -		.mode		= 0644,
> -		.proc_handler	= proc_dointvec
> -	},
> -	{}
> -}; 
> -
> -static ctl_table debug_root_table2[] = { 
> -	{
> -		.ctl_name = CTL_DEBUG,
> -		.procname = "debug",
> -		.mode = 0555,
> -		.child = debug_table2
> -	},
> -	{}
> -}; 
> -
> -static __init int x8664_sysctl_init(void)
> -{ 
> -	register_sysctl_table(debug_root_table2);
> -	return 0;
> -}
> -__initcall(x8664_sysctl_init);
> -#endif
> -
>  /* A pseudo VMA to allow ptrace access for the vsyscall page.  This only
>     covers the 64bit vsyscall page now. 32bit has a real VMA now and does
>     not need special handling anymore. */
> diff --git a/include/asm-x86_64/proto.h b/include/asm-x86_64/proto.h
> index 85255db..4fad501 100644
> --- a/include/asm-x86_64/proto.h
> +++ b/include/asm-x86_64/proto.h
> @@ -75,8 +75,6 @@ extern void setup_node_bootmem(int nodeid, unsigned long start, unsigned long en
>  extern void early_quirks(void);
>  extern void check_efer(void);
>  
> -extern int unhandled_signal(struct task_struct *tsk, int sig);
> -
>  extern void select_idle_routine(const struct cpuinfo_x86 *c);
>  
>  extern unsigned long table_start, table_end;
> diff --git a/include/linux/signal.h b/include/linux/signal.h
> index ea91abe..0ae3388 100644
> --- a/include/linux/signal.h
> +++ b/include/linux/signal.h
> @@ -237,12 +237,15 @@ extern int group_send_sig_info(int sig, struct siginfo *info, struct task_struct
>  extern int __group_send_sig_info(int, struct siginfo *, struct task_struct *);
>  extern long do_sigpending(void __user *, unsigned long);
>  extern int sigprocmask(int, sigset_t *, sigset_t *);
> +extern int show_unhandled_signals;
>  
>  struct pt_regs;
>  extern int get_signal_to_deliver(siginfo_t *info, struct k_sigaction *return_ka, struct pt_regs *regs, void *cookie);
>  
>  extern struct kmem_cache *sighand_cachep;
>  
> +int unhandled_signal(struct task_struct *tsk, int sig);
> +
>  /*
>   * In POSIX a signal is sent either to a specific thread (Linux task)
>   * or to the process as a whole (Linux thread group).  How the signal
> diff --git a/kernel/signal.c b/kernel/signal.c
> index 39d1227..ef8156a 100644
> --- a/kernel/signal.c
> +++ b/kernel/signal.c
> @@ -255,6 +255,16 @@ flush_signal_handlers(struct task_struct *t, int force_default)
>  	}
>  }
>  
> +int unhandled_signal(struct task_struct *tsk, int sig)
> +{
> +	if (is_init(tsk))
> +		return 1;
> +	if (tsk->ptrace & PT_PTRACED)
> +		return 0;
> +	return (tsk->sighand->action[sig-1].sa.sa_handler == SIG_IGN) ||
> +		(tsk->sighand->action[sig-1].sa.sa_handler == SIG_DFL);
> +}
> +
>  
>  /* Notify the system that a driver wants to block all signals for this
>   * process, and wants to be notified if any signals at all were to be
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index 7063ebc..af7002f 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -1153,6 +1153,16 @@ static ctl_table fs_table[] = {
>  };
>  
>  static ctl_table debug_table[] = {
> +#ifdef CONFIG_X86
> +	{
> +		.ctl_name	= CTL_UNNUMBERED,
> +		.procname	= "exception-trace",
> +		.data		= &show_unhandled_signals,
> +		.maxlen		= sizeof(int),
> +		.mode		= 0644,
> +		.proc_handler	= proc_dointvec
> +	},
> +#endif
>  	{ .ctl_name = 0 }
>  };
>  
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

Re: i386-show-unhandled-signals-v3

[Masoud Sharbiani]

On 7/25/07, Kirill Korotaev <dev@openvz.org> wrote:
> plz don't enable it by default... :/
> any user can spam syslog with these messages and if syslog is run as root
> can take the whole diskspace...


Yeah, but:
1) Right now (without this patch), it is enabled by default with _no_
rate control in _all_ kernels; I ran the tiny program that is here,
and it wasn't fun to watch.
2) With this patch it will be rate controlled using
printk_ratelimit(), thus reducing the amount of spam immensely.

Of course, we can disable both of them, is this what you (and
everybody else) want?

Masoud

> Thanks,
> Kirill
>
> Masoud Asgharifard Sharbiani wrote:
> > Hello,
> > This patch makes the i386 behave the same way that x86_64 does when a
> > segfault happens.  A line gets printed to the kernel log so that tools
> > that
> > need to check for failures can behave more uniformly between
> > debug.show_unhandled_signals sysctl variable to 0 (or by doing echo 0 >
> > /proc/sys/debug/exception-trace)
> >
> > Also, all of the lines being printed are now using printk_ratelimit() to
> > deny the ability of DoS from a local user with a program like the
> > following:
> >
> > main()
> > {
> >        while (1)
> >                if (!fork()) *(int *)0 = 0;
> > }
> >
> > This new revision also includes the fix that Andrew did which got rid of
> > new sysctl that was added to the system in earlier versions of this.
> > Also, 'show-unhandled-signals' sysctl has been renamed back to the old
> > 'exception-trace' to avoid breakage of people's scripts.
> >
> > cheers,
> > Masoud Sharbiani
> >
> > Signed-off-by: Masoud Sharbiani <masouds@google.com>
> > Cc: Andi Kleen <ak@suse.de>
> >
> > ---
> >  arch/i386/kernel/signal.c   |    7 +++++++
> >  arch/i386/kernel/traps.c    |    7 +++++++
> >  arch/i386/mm/fault.c        |   10 ++++++++++
> >  arch/x86_64/kernel/signal.c |    2 +-
> >  arch/x86_64/kernel/traps.c  |    6 ++++--
> >  arch/x86_64/mm/fault.c      |   15 +++------------
> >  arch/x86_64/mm/init.c       |   35 -----------------------------------
> >  include/asm-x86_64/proto.h  |    2 --
> >  include/linux/signal.h      |    3 +++
> >  kernel/signal.c             |   10 ++++++++++
> >  kernel/sysctl.c             |   10 ++++++++++
> >  11 files changed, 55 insertions(+), 52 deletions(-)
> >
> > diff --git a/arch/i386/kernel/signal.c b/arch/i386/kernel/signal.c
> > index d574e38..f5dd856 100644
> > --- a/arch/i386/kernel/signal.c
> > +++ b/arch/i386/kernel/signal.c
> > @@ -199,6 +199,13 @@ asmlinkage int sys_sigreturn(unsigned long __unused)
> >       return eax;
> >
> >  badframe:
> > +     if (show_unhandled_signals && printk_ratelimit())
> > +             printk("%s%s[%d] bad frame in sigreturn frame:%p eip:%lx"
> > +                    " esp:%lx oeax:%lx\n",
> > +                 current->pid > 1 ? KERN_INFO : KERN_EMERG,
> > +                 current->comm, current->pid, frame, regs->eip,
> > +                 regs->esp, regs->orig_eax);
> > +
> >       force_sig(SIGSEGV, current);
> >       return 0;
> >  }
> > diff --git a/arch/i386/kernel/traps.c b/arch/i386/kernel/traps.c
> > index 18c1c28..c20283c 100644
> > --- a/arch/i386/kernel/traps.c
> > +++ b/arch/i386/kernel/traps.c
> > @@ -611,6 +611,13 @@ fastcall void __kprobes do_general_protection(struct pt_regs * regs,
> >
> >       current->thread.error_code = error_code;
> >       current->thread.trap_no = 13;
> > +     if (show_unhandled_signals && unhandled_signal(current, SIGSEGV) &&
> > +         printk_ratelimit())
> > +             printk(KERN_INFO
> > +                 "%s[%d] general protection eip:%lx esp:%lx error:%lx\n",
> > +                 current->comm, current->pid,
> > +                 regs->eip, regs->esp, error_code);
> > +
> >       force_sig(SIGSEGV, current);
> >       return;
> >
> > diff --git a/arch/i386/mm/fault.c b/arch/i386/mm/fault.c
> > index 1ecb3e4..52c940b 100644
> > --- a/arch/i386/mm/fault.c
> > +++ b/arch/i386/mm/fault.c
> > @@ -283,6 +283,8 @@ static inline int vmalloc_fault(unsigned long address)
> >       return 0;
> >  }
> >
> > +int show_unhandled_signals = 1;
> > +
> >  /*
> >   * This routine handles page faults.  It determines the address,
> >   * and the problem, and then passes it off to one of the appropriate
> > @@ -470,6 +472,14 @@ bad_area_nosemaphore:
> >               if (is_prefetch(regs, address, error_code))
> >                       return;
> >
> > +             if (show_unhandled_signals && unhandled_signal(tsk, SIGSEGV) &&
> > +                 printk_ratelimit()) {
> > +                     printk("%s%s[%d]: segfault at %08lx eip %08lx "
> > +                         "esp %08lx error %lx\n",
> > +                         tsk->pid > 1 ? KERN_INFO : KERN_EMERG,
> > +                         tsk->comm, tsk->pid, address, regs->eip,
> > +                         regs->esp, error_code);
> > +             }
> >               tsk->thread.cr2 = address;
> >               /* Kernel addresses are always protection faults */
> >               tsk->thread.error_code = error_code | (address >= TASK_SIZE);
> > diff --git a/arch/x86_64/kernel/signal.c b/arch/x86_64/kernel/signal.c
> > index 290f5d8..f9506f6 100644
> > --- a/arch/x86_64/kernel/signal.c
> > +++ b/arch/x86_64/kernel/signal.c
> > @@ -480,7 +480,7 @@ do_notify_resume(struct pt_regs *regs, void *unused, __u32 thread_info_flags)
> >  void signal_fault(struct pt_regs *regs, void __user *frame, char *where)
> >  {
> >       struct task_struct *me = current;
> > -     if (exception_trace)
> > +     if (show_unhandled_signals && printk_ratelimit())
> >               printk("%s[%d] bad frame in %s frame:%p rip:%lx rsp:%lx orax:%lx\n",
> >              me->comm,me->pid,where,frame,regs->rip,regs->rsp,regs->orig_rax);
> >
> > diff --git a/arch/x86_64/kernel/traps.c b/arch/x86_64/kernel/traps.c
> > index 74cbeb2..b9660c4 100644
> > --- a/arch/x86_64/kernel/traps.c
> > +++ b/arch/x86_64/kernel/traps.c
> > @@ -580,7 +580,8 @@ static void __kprobes do_trap(int trapnr, int signr, char *str,
> >               tsk->thread.error_code = error_code;
> >               tsk->thread.trap_no = trapnr;
> >
> > -             if (exception_trace && unhandled_signal(tsk, signr))
> > +             if (show_unhandled_signals && unhandled_signal(tsk, signr) &&
> > +                 printk_ratelimit())
> >                       printk(KERN_INFO
> >                              "%s[%d] trap %s rip:%lx rsp:%lx error:%lx\n",
> >                              tsk->comm, tsk->pid, str,
> > @@ -684,7 +685,8 @@ asmlinkage void __kprobes do_general_protection(struct pt_regs * regs,
> >               tsk->thread.error_code = error_code;
> >               tsk->thread.trap_no = 13;
> >
> > -             if (exception_trace && unhandled_signal(tsk, SIGSEGV))
> > +             if (show_unhandled_signals && unhandled_signal(tsk, SIGSEGV) &&
> > +                 printk_ratelimit())
> >                       printk(KERN_INFO
> >                      "%s[%d] general protection rip:%lx rsp:%lx error:%lx\n",
> >                              tsk->comm, tsk->pid,
> > diff --git a/arch/x86_64/mm/fault.c b/arch/x86_64/mm/fault.c
> > index 635e58d..0412824 100644
> > --- a/arch/x86_64/mm/fault.c
> > +++ b/arch/x86_64/mm/fault.c
> > @@ -221,16 +221,6 @@ static int is_errata93(struct pt_regs *regs, unsigned long address)
> >       return 0;
> >  }
> >
> > -int unhandled_signal(struct task_struct *tsk, int sig)
> > -{
> > -     if (is_init(tsk))
> > -             return 1;
> > -     if (tsk->ptrace & PT_PTRACED)
> > -             return 0;
> > -     return (tsk->sighand->action[sig-1].sa.sa_handler == SIG_IGN) ||
> > -             (tsk->sighand->action[sig-1].sa.sa_handler == SIG_DFL);
> > -}
> > -
> >  static noinline void pgtable_bad(unsigned long address, struct pt_regs *regs,
> >                                unsigned long error_code)
> >  {
> > @@ -302,7 +292,7 @@ static int vmalloc_fault(unsigned long address)
> >  }
> >
> >  int page_fault_trace = 0;
> > -int exception_trace = 1;
> > +int show_unhandled_signals = 1;
> >
> >  /*
> >   * This routine handles page faults.  It determines the address,
> > @@ -495,7 +485,8 @@ bad_area_nosemaphore:
> >                   (address >> 32))
> >                       return;
> >
> > -             if (exception_trace && unhandled_signal(tsk, SIGSEGV)) {
> > +             if (show_unhandled_signals && unhandled_signal(tsk, SIGSEGV) &&
> > +                 printk_ratelimit()) {
> >                       printk(
> >                      "%s%s[%d]: segfault at %016lx rip %016lx rsp %016lx error %lx\n",
> >                                       tsk->pid > 1 ? KERN_INFO : KERN_EMERG,
> > diff --git a/arch/x86_64/mm/init.c b/arch/x86_64/mm/init.c
> > index 9a0e98a..5096168 100644
> > --- a/arch/x86_64/mm/init.c
> > +++ b/arch/x86_64/mm/init.c
> > @@ -697,41 +697,6 @@ int kern_addr_valid(unsigned long addr)
> >       return pfn_valid(pte_pfn(*pte));
> >  }
> >
> > -#ifdef CONFIG_SYSCTL
> > -#include <linux/sysctl.h>
> > -
> > -extern int exception_trace, page_fault_trace;
> > -
> > -static ctl_table debug_table2[] = {
> > -     {
> > -             .ctl_name       = 99,
> > -             .procname       = "exception-trace",
> > -             .data           = &exception_trace,
> > -             .maxlen         = sizeof(int),
> > -             .mode           = 0644,
> > -             .proc_handler   = proc_dointvec
> > -     },
> > -     {}
> > -};
> > -
> > -static ctl_table debug_root_table2[] = {
> > -     {
> > -             .ctl_name = CTL_DEBUG,
> > -             .procname = "debug",
> > -             .mode = 0555,
> > -             .child = debug_table2
> > -     },
> > -     {}
> > -};
> > -
> > -static __init int x8664_sysctl_init(void)
> > -{
> > -     register_sysctl_table(debug_root_table2);
> > -     return 0;
> > -}
> > -__initcall(x8664_sysctl_init);
> > -#endif
> > -
> >  /* A pseudo VMA to allow ptrace access for the vsyscall page.  This only
> >     covers the 64bit vsyscall page now. 32bit has a real VMA now and does
> >     not need special handling anymore. */
> > diff --git a/include/asm-x86_64/proto.h b/include/asm-x86_64/proto.h
> > index 85255db..4fad501 100644
> > --- a/include/asm-x86_64/proto.h
> > +++ b/include/asm-x86_64/proto.h
> > @@ -75,8 +75,6 @@ extern void setup_node_bootmem(int nodeid, unsigned long start, unsigned long en
> >  extern void early_quirks(void);
> >  extern void check_efer(void);
> >
> > -extern int unhandled_signal(struct task_struct *tsk, int sig);
> > -
> >  extern void select_idle_routine(const struct cpuinfo_x86 *c);
> >
> >  extern unsigned long table_start, table_end;
> > diff --git a/include/linux/signal.h b/include/linux/signal.h
> > index ea91abe..0ae3388 100644
> > --- a/include/linux/signal.h
> > +++ b/include/linux/signal.h
> > @@ -237,12 +237,15 @@ extern int group_send_sig_info(int sig, struct siginfo *info, struct task_struct
> >  extern int __group_send_sig_info(int, struct siginfo *, struct task_struct *);
> >  extern long do_sigpending(void __user *, unsigned long);
> >  extern int sigprocmask(int, sigset_t *, sigset_t *);
> > +extern int show_unhandled_signals;
> >
> >  struct pt_regs;
> >  extern int get_signal_to_deliver(siginfo_t *info, struct k_sigaction *return_ka, struct pt_regs *regs, void *cookie);
> >
> >  extern struct kmem_cache *sighand_cachep;
> >
> > +int unhandled_signal(struct task_struct *tsk, int sig);
> > +
> >  /*
> >   * In POSIX a signal is sent either to a specific thread (Linux task)
> >   * or to the process as a whole (Linux thread group).  How the signal
> > diff --git a/kernel/signal.c b/kernel/signal.c
> > index 39d1227..ef8156a 100644
> > --- a/kernel/signal.c
> > +++ b/kernel/signal.c
> > @@ -255,6 +255,16 @@ flush_signal_handlers(struct task_struct *t, int force_default)
> >       }
> >  }
> >
> > +int unhandled_signal(struct task_struct *tsk, int sig)
> > +{
> > +     if (is_init(tsk))
> > +             return 1;
> > +     if (tsk->ptrace & PT_PTRACED)
> > +             return 0;
> > +     return (tsk->sighand->action[sig-1].sa.sa_handler == SIG_IGN) ||
> > +             (tsk->sighand->action[sig-1].sa.sa_handler == SIG_DFL);
> > +}
> > +
> >
> >  /* Notify the system that a driver wants to block all signals for this
> >   * process, and wants to be notified if any signals at all were to be
> > diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> > index 7063ebc..af7002f 100644
> > --- a/kernel/sysctl.c
> > +++ b/kernel/sysctl.c
> > @@ -1153,6 +1153,16 @@ static ctl_table fs_table[] = {
> >  };
> >
> >  static ctl_table debug_table[] = {
> > +#ifdef CONFIG_X86
> > +     {
> > +             .ctl_name       = CTL_UNNUMBERED,
> > +             .procname       = "exception-trace",
> > +             .data           = &show_unhandled_signals,
> > +             .maxlen         = sizeof(int),
> > +             .mode           = 0644,
> > +             .proc_handler   = proc_dointvec
> > +     },
> > +#endif
> >       { .ctl_name = 0 }
> >  };
> >
> > -
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/
> >
>
>

Re: i386-show-unhandled-signals-v3

[Kirill Korotaev]

Masoud Sharbiani wrote:
> On 7/25/07, Kirill Korotaev <dev@openvz.org> wrote:
> 
>>plz don't enable it by default... :/
>>any user can spam syslog with these messages and if syslog is run as root
>>can take the whole diskspace...
> 
> 
> 
> Yeah, but:
> 1) Right now (without this patch), it is enabled by default with _no_
> rate control in _all_ kernels; I ran the tiny program that is here,
> and it wasn't fun to watch.

agree. we disable it in OpenVZ kernels due to this.

> 2) With this patch it will be rate controlled using
> printk_ratelimit(), thus reducing the amount of spam immensely.
> 
> Of course, we can disable both of them, is this what you (and
> everybody else) want?

As for me - I would vote for disabling this by default.
If people vote for leaving it ON, then ratelimit is a must imho.

Thanks,
Kirill

> Masoud
> 
> 
>>Thanks,
>>Kirill
>>
>>Masoud Asgharifard Sharbiani wrote:
>>
>>>Hello,
>>>This patch makes the i386 behave the same way that x86_64 does when a
>>>segfault happens.  A line gets printed to the kernel log so that tools
>>>that
>>>need to check for failures can behave more uniformly between
>>>debug.show_unhandled_signals sysctl variable to 0 (or by doing echo 0 >
>>>/proc/sys/debug/exception-trace)
>>>
>>>Also, all of the lines being printed are now using printk_ratelimit() to
>>>deny the ability of DoS from a local user with a program like the
>>>following:
>>>
>>>main()
>>>{
>>>       while (1)
>>>               if (!fork()) *(int *)0 = 0;
>>>}
>>>
>>>This new revision also includes the fix that Andrew did which got rid of
>>>new sysctl that was added to the system in earlier versions of this.
>>>Also, 'show-unhandled-signals' sysctl has been renamed back to the old
>>>'exception-trace' to avoid breakage of people's scripts.
>>>
>>>cheers,
>>>Masoud Sharbiani
>>>
>>>Signed-off-by: Masoud Sharbiani <masouds@google.com>
>>>Cc: Andi Kleen <ak@suse.de>
>>>
>>>---
>>> arch/i386/kernel/signal.c   |    7 +++++++
>>> arch/i386/kernel/traps.c    |    7 +++++++
>>> arch/i386/mm/fault.c        |   10 ++++++++++
>>> arch/x86_64/kernel/signal.c |    2 +-
>>> arch/x86_64/kernel/traps.c  |    6 ++++--
>>> arch/x86_64/mm/fault.c      |   15 +++------------
>>> arch/x86_64/mm/init.c       |   35 -----------------------------------
>>> include/asm-x86_64/proto.h  |    2 --
>>> include/linux/signal.h      |    3 +++
>>> kernel/signal.c             |   10 ++++++++++
>>> kernel/sysctl.c             |   10 ++++++++++
>>> 11 files changed, 55 insertions(+), 52 deletions(-)
>>>
>>>diff --git a/arch/i386/kernel/signal.c b/arch/i386/kernel/signal.c
>>>index d574e38..f5dd856 100644
>>>--- a/arch/i386/kernel/signal.c
>>>+++ b/arch/i386/kernel/signal.c
>>>@@ -199,6 +199,13 @@ asmlinkage int sys_sigreturn(unsigned long __unused)
>>>      return eax;
>>>
>>> badframe:
>>>+     if (show_unhandled_signals && printk_ratelimit())
>>>+             printk("%s%s[%d] bad frame in sigreturn frame:%p eip:%lx"
>>>+                    " esp:%lx oeax:%lx\n",
>>>+                 current->pid > 1 ? KERN_INFO : KERN_EMERG,
>>>+                 current->comm, current->pid, frame, regs->eip,
>>>+                 regs->esp, regs->orig_eax);
>>>+
>>>      force_sig(SIGSEGV, current);
>>>      return 0;
>>> }
>>>diff --git a/arch/i386/kernel/traps.c b/arch/i386/kernel/traps.c
>>>index 18c1c28..c20283c 100644
>>>--- a/arch/i386/kernel/traps.c
>>>+++ b/arch/i386/kernel/traps.c
>>>@@ -611,6 +611,13 @@ fastcall void __kprobes do_general_protection(struct pt_regs * regs,
>>>
>>>      current->thread.error_code = error_code;
>>>      current->thread.trap_no = 13;
>>>+     if (show_unhandled_signals && unhandled_signal(current, SIGSEGV) &&
>>>+         printk_ratelimit())
>>>+             printk(KERN_INFO
>>>+                 "%s[%d] general protection eip:%lx esp:%lx error:%lx\n",
>>>+                 current->comm, current->pid,
>>>+                 regs->eip, regs->esp, error_code);
>>>+
>>>      force_sig(SIGSEGV, current);
>>>      return;
>>>
>>>diff --git a/arch/i386/mm/fault.c b/arch/i386/mm/fault.c
>>>index 1ecb3e4..52c940b 100644
>>>--- a/arch/i386/mm/fault.c
>>>+++ b/arch/i386/mm/fault.c
>>>@@ -283,6 +283,8 @@ static inline int vmalloc_fault(unsigned long address)
>>>      return 0;
>>> }
>>>
>>>+int show_unhandled_signals = 1;
>>>+
>>> /*
>>>  * This routine handles page faults.  It determines the address,
>>>  * and the problem, and then passes it off to one of the appropriate
>>>@@ -470,6 +472,14 @@ bad_area_nosemaphore:
>>>              if (is_prefetch(regs, address, error_code))
>>>                      return;
>>>
>>>+             if (show_unhandled_signals && unhandled_signal(tsk, SIGSEGV) &&
>>>+                 printk_ratelimit()) {
>>>+                     printk("%s%s[%d]: segfault at %08lx eip %08lx "
>>>+                         "esp %08lx error %lx\n",
>>>+                         tsk->pid > 1 ? KERN_INFO : KERN_EMERG,
>>>+                         tsk->comm, tsk->pid, address, regs->eip,
>>>+                         regs->esp, error_code);
>>>+             }
>>>              tsk->thread.cr2 = address;
>>>              /* Kernel addresses are always protection faults */
>>>              tsk->thread.error_code = error_code | (address >= TASK_SIZE);
>>>diff --git a/arch/x86_64/kernel/signal.c b/arch/x86_64/kernel/signal.c
>>>index 290f5d8..f9506f6 100644
>>>--- a/arch/x86_64/kernel/signal.c
>>>+++ b/arch/x86_64/kernel/signal.c
>>>@@ -480,7 +480,7 @@ do_notify_resume(struct pt_regs *regs, void *unused, __u32 thread_info_flags)
>>> void signal_fault(struct pt_regs *regs, void __user *frame, char *where)
>>> {
>>>      struct task_struct *me = current;
>>>-     if (exception_trace)
>>>+     if (show_unhandled_signals && printk_ratelimit())
>>>              printk("%s[%d] bad frame in %s frame:%p rip:%lx rsp:%lx orax:%lx\n",
>>>             me->comm,me->pid,where,frame,regs->rip,regs->rsp,regs->orig_rax);
>>>
>>>diff --git a/arch/x86_64/kernel/traps.c b/arch/x86_64/kernel/traps.c
>>>index 74cbeb2..b9660c4 100644
>>>--- a/arch/x86_64/kernel/traps.c
>>>+++ b/arch/x86_64/kernel/traps.c
>>>@@ -580,7 +580,8 @@ static void __kprobes do_trap(int trapnr, int signr, char *str,
>>>              tsk->thread.error_code = error_code;
>>>              tsk->thread.trap_no = trapnr;
>>>
>>>-             if (exception_trace && unhandled_signal(tsk, signr))
>>>+             if (show_unhandled_signals && unhandled_signal(tsk, signr) &&
>>>+                 printk_ratelimit())
>>>                      printk(KERN_INFO
>>>                             "%s[%d] trap %s rip:%lx rsp:%lx error:%lx\n",
>>>                             tsk->comm, tsk->pid, str,
>>>@@ -684,7 +685,8 @@ asmlinkage void __kprobes do_general_protection(struct pt_regs * regs,
>>>              tsk->thread.error_code = error_code;
>>>              tsk->thread.trap_no = 13;
>>>
>>>-             if (exception_trace && unhandled_signal(tsk, SIGSEGV))
>>>+             if (show_unhandled_signals && unhandled_signal(tsk, SIGSEGV) &&
>>>+                 printk_ratelimit())
>>>                      printk(KERN_INFO
>>>                     "%s[%d] general protection rip:%lx rsp:%lx error:%lx\n",
>>>                             tsk->comm, tsk->pid,
>>>diff --git a/arch/x86_64/mm/fault.c b/arch/x86_64/mm/fault.c
>>>index 635e58d..0412824 100644
>>>--- a/arch/x86_64/mm/fault.c
>>>+++ b/arch/x86_64/mm/fault.c
>>>@@ -221,16 +221,6 @@ static int is_errata93(struct pt_regs *regs, unsigned long address)
>>>      return 0;
>>> }
>>>
>>>-int unhandled_signal(struct task_struct *tsk, int sig)
>>>-{
>>>-     if (is_init(tsk))
>>>-             return 1;
>>>-     if (tsk->ptrace & PT_PTRACED)
>>>-             return 0;
>>>-     return (tsk->sighand->action[sig-1].sa.sa_handler == SIG_IGN) ||
>>>-             (tsk->sighand->action[sig-1].sa.sa_handler == SIG_DFL);
>>>-}
>>>-
>>> static noinline void pgtable_bad(unsigned long address, struct pt_regs *regs,
>>>                               unsigned long error_code)
>>> {
>>>@@ -302,7 +292,7 @@ static int vmalloc_fault(unsigned long address)
>>> }
>>>
>>> int page_fault_trace = 0;
>>>-int exception_trace = 1;
>>>+int show_unhandled_signals = 1;
>>>
>>> /*
>>>  * This routine handles page faults.  It determines the address,
>>>@@ -495,7 +485,8 @@ bad_area_nosemaphore:
>>>                  (address >> 32))
>>>                      return;
>>>
>>>-             if (exception_trace && unhandled_signal(tsk, SIGSEGV)) {
>>>+             if (show_unhandled_signals && unhandled_signal(tsk, SIGSEGV) &&
>>>+                 printk_ratelimit()) {
>>>                      printk(
>>>                     "%s%s[%d]: segfault at %016lx rip %016lx rsp %016lx error %lx\n",
>>>                                      tsk->pid > 1 ? KERN_INFO : KERN_EMERG,
>>>diff --git a/arch/x86_64/mm/init.c b/arch/x86_64/mm/init.c
>>>index 9a0e98a..5096168 100644
>>>--- a/arch/x86_64/mm/init.c
>>>+++ b/arch/x86_64/mm/init.c
>>>@@ -697,41 +697,6 @@ int kern_addr_valid(unsigned long addr)
>>>      return pfn_valid(pte_pfn(*pte));
>>> }
>>>
>>>-#ifdef CONFIG_SYSCTL
>>>-#include <linux/sysctl.h>
>>>-
>>>-extern int exception_trace, page_fault_trace;
>>>-
>>>-static ctl_table debug_table2[] = {
>>>-     {
>>>-             .ctl_name       = 99,
>>>-             .procname       = "exception-trace",
>>>-             .data           = &exception_trace,
>>>-             .maxlen         = sizeof(int),
>>>-             .mode           = 0644,
>>>-             .proc_handler   = proc_dointvec
>>>-     },
>>>-     {}
>>>-};
>>>-
>>>-static ctl_table debug_root_table2[] = {
>>>-     {
>>>-             .ctl_name = CTL_DEBUG,
>>>-             .procname = "debug",
>>>-             .mode = 0555,
>>>-             .child = debug_table2
>>>-     },
>>>-     {}
>>>-};
>>>-
>>>-static __init int x8664_sysctl_init(void)
>>>-{
>>>-     register_sysctl_table(debug_root_table2);
>>>-     return 0;
>>>-}
>>>-__initcall(x8664_sysctl_init);
>>>-#endif
>>>-
>>> /* A pseudo VMA to allow ptrace access for the vsyscall page.  This only
>>>    covers the 64bit vsyscall page now. 32bit has a real VMA now and does
>>>    not need special handling anymore. */
>>>diff --git a/include/asm-x86_64/proto.h b/include/asm-x86_64/proto.h
>>>index 85255db..4fad501 100644
>>>--- a/include/asm-x86_64/proto.h
>>>+++ b/include/asm-x86_64/proto.h
>>>@@ -75,8 +75,6 @@ extern void setup_node_bootmem(int nodeid, unsigned long start, unsigned long en
>>> extern void early_quirks(void);
>>> extern void check_efer(void);
>>>
>>>-extern int unhandled_signal(struct task_struct *tsk, int sig);
>>>-
>>> extern void select_idle_routine(const struct cpuinfo_x86 *c);
>>>
>>> extern unsigned long table_start, table_end;
>>>diff --git a/include/linux/signal.h b/include/linux/signal.h
>>>index ea91abe..0ae3388 100644
>>>--- a/include/linux/signal.h
>>>+++ b/include/linux/signal.h
>>>@@ -237,12 +237,15 @@ extern int group_send_sig_info(int sig, struct siginfo *info, struct task_struct
>>> extern int __group_send_sig_info(int, struct siginfo *, struct task_struct *);
>>> extern long do_sigpending(void __user *, unsigned long);
>>> extern int sigprocmask(int, sigset_t *, sigset_t *);
>>>+extern int show_unhandled_signals;
>>>
>>> struct pt_regs;
>>> extern int get_signal_to_deliver(siginfo_t *info, struct k_sigaction *return_ka, struct pt_regs *regs, void *cookie);
>>>
>>> extern struct kmem_cache *sighand_cachep;
>>>
>>>+int unhandled_signal(struct task_struct *tsk, int sig);
>>>+
>>> /*
>>>  * In POSIX a signal is sent either to a specific thread (Linux task)
>>>  * or to the process as a whole (Linux thread group).  How the signal
>>>diff --git a/kernel/signal.c b/kernel/signal.c
>>>index 39d1227..ef8156a 100644
>>>--- a/kernel/signal.c
>>>+++ b/kernel/signal.c
>>>@@ -255,6 +255,16 @@ flush_signal_handlers(struct task_struct *t, int force_default)
>>>      }
>>> }
>>>
>>>+int unhandled_signal(struct task_struct *tsk, int sig)
>>>+{
>>>+     if (is_init(tsk))
>>>+             return 1;
>>>+     if (tsk->ptrace & PT_PTRACED)
>>>+             return 0;
>>>+     return (tsk->sighand->action[sig-1].sa.sa_handler == SIG_IGN) ||
>>>+             (tsk->sighand->action[sig-1].sa.sa_handler == SIG_DFL);
>>>+}
>>>+
>>>
>>> /* Notify the system that a driver wants to block all signals for this
>>>  * process, and wants to be notified if any signals at all were to be
>>>diff --git a/kernel/sysctl.c b/kernel/sysctl.c
>>>index 7063ebc..af7002f 100644
>>>--- a/kernel/sysctl.c
>>>+++ b/kernel/sysctl.c
>>>@@ -1153,6 +1153,16 @@ static ctl_table fs_table[] = {
>>> };
>>>
>>> static ctl_table debug_table[] = {
>>>+#ifdef CONFIG_X86
>>>+     {
>>>+             .ctl_name       = CTL_UNNUMBERED,
>>>+             .procname       = "exception-trace",
>>>+             .data           = &show_unhandled_signals,
>>>+             .maxlen         = sizeof(int),
>>>+             .mode           = 0644,
>>>+             .proc_handler   = proc_dointvec
>>>+     },
>>>+#endif
>>>      { .ctl_name = 0 }
>>> };
>>>
>>>-
>>>To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>>>the body of a message to majordomo@vger.kernel.org
>>>More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>Please read the FAQ at  http://www.tux.org/lkml/
>>>
>>
>>
> 

Re: i386-show-unhandled-signals-v3

[Andi Kleen]

On Wednesday 25 July 2007 16:45, Kirill Korotaev wrote:
> plz don't enable it by default... :/
> any user can spam syslog with these messages and if syslog is run as root
> can take the whole diskspace...

There are plenty of other ways to cause syslog messages anyways; this argument 
is 100% bogus.

-Andi

Re: i386-show-unhandled-signals-v3

[Andrew Morton]

On Wed, 25 Jul 2007 16:57:43 +0200
Andi Kleen <ak@suse.de> wrote:

> On Wednesday 25 July 2007 16:45, Kirill Korotaev wrote:
> > plz don't enable it by default... :/
> > any user can spam syslog with these messages and if syslog is run as root
> > can take the whole diskspace...
> 
> There are plenty of other ways to cause syslog messages anyways;

tell us what they are and we'll fix them?

> this argument is 100% bogus.

people don't like leaving themselves open to logspamming.


For this particular issue: someone please send a patch.

Re: i386-show-unhandled-signals-v3

[Masoud Sharbiani]

On 7/25/07, Andrew Morton <akpm@linux-foundation.org> wrote:
> On Wed, 25 Jul 2007 16:57:43 +0200
> Andi Kleen <ak@suse.de> wrote:
>
> > On Wednesday 25 July 2007 16:45, Kirill Korotaev wrote:
> > > plz don't enable it by default... :/
> > > any user can spam syslog with these messages and if syslog is run as root
> > > can take the whole diskspace...
> >
> > There are plenty of other ways to cause syslog messages anyways;
>
> tell us what they are and we'll fix them?
>
> > this argument is 100% bogus.
>
> people don't like leaving themselves open to logspamming.
>
>
> For this particular issue: someone please send a patch.
>
Andrew,
This is rate limited; Do you need me to rewrite it with it being
disabled by default?

Masoud

Re: i386-show-unhandled-signals-v3

[Andrew Morton]

On Wed, 25 Jul 2007 14:07:56 -0700
"Masoud Sharbiani" <masouds@google.com> wrote:

> On 7/25/07, Andrew Morton <akpm@linux-foundation.org> wrote:
> > On Wed, 25 Jul 2007 16:57:43 +0200
> > Andi Kleen <ak@suse.de> wrote:
> >
> > > On Wednesday 25 July 2007 16:45, Kirill Korotaev wrote:
> > > > plz don't enable it by default... :/
> > > > any user can spam syslog with these messages and if syslog is run as root
> > > > can take the whole diskspace...
> > >
> > > There are plenty of other ways to cause syslog messages anyways;
> >
> > tell us what they are and we'll fix them?
> >
> > > this argument is 100% bogus.
> >
> > people don't like leaving themselves open to logspamming.
> >
> >
> > For this particular issue: someone please send a patch.
> >
> Andrew,
> This is rate limited; Do you need me to rewrite it with it being
> disabled by default?
> 

Yes please.

Look: if there's a way in which an unprivileged user can trigger a printk
we fix it, end of story.  I don't know why this even slightly
controversial.

Re: i386-show-unhandled-signals-v3

[Masoud Asgharifard Sharbiani]

On Wed, Jul 25, 2007 at 04:25:28PM -0700, Andrew Morton wrote:
> On Wed, 25 Jul 2007 14:07:56 -0700
> "Masoud Sharbiani" <masouds@google.com> wrote:
> 
> > On 7/25/07, Andrew Morton <akpm@linux-foundation.org> wrote:
> > > On Wed, 25 Jul 2007 16:57:43 +0200
> > > Andi Kleen <ak@suse.de> wrote:
> > >
> > > > On Wednesday 25 July 2007 16:45, Kirill Korotaev wrote:
> > > > > plz don't enable it by default... :/
> > > > > any user can spam syslog with these messages and if syslog is run as root
> > > > > can take the whole diskspace...
> > > >
> > > > There are plenty of other ways to cause syslog messages anyways;
> > >
> > > tell us what they are and we'll fix them?
> > >
> > > > this argument is 100% bogus.
> > >
> > > people don't like leaving themselves open to logspamming.
> > >
> > >
> > > For this particular issue: someone please send a patch.
> > >
> > Andrew,
> > This is rate limited; Do you need me to rewrite it with it being
> > disabled by default?
> > 
> 
> Yes please.
> 
> Look: if there's a way in which an unprivileged user can trigger a printk
> we fix it, end of story.  I don't know why this even slightly
> controversial.
> 

Fair enough. Here it is:
---------------
Hello,
This patch makes the i386 behave the same way that x86_64 does when a
segfault happens. A line gets printed to the kernel log so that tools
that need to check for failures can behave more uniformly between
different kernels. Like x86_64, it can be disabled by setting
debug.show_unhandled_signals sysctl variable to 0 (or by doing
echo 0 > /proc/sys/debug/show_unhandled_signals)

Also, all of the lines being printed are now using printk_ratelimit()
to deny the ability of DoS from a local user with a program like the
following:
main()
{
       while (1)
               if (!fork()) *(int *)0 = 0;
}


cheers,
Masoud

Signed-off-by: Masoud Sharbiani <masouds@google.com

diff --git a/arch/i386/kernel/signal.c b/arch/i386/kernel/signal.c
index d574e38..f5dd856 100644
--- a/arch/i386/kernel/signal.c
+++ b/arch/i386/kernel/signal.c
@@ -199,6 +199,13 @@ asmlinkage int sys_sigreturn(unsigned long __unused)
 	return eax;
 
 badframe:
+	if (show_unhandled_signals && printk_ratelimit())
+		printk("%s%s[%d] bad frame in sigreturn frame:%p eip:%lx"
+		       " esp:%lx oeax:%lx\n",
+		    current->pid > 1 ? KERN_INFO : KERN_EMERG,
+		    current->comm, current->pid, frame, regs->eip,
+		    regs->esp, regs->orig_eax);
+
 	force_sig(SIGSEGV, current);
 	return 0;
 }	
diff --git a/arch/i386/kernel/traps.c b/arch/i386/kernel/traps.c
index 18c1c28..c20283c 100644
--- a/arch/i386/kernel/traps.c
+++ b/arch/i386/kernel/traps.c
@@ -611,6 +611,13 @@ fastcall void __kprobes do_general_protection(struct pt_regs * regs,
 
 	current->thread.error_code = error_code;
 	current->thread.trap_no = 13;
+	if (show_unhandled_signals && unhandled_signal(current, SIGSEGV) &&
+	    printk_ratelimit())
+		printk(KERN_INFO
+		    "%s[%d] general protection eip:%lx esp:%lx error:%lx\n",
+		    current->comm, current->pid,
+		    regs->eip, regs->esp, error_code);
+
 	force_sig(SIGSEGV, current);
 	return;
 
diff --git a/arch/i386/mm/fault.c b/arch/i386/mm/fault.c
index 1ecb3e4..52c940b 100644
--- a/arch/i386/mm/fault.c
+++ b/arch/i386/mm/fault.c
@@ -283,6 +283,8 @@ static inline int vmalloc_fault(unsigned long address)
 	return 0;
 }
 
+int show_unhandled_signals = 0;
+
 /*
  * This routine handles page faults.  It determines the address,
  * and the problem, and then passes it off to one of the appropriate
@@ -470,6 +472,14 @@ bad_area_nosemaphore:
 		if (is_prefetch(regs, address, error_code))
 			return;
 
+		if (show_unhandled_signals && unhandled_signal(tsk, SIGSEGV) &&
+		    printk_ratelimit()) {
+			printk("%s%s[%d]: segfault at %08lx eip %08lx "
+			    "esp %08lx error %lx\n",
+			    tsk->pid > 1 ? KERN_INFO : KERN_EMERG,
+			    tsk->comm, tsk->pid, address, regs->eip,
+			    regs->esp, error_code);
+		}
 		tsk->thread.cr2 = address;
 		/* Kernel addresses are always protection faults */
 		tsk->thread.error_code = error_code | (address >= TASK_SIZE);
diff --git a/arch/x86_64/kernel/signal.c b/arch/x86_64/kernel/signal.c
index 290f5d8..f9506f6 100644
--- a/arch/x86_64/kernel/signal.c
+++ b/arch/x86_64/kernel/signal.c
@@ -480,7 +480,7 @@ do_notify_resume(struct pt_regs *regs, void *unused, __u32 thread_info_flags)
 void signal_fault(struct pt_regs *regs, void __user *frame, char *where)
 { 
 	struct task_struct *me = current; 
-	if (exception_trace)
+	if (show_unhandled_signals && printk_ratelimit())
 		printk("%s[%d] bad frame in %s frame:%p rip:%lx rsp:%lx orax:%lx\n",
 	       me->comm,me->pid,where,frame,regs->rip,regs->rsp,regs->orig_rax); 
 
diff --git a/arch/x86_64/kernel/traps.c b/arch/x86_64/kernel/traps.c
index 74cbeb2..b9660c4 100644
--- a/arch/x86_64/kernel/traps.c
+++ b/arch/x86_64/kernel/traps.c
@@ -580,7 +580,8 @@ static void __kprobes do_trap(int trapnr, int signr, char *str,
 		tsk->thread.error_code = error_code;
 		tsk->thread.trap_no = trapnr;
 
-		if (exception_trace && unhandled_signal(tsk, signr))
+		if (show_unhandled_signals && unhandled_signal(tsk, signr) &&
+		    printk_ratelimit())
 			printk(KERN_INFO
 			       "%s[%d] trap %s rip:%lx rsp:%lx error:%lx\n",
 			       tsk->comm, tsk->pid, str,
@@ -684,7 +685,8 @@ asmlinkage void __kprobes do_general_protection(struct pt_regs * regs,
 		tsk->thread.error_code = error_code;
 		tsk->thread.trap_no = 13;
 
-		if (exception_trace && unhandled_signal(tsk, SIGSEGV))
+		if (show_unhandled_signals && unhandled_signal(tsk, SIGSEGV) &&
+		    printk_ratelimit())
 			printk(KERN_INFO
 		       "%s[%d] general protection rip:%lx rsp:%lx error:%lx\n",
 			       tsk->comm, tsk->pid,
diff --git a/arch/x86_64/mm/fault.c b/arch/x86_64/mm/fault.c
index 635e58d..0412824 100644
--- a/arch/x86_64/mm/fault.c
+++ b/arch/x86_64/mm/fault.c
@@ -221,16 +221,6 @@ static int is_errata93(struct pt_regs *regs, unsigned long address)
 	return 0;
 } 
 
-int unhandled_signal(struct task_struct *tsk, int sig)
-{
-	if (is_init(tsk))
-		return 1;
-	if (tsk->ptrace & PT_PTRACED)
-		return 0;
-	return (tsk->sighand->action[sig-1].sa.sa_handler == SIG_IGN) ||
-		(tsk->sighand->action[sig-1].sa.sa_handler == SIG_DFL);
-}
-
 static noinline void pgtable_bad(unsigned long address, struct pt_regs *regs,
 				 unsigned long error_code)
 {
@@ -302,7 +292,7 @@ static int vmalloc_fault(unsigned long address)
 }
 
 int page_fault_trace = 0;
-int exception_trace = 1;
+int show_unhandled_signals = 0;
 
 /*
  * This routine handles page faults.  It determines the address,
@@ -495,7 +485,8 @@ bad_area_nosemaphore:
 		    (address >> 32))
 			return;
 
-		if (exception_trace && unhandled_signal(tsk, SIGSEGV)) {
+		if (show_unhandled_signals && unhandled_signal(tsk, SIGSEGV) &&
+		    printk_ratelimit()) {
 			printk(
 		       "%s%s[%d]: segfault at %016lx rip %016lx rsp %016lx error %lx\n",
 					tsk->pid > 1 ? KERN_INFO : KERN_EMERG,
diff --git a/arch/x86_64/mm/init.c b/arch/x86_64/mm/init.c
index 9a0e98a..5096168 100644
--- a/arch/x86_64/mm/init.c
+++ b/arch/x86_64/mm/init.c
@@ -697,41 +697,6 @@ int kern_addr_valid(unsigned long addr)
 	return pfn_valid(pte_pfn(*pte));
 }
 
-#ifdef CONFIG_SYSCTL
-#include <linux/sysctl.h>
-
-extern int exception_trace, page_fault_trace;
-
-static ctl_table debug_table2[] = {
-	{
-		.ctl_name	= 99,
-		.procname	= "exception-trace",
-		.data		= &exception_trace,
-		.maxlen		= sizeof(int),
-		.mode		= 0644,
-		.proc_handler	= proc_dointvec
-	},
-	{}
-}; 
-
-static ctl_table debug_root_table2[] = { 
-	{
-		.ctl_name = CTL_DEBUG,
-		.procname = "debug",
-		.mode = 0555,
-		.child = debug_table2
-	},
-	{}
-}; 
-
-static __init int x8664_sysctl_init(void)
-{ 
-	register_sysctl_table(debug_root_table2);
-	return 0;
-}
-__initcall(x8664_sysctl_init);
-#endif
-
 /* A pseudo VMA to allow ptrace access for the vsyscall page.  This only
    covers the 64bit vsyscall page now. 32bit has a real VMA now and does
    not need special handling anymore. */
diff --git a/include/asm-x86_64/proto.h b/include/asm-x86_64/proto.h
index 85255db..4fad501 100644
--- a/include/asm-x86_64/proto.h
+++ b/include/asm-x86_64/proto.h
@@ -75,8 +75,6 @@ extern void setup_node_bootmem(int nodeid, unsigned long start, unsigned long en
 extern void early_quirks(void);
 extern void check_efer(void);
 
-extern int unhandled_signal(struct task_struct *tsk, int sig);
-
 extern void select_idle_routine(const struct cpuinfo_x86 *c);
 
 extern unsigned long table_start, table_end;
diff --git a/include/linux/signal.h b/include/linux/signal.h
index ea91abe..0ae3388 100644
--- a/include/linux/signal.h
+++ b/include/linux/signal.h
@@ -237,12 +237,15 @@ extern int group_send_sig_info(int sig, struct siginfo *info, struct task_struct
 extern int __group_send_sig_info(int, struct siginfo *, struct task_struct *);
 extern long do_sigpending(void __user *, unsigned long);
 extern int sigprocmask(int, sigset_t *, sigset_t *);
+extern int show_unhandled_signals;
 
 struct pt_regs;
 extern int get_signal_to_deliver(siginfo_t *info, struct k_sigaction *return_ka, struct pt_regs *regs, void *cookie);
 
 extern struct kmem_cache *sighand_cachep;
 
+int unhandled_signal(struct task_struct *tsk, int sig);
+
 /*
  * In POSIX a signal is sent either to a specific thread (Linux task)
  * or to the process as a whole (Linux thread group).  How the signal
diff --git a/kernel/signal.c b/kernel/signal.c
index 39d1227..ef8156a 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -255,6 +255,16 @@ flush_signal_handlers(struct task_struct *t, int force_default)
 	}
 }
 
+int unhandled_signal(struct task_struct *tsk, int sig)
+{
+	if (is_init(tsk))
+		return 1;
+	if (tsk->ptrace & PT_PTRACED)
+		return 0;
+	return (tsk->sighand->action[sig-1].sa.sa_handler == SIG_IGN) ||
+		(tsk->sighand->action[sig-1].sa.sa_handler == SIG_DFL);
+}
+
 
 /* Notify the system that a driver wants to block all signals for this
  * process, and wants to be notified if any signals at all were to be
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 7063ebc..af7002f 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -1153,6 +1153,16 @@ static ctl_table fs_table[] = {
 };
 
 static ctl_table debug_table[] = {
+#ifdef CONFIG_X86
+	{
+		.ctl_name	= CTL_UNNUMBERED,
+		.procname	= "exception_trace",
+		.data		= &show_unhandled_signals,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= proc_dointvec
+	},
+#endif
 	{ .ctl_name = 0 }
 };
 

Re: i386-show-unhandled-signals-v3

[Andrew Morton]

On Wed, 25 Jul 2007 16:40:06 -0700
masouds@google.com (Masoud Asgharifard Sharbiani) wrote:

> > Look: if there's a way in which an unprivileged user can trigger a printk
> > we fix it, end of story.  I don't know why this even slightly
> > controversial.
> > 
> 
> Fair enough. Here it is:

My favourite words.

> ---------------
> Hello,
> This patch makes the i386 behave the same way that x86_64 does when a
> segfault happens. A line gets printed to the kernel log so that tools
> that need to check for failures can behave more uniformly between
> different kernels. Like x86_64, it can be disabled by setting
> debug.show_unhandled_signals sysctl variable to 0 (or by doing
> echo 0 > /proc/sys/debug/show_unhandled_signals)

Do we really need the ratelimiting?  If the admin turns this on then he's
presumably prepared for the consequences.

I guess "yes", as people (even distros) are likely to turn this on and
forget about it.

The patch is larger than I expected, ho hum.

Re: i386-show-unhandled-signals-v3

[Masoud Sharbiani]

On 7/25/07, Andrew Morton <akpm@linux-foundation.org> wrote:
> On Wed, 25 Jul 2007 16:40:06 -0700
> masouds@google.com (Masoud Asgharifard Sharbiani) wrote:
>
> > > Look: if there's a way in which an unprivileged user can trigger a printk
> > > we fix it, end of story.  I don't know why this even slightly
> > > controversial.
> > >
> >
> > Fair enough. Here it is:
>
> My favourite words.
>
> > ---------------
> > Hello,
> > This patch makes the i386 behave the same way that x86_64 does when a
> > segfault happens. A line gets printed to the kernel log so that tools
> > that need to check for failures can behave more uniformly between
> > different kernels. Like x86_64, it can be disabled by setting
> > debug.show_unhandled_signals sysctl variable to 0 (or by doing
> > echo 0 > /proc/sys/debug/show_unhandled_signals)
>
> Do we really need the ratelimiting?  If the admin turns this on then he's
> presumably prepared for the consequences.
>
> I guess "yes", as people (even distros) are likely to turn this on and
> forget about it.
>
> The patch is larger than I expected, ho hum.
>

So, we happy? What else I can chop from this patch to make it more
acceptable for the people involved?
Please be advised that with this patch, the old exception_trace that
was enabled becomes disabled by default; x86_64 had that enabled, and
i386 didn't have anything...
cheers,
Masoud

Re: i386-show-unhandled-signals-v3

[Andrew Morton]

On Wed, 25 Jul 2007 16:40:06 -0700 masouds@google.com (Masoud Asgharifard Sharbiani) wrote:

> This patch makes the i386 behave the same way that x86_64 does when a
> segfault happens. A line gets printed to the kernel log so that tools
> that need to check for failures can behave more uniformly between
> different kernels. Like x86_64, it can be disabled by setting
> debug.show_unhandled_signals sysctl variable to 0 (or by doing
> echo 0 > /proc/sys/debug/show_unhandled_signals)

Is that still correct?  Methinks /proc/sys/debug/exception-trace.

<Looks sadly at Documentation/filesystems/proc.txt>

<Argh, your patch was reversed. Applied with patch -R.>

> Also, all of the lines being printed are now using printk_ratelimit()
> to deny the ability of DoS from a local user with a program like the
> following:
> main()
> {
>        while (1)
>                if (!fork()) *(int *)0 = 0;
> }

yup.

Re: i386-show-unhandled-signals-v3

[Rene Herman]

On 07/26/2007 01:25 AM, Andrew Morton wrote:

> On Wed, 25 Jul 2007 14:07:56 -0700
> "Masoud Sharbiani" <masouds@google.com> wrote:

>> This is rate limited; Do you need me to rewrite it with it being
>> disabled by default?
> 
> Yes please.
> 
> Look: if there's a way in which an unprivileged user can trigger a printk
> we fix it, end of story. I don't know why this even slightly controversial.

rene@7ixe4:/tmp$ su -c "dmesg -c >/dev/null"
rene@7ixe4:/tmp$ cdparanoia -B

[ ... ]

rene@7ixe4:/tmp$ dmesg | wc -l
158
rene@7ixe4:/tmp$ dmesg | tail -20
sg_write: data in/out 30576/30576 bytes for SCSI command 0xbe--guessing data in;
    program cdparanoia not setting count and/or reply_len properly
printk: 252 messages suppressed.
sg_write: data in/out 16464/16464 bytes for SCSI command 0xbe--guessing data in;
    program cdparanoia not setting count and/or reply_len properly
printk: 245 messages suppressed.
sg_write: data in/out 30576/30576 bytes for SCSI command 0xbe--guessing data in;
    program cdparanoia not setting count and/or reply_len properly
printk: 243 messages suppressed.
sg_write: data in/out 30576/30576 bytes for SCSI command 0xbe--guessing data in;
    program cdparanoia not setting count and/or reply_len properly
printk: 242 messages suppressed.
sg_write: data in/out 30576/30576 bytes for SCSI command 0xbe--guessing data in;
    program cdparanoia not setting count and/or reply_len properly
printk: 255 messages suppressed.
sg_write: data in/out 30576/30576 bytes for SCSI command 0xbe--guessing data in;
    program cdparanoia not setting count and/or reply_len properly
printk: 242 messages suppressed.
sg_write: data in/out 30576/30576 bytes for SCSI command 0xbe--guessing data in;
    program cdparanoia not setting count and/or reply_len properly

cdparanoia does require access to the /dev/sg? that corresponds to /dev/cdrom 
but at least udev (here) makes that node be a (root,cdrom) b-rw-rw--- device 
(and requiring root privileges to rip CDs would certainly not be nice).

Rene.

Re: i386-show-unhandled-signals-v3

[Andi Kleen]

> Look: if there's a way in which an unprivileged user can trigger a printk
> we fix it, end of story. 

I'm firmly against disabling it on x86-64 by default. The printks are extremly
useful and have found many bugs in the past.

-Andi

Re: i386-show-unhandled-signals-v3

[Andrew Morton]

On Thu, 26 Jul 2007 11:46:23 +0200 Andi Kleen <ak@suse.de> wrote:

> 
> > Look: if there's a way in which an unprivileged user can trigger a printk
> > we fix it, end of story. 
> 
> I'm firmly against disabling it on x86-64 by default.

We know you are, and the consensus and past practice disagree with you, as
you well know.

> The printks are extremly
> useful and have found many bugs in the past.

So you turn it on if your applications are playing up.  bfd.


Still waiting for your report of all the other means by which unpriviliged
users can spam the logs, btw.  Of course, your attitude here makes a
mockery of all our other care and effort in this area.

Re: i386-show-unhandled-signals-v3

[Andi Kleen]

On Thursday 26 July 2007 12:14:06 Andrew Morton wrote:
> On Thu, 26 Jul 2007 11:46:23 +0200 Andi Kleen <ak@suse.de> wrote:
> 
> > 
> > > Look: if there's a way in which an unprivileged user can trigger a printk
> > > we fix it, end of story. 
> > 
> > I'm firmly against disabling it on x86-64 by default.
> 
> We know you are, and the consensus and past practice disagree with you, as
> you well know.

Well that doesn't mean that the practice makes much sense.

Security (which is probably too strong a word for these relatively
weak DoS anyways; it's not like anybody's data gets leaked like
Alan hinted at with his bogus analogy) is only useful if it's perfect; if it 
has holes that cannot be plugged it is just wasted effort and likely 
harming other good causes (like bug free software) 
 
> > The printks are extremly
> > useful and have found many bugs in the past.
> 
> So you turn it on if your applications are playing up.  bfd.

You might not know applications are segfaulting. e.g. when I originally
enabled it we found that a few obscure cases in a default system
were occasionally segfaulting, but nobody noticed because there
wasn't a really visible malfunction. Still fixing those made
a better system.
 
> Still waiting for your report of all the other means by which unpriviliged
> users can spam the logs, btw.  Of course, your attitude here makes a
> mockery of all our other care and effort in this area.

One standard way is to overflow the socket limits or the TCP memory allocation 
for example. Or just run the system out of memory; that will get plenty of logs
(don't say ulimit now, you know as well as me that they are not good enough
to prevent oom from users). Or use an unaligned a.out executable.

That was just from a quick look, I'm sure there are more.

Some of these are rate limited, but rate limiting just means it will take longer
to fill the disk. 

There are also a couple (rate limited ones) that can be triggered from the network.

Anyways if you feel strongly about it then rate limit the x86-64 ones
(Masoud's patch did this anyways); but please don't turn them off.

-Andi

Re: i386-show-unhandled-signals-v3

[Alan Cox]

> > So you turn it on if your applications are playing up.  bfd.
> 
> You might not know applications are segfaulting. e.g. when I originally
> enabled it we found that a few obscure cases in a default system
> were occasionally segfaulting, but nobody noticed because there
> wasn't a really visible malfunction. Still fixing those made
> a better system.

There problem is that if you get something going crazy segfaulting your
logging itself can make the problem far worse. The rate limiting is as
much important to stop accidents as stop malicious attack.

> for example. Or just run the system out of memory; that will get plenty of logs
> (don't say ulimit now, you know as well as me that they are not good enough
> to prevent oom from users). Or use an unaligned a.out executable.

Zero overcommit keeps it pretty sane - most times. 

> Some of these are rate limited, but rate limiting just means it will take longer
> to fill the disk. 

Which is good news and gives you a lot longer (or your tools a lot
longer) to react and sort it out. It also avoids the disk thrashing and
performance hit you can cause.

> Anyways if you feel strongly about it then rate limit the x86-64 ones
> (Masoud's patch did this anyways); but please don't turn them off.

Its configurable easily enough so I'm not sure it matters which way it is
set, but it does need to be a -safe- default - either rate limited or off.

Re: i386-show-unhandled-signals-v3

[Alan Cox]

On Thu, 26 Jul 2007 11:46:23 +0200
Andi Kleen <ak@suse.de> wrote:

> 
> > Look: if there's a way in which an unprivileged user can trigger a printk
> > we fix it, end of story. 
> 
> I'm firmly against disabling it on x86-64 by default. The printks are extremly
> useful and have found many bugs in the past.

Then add the rate limiting by default.

Lots of things are convenient. Being able to read arbitary kernel memory
from user space is frequently convenient for debugging but that doesn't
mean its a good idea or should be allowed

Re: i386-show-unhandled-signals-v3

[Rene Herman]

On 07/26/2007 12:16 PM, Alan Cox wrote:

> On Thu, 26 Jul 2007 11:46:23 +0200
> Andi Kleen <ak@suse.de> wrote:
> 
>>> Look: if there's a way in which an unprivileged user can trigger a printk
>>> we fix it, end of story. 
>> I'm firmly against disabling it on x86-64 by default. The printks are extremly
>> useful and have found many bugs in the past.
> 
> Then add the rate limiting by default.

The messages were rate limited -- Andrew said they couldn't be on default even 
rate limited.

Rene.

Re: i386-show-unhandled-signals-v3

[Andrew Morton]

On Thu, 26 Jul 2007 12:17:28 +0200 Rene Herman <rene.herman@gmail.com> wrote:

> On 07/26/2007 12:16 PM, Alan Cox wrote:
> 
> > On Thu, 26 Jul 2007 11:46:23 +0200
> > Andi Kleen <ak@suse.de> wrote:
> > 
> >>> Look: if there's a way in which an unprivileged user can trigger a printk
> >>> we fix it, end of story. 
> >> I'm firmly against disabling it on x86-64 by default. The printks are extremly
> >> useful and have found many bugs in the past.
> > 
> > Then add the rate limiting by default.
> 
> The messages were rate limited -- Andrew said they couldn't be on default even 
> rate limited.
> 

Andrew didn't think too hard, sorry.  I agree that on-by-default is a reasonable
compromise once they're rate-limited.