linux-kernel - Re: [PATCH] show message when exceeded rlimit of pending signals

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4AEBEE38.50108@miraclelinux.com>
Date:	Sat, 31 Oct 2009 16:58:48 +0900
From:	Naohiro Ooiwa <nooiwa@...aclelinux.com>
To:	Andrew Morton <akpm@...ux-foundation.org>
CC:	Ingo Molnar <mingo@...e.hu>,
	Hiroshi Shimamoto <h-shimamoto@...jp.nec.com>,
	roland@...hat.com, Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Thomas Gleixner <tglx@...utronix.de>,
	LKML <linux-kernel@...r.kernel.org>, oleg@...hat.com
Subject: Re: [PATCH] show message when exceeded rlimit of pending signals

Andrew Morton wrote:
> On Fri, 30 Oct 2009 20:36:31 +0900
> Naohiro Ooiwa <nooiwa@...aclelinux.com> wrote:
> 
>> Hi Ingo,
>>
>> I wrote proper changelog entry.
>> And I resent the patch. I added KERN_INFO to printk.
>>
>>
>>
>> When the system has too many timers or too many aggregate
>> queued signals, the EAGAIN error is returned to application
>> from kernel, including timer_create().
>> It means that exceeded limit of pending signals at all.
>> But we can't imagine it.
>>
>> This patch show the message when reached limit of pending signals.
>> If you see this message and your system behaved unexpectedly,
>> you can run following command.
>>    # ulimit -i unlimited
>>
>> With help from Hiroshi Shimamoto <h-shimamoto@...jp.nec.com>.
>>
>>
>> ...
>>
>> diff --git a/kernel/signal.c b/kernel/signal.c
>> index 6705320..50e10dc 100644
>> --- a/kernel/signal.c
>> +++ b/kernel/signal.c
>> @@ -41,6 +41,8 @@
>>
>>  static struct kmem_cache *sigqueue_cachep;
>>
>> +int print_fatal_signals __read_mostly;
>> +
>>  static void __user *sig_handler(struct task_struct *t, int sig)
>>  {
>>  	return t->sighand->action[sig - 1].sa.sa_handler;
>> @@ -188,6 +190,14 @@ int next_signal(struct sigpending *pending, sigset_t *mask)
>>  	return sig;
>>  }
>>
>> +static void show_reach_rlimit_sigpending(void)
>> +{
>> +	if (!printk_ratelimit())
>> +		return;
> 
> printk_ratelimit() is a bad thing and we should be working toward
> removing it altogether, not adding new callers.
> 
> Because it uses global state.  So if subsystem A is trying to generate
> lots of printk's, subsystem B's important message might get
> accidentally suppressed.
> 
> It's better to use DEFINE_RATELIMIT_STATE() and __ratelimit() directly.


Thank you for your advices.
And I was glad to talk to you in Japan Linux Symposium.

I got it, now that you mention it.
I will fix my patch.

> 
>> +	printk(KERN_INFO "%s/%d: reached the limit of pending signals.\n",
>> +				current->comm, current->pid);
> 
> I suggest that this be
> 
> 	"reached RLIMIT_SIGPENDING"
> 
> because RLIMIT_SIGPENDING is a well-understood term and concept.
> 

OK, I see.

>>  static void print_fatal_signal(struct pt_regs *regs, int signr)
>>  {
>> -	printk("%s/%d: potentially unexpected fatal signal %d.\n",
>> +	printk(KERN_INFO "%s/%d: potentially unexpected fatal signal %d.\n",
>>  		current->comm, task_pid_nr(current), signr);
>>
> 
> This is an unchangelogged, unrelated, non-backward-compatible
> user-visible change.  For some people, their machine which used to
> print this warning will mysteriously stop doing so when they upgrade
> their kernels.
> 
> That doesn't mean that we shouldn't make the change.  But we should
> have a think about it and we shouldn't hide changes of this nature
> inside some other patch like this.
> 

You are right.
I'm sorry, I shouldn't habe done it.


Thanks you.
Naohiro Ooiwa
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/