[PATCH] Always send siginfo for synchronous signals

[PATCH] Always send siginfo for synchronous signals

[Jeremy Fitzhardinge]

[-- Attachment #1: Type: text/plain, Size: 1017 bytes --]

Valgrind is critically dependent on getting siginfo with its synchronous
(caused by an instruction fault) signals; if it gets, say, a SIGSEGV
which doesn't have siginfo, it must terminate ASAP because it really
can't make any more progress without knowing what caused the SIGSEGV.

The trouble is that if some other completely unrelated program the user
is running at the time builds up a large queue of pending signals for
some reason (as KDE seems to on SuSE 9.2), it will cause Valgrind to
fail for that user, apparently inexplicably.

It seems to me that the kernel should always deliver siginfo with
synchronous fault signals (SIGSEGV, SIGBUS, SIGFPE, SIGTRAP, SIGILL).
They can't ever be blocked (because they're unconditionally fatal if you
do block them), and therefore never queued.  By definition, the task
which causes the signal is running at the time, and so can be
immediately delivered a siginfo without having to allocate one (except
temporarily).

Proposed patch against 2.6.11-rc4 attached.

    J


[-- Attachment #2: always-send-siginfo-for-faults.patch --]
[-- Type: text/x-patch, Size: 3448 bytes --]

If we're sending a signal relating to a faulting instruction, then
always generate siginfo for that signal.  

If the user has some unrelated process which has managed to consume
the user's entire allocation of siginfo, then signals will start being
delivered without siginfo.  Some programs absolutely depend on getting
siginfo for signals like SIGSEGV, and get very confused if they see a
SEGV without siginfo.

Such signals cannot be blocked (they're immediately fatal if they
are), and therefore cannot be queued.  There's therefore no risk of
resource starvation.

Signed-off-by: Jeremy Fitzhardinge <jeremy@goop.org>

Index: local-2.6/kernel/signal.c
===================================================================
--- local-2.6.orig/kernel/signal.c	2005-02-22 20:35:30.000000000 -0800
+++ local-2.6/kernel/signal.c	2005-02-22 20:43:16.000000000 -0800
@@ -136,6 +136,10 @@ static kmem_cache_t *sigqueue_cachep;
 #define SIG_KERNEL_IGNORE_MASK (\
         M(SIGCONT)   |  M(SIGCHLD)   |  M(SIGWINCH)  |  M(SIGURG)    )
 
+#define SIG_KERNEL_SYNC_MASK (\
+	M(SIGSEGV)   |  M(SIGBUS)    | M(SIGILL)     |  M(SIGFPE)    | \
+	M(SIGTRAP) )
+
 #define sig_kernel_only(sig) \
 		(((sig) < SIGRTMIN)  && T(sig, SIG_KERNEL_ONLY_MASK))
 #define sig_kernel_coredump(sig) \
@@ -144,6 +148,8 @@ static kmem_cache_t *sigqueue_cachep;
 		(((sig) < SIGRTMIN)  && T(sig, SIG_KERNEL_IGNORE_MASK))
 #define sig_kernel_stop(sig) \
 		(((sig) < SIGRTMIN)  && T(sig, SIG_KERNEL_STOP_MASK))
+#define sig_kernel_sync(sig) \
+		(((sig) < SIGRTMIN)  && T(sig, SIG_KERNEL_SYNC_MASK))
 
 #define sig_user_defined(t, signr) \
 	(((t)->sighand->action[(signr)-1].sa.sa_handler != SIG_DFL) &&	\
@@ -260,11 +266,12 @@ next_signal(struct sigpending *pending, 
 	return sig;
 }
 
-static struct sigqueue *__sigqueue_alloc(struct task_struct *t, int flags)
+static struct sigqueue *__sigqueue_alloc(struct task_struct *t, int flags, int always)
 {
 	struct sigqueue *q = NULL;
 
-	if (atomic_read(&t->user->sigpending) <
+	if (always || 
+	    atomic_read(&t->user->sigpending) <
 			t->signal->rlim[RLIMIT_SIGPENDING].rlim_cur)
 		q = kmem_cache_alloc(sigqueue_cachep, flags);
 	if (q) {
@@ -777,6 +784,7 @@ static int send_signal(int sig, struct s
 {
 	struct sigqueue * q = NULL;
 	int ret = 0;
+	int always;
 
 	/*
 	 * fast-pathed signals for kernel-internal things like SIGSTOP
@@ -785,6 +793,13 @@ static int send_signal(int sig, struct s
 	if ((unsigned long)info == 2)
 		goto out_set;
 
+	/* Always attempt to send siginfo with an unblocked
+	   fault-generated signal. */
+	always = sig_kernel_sync(sig) &&
+		!sigismember(&t->blocked, sig) &&
+		(unsigned long)info > 2 &&
+		info->si_code > SI_USER;
+
 	/* Real-time signals must be queued if sent by sigqueue, or
 	   some other real-time mechanism.  It is implementation
 	   defined whether kill() does so.  We attempt to do so, on
@@ -793,7 +808,7 @@ static int send_signal(int sig, struct s
 	   make sure at least one signal gets delivered and don't
 	   pass on the info struct.  */
 
-	q = __sigqueue_alloc(t, GFP_ATOMIC);
+	q = __sigqueue_alloc(t, GFP_ATOMIC, always);
 	if (q) {
 		list_add_tail(&q->list, &signals->list);
 		switch ((unsigned long) info) {
@@ -1316,7 +1331,7 @@ struct sigqueue *sigqueue_alloc(void)
 {
 	struct sigqueue *q;
 
-	if ((q = __sigqueue_alloc(current, GFP_KERNEL)))
+	if ((q = __sigqueue_alloc(current, GFP_KERNEL, 0)))
 		q->flags |= SIGQUEUE_PREALLOC;
 	return(q);
 }

Re: [PATCH] Always send siginfo for synchronous signals

[Chris Wright]

* Jeremy Fitzhardinge (jeremy@goop.org) wrote:
> Valgrind is critically dependent on getting siginfo with its synchronous
> (caused by an instruction fault) signals; if it gets, say, a SIGSEGV
> which doesn't have siginfo, it must terminate ASAP because it really
> can't make any more progress without knowing what caused the SIGSEGV.
> 
> The trouble is that if some other completely unrelated program the user
> is running at the time builds up a large queue of pending signals for
> some reason (as KDE seems to on SuSE 9.2), it will cause Valgrind to
> fail for that user, apparently inexplicably.

It's not quite inexplicable.  It means that task has hit its limit for
pending signals ;-)  But I agree, this should be fixed.  I think I had
tested this with broken test cases, thanks for catching.

> --- local-2.6.orig/kernel/signal.c	2005-02-22 20:35:30.000000000 -0800
> +++ local-2.6/kernel/signal.c	2005-02-22 20:43:16.000000000 -0800
> @@ -136,6 +136,10 @@ static kmem_cache_t *sigqueue_cachep;
>  #define SIG_KERNEL_IGNORE_MASK (\
>          M(SIGCONT)   |  M(SIGCHLD)   |  M(SIGWINCH)  |  M(SIGURG)    )
>  
> +#define SIG_KERNEL_SYNC_MASK (\
> +	M(SIGSEGV)   |  M(SIGBUS)    | M(SIGILL)     |  M(SIGFPE)    | \
> +	M(SIGTRAP) )
> +
>  #define sig_kernel_only(sig) \
>  		(((sig) < SIGRTMIN)  && T(sig, SIG_KERNEL_ONLY_MASK))
>  #define sig_kernel_coredump(sig) \
> @@ -144,6 +148,8 @@ static kmem_cache_t *sigqueue_cachep;
>  		(((sig) < SIGRTMIN)  && T(sig, SIG_KERNEL_IGNORE_MASK))
>  #define sig_kernel_stop(sig) \
>  		(((sig) < SIGRTMIN)  && T(sig, SIG_KERNEL_STOP_MASK))
> +#define sig_kernel_sync(sig) \
> +		(((sig) < SIGRTMIN)  && T(sig, SIG_KERNEL_SYNC_MASK))
>  
>  #define sig_user_defined(t, signr) \
>  	(((t)->sighand->action[(signr)-1].sa.sa_handler != SIG_DFL) &&	\
> @@ -260,11 +266,12 @@ next_signal(struct sigpending *pending, 
>  	return sig;
>  }
>  
> -static struct sigqueue *__sigqueue_alloc(struct task_struct *t, int flags)
> +static struct sigqueue *__sigqueue_alloc(struct task_struct *t, int flags, int always)

maybe force_info instead of always?

>  {
>  	struct sigqueue *q = NULL;
>  
> -	if (atomic_read(&t->user->sigpending) <
> +	if (always || 
> +	    atomic_read(&t->user->sigpending) <
>  			t->signal->rlim[RLIMIT_SIGPENDING].rlim_cur)
>  		q = kmem_cache_alloc(sigqueue_cachep, flags);
>  	if (q) {
> @@ -777,6 +784,7 @@ static int send_signal(int sig, struct s
>  {
>  	struct sigqueue * q = NULL;
>  	int ret = 0;
> +	int always;

Could we call it force_info?

>  	/*
>  	 * fast-pathed signals for kernel-internal things like SIGSTOP
> @@ -785,6 +793,13 @@ static int send_signal(int sig, struct s
>  	if ((unsigned long)info == 2)
>  		goto out_set;
>  
> +	/* Always attempt to send siginfo with an unblocked
> +	   fault-generated signal. */
> +	always = sig_kernel_sync(sig) &&
> +		!sigismember(&t->blocked, sig) &&

Aren't these already unblocked?

> +		(unsigned long)info > 2 &&
> +		info->si_code > SI_USER;

In what case is != SI_KERNEL OK?

thanks,
-chris
-- 
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net

Re: [PATCH] Always send siginfo for synchronous signals

[Jeremy Fitzhardinge]

Chris Wright wrote:

>It's not quite inexplicable.  It means that task has hit its limit for
>pending signals ;-)  But I agree, this should be fixed.  I think I had
>tested this with broken test cases, thanks for catching.
>  
>
It's particularly confusing for users, because it's a per-user limit
rather than a per-process one, and its not at all apparent which program
is causing the problem (/proc/N/status will tell you that a process has
a signal pending, but it won't tell you how many are pending).

In fact, bugs with these symptoms have been reported against Valgrind
from time to time for years, and its only recently I worked out what's
going on (mostly because I introduced a bug which caused Valgrind to do
it to itself).

>>+static struct sigqueue *__sigqueue_alloc(struct task_struct *t, int flags, int always)
>>    
>>
>maybe force_info instead of always?
>  
>
I suppose, but it doesn't "force" it really.  The allocation could still
fail (it is GFP_ATOMIC after all), and you'd still get no siginfo.  I
don't care much either way.

>> 	/*
>> 	 * fast-pathed signals for kernel-internal things like SIGSTOP
>>@@ -785,6 +793,13 @@ static int send_signal(int sig, struct s
>> 	if ((unsigned long)info == 2)
>> 		goto out_set;
>> 
>>+	/* Always attempt to send siginfo with an unblocked
>>+	   fault-generated signal. */
>>+	always = sig_kernel_sync(sig) &&
>>+		!sigismember(&t->blocked, sig) &&
>>    
>>
>Aren't these already unblocked?
>  
>
I can't think of a case where they wouldn't be, but I wanted to make
sure this couldn't be used to create a new DoS.

>>+		(unsigned long)info > 2 &&
>>+		info->si_code > SI_USER;
>>    
>>
>In what case is != SI_KERNEL OK?
>  
>
Fault signals rarely have an si_code of SI_KERNEL (0x80); they generally
have a small integer to describe what the fault was really about
(SEGV_MAPERR, etc).  All si_codes > SI_USER (0) are defined to have come
from the kernel.  Hm, I see there's a macro, SI_FROMKERNEL, for doing
this test.

Updated patch attached.

    J

Re: [PATCH] Always send siginfo for synchronous signals

[Chris Wright]

* Jeremy Fitzhardinge (jeremy@goop.org) wrote:
> Chris Wright wrote:
> 
> >It's not quite inexplicable.  It means that task has hit its limit for
> >pending signals ;-)  But I agree, this should be fixed.  I think I had
> >tested this with broken test cases, thanks for catching.
> >
> It's particularly confusing for users, because it's a per-user limit
> rather than a per-process one, and its not at all apparent which program
> is causing the problem (/proc/N/status will tell you that a process has
> a signal pending, but it won't tell you how many are pending).

Suggestion for good place to display that info?

> In fact, bugs with these symptoms have been reported against Valgrind
> from time to time for years, and its only recently I worked out what's
> going on (mostly because I introduced a bug which caused Valgrind to do
> it to itself).

This code is pretty new (since 2.6.8-rc1, last June), so I expect some
other issue in the years past.

> >>+static struct sigqueue *__sigqueue_alloc(struct task_struct *t, int flags, int always)
> >>    
> >>
> >maybe force_info instead of always?
> >  
> >
> I suppose, but it doesn't "force" it really.  The allocation could still
> fail (it is GFP_ATOMIC after all), and you'd still get no siginfo.  I
> don't care much either way.
> 
> >> 	/*
> >> 	 * fast-pathed signals for kernel-internal things like SIGSTOP
> >>@@ -785,6 +793,13 @@ static int send_signal(int sig, struct s
> >> 	if ((unsigned long)info == 2)
> >> 		goto out_set;
> >> 
> >>+	/* Always attempt to send siginfo with an unblocked
> >>+	   fault-generated signal. */
> >>+	always = sig_kernel_sync(sig) &&
> >>+		!sigismember(&t->blocked, sig) &&
> >>    
> >>
> >Aren't these already unblocked?
> >  
> >
> I can't think of a case where they wouldn't be, but I wanted to make
> sure this couldn't be used to create a new DoS.

I suppose it could be blocked and the source could be send_sig_info()
instead of force_sig_info.  It seems quite reasonable to let sync kernel
generated signals through, but I'm inclined to keep hole small to avoid
DoS vector.  So I agree, leave it in for now.

> >>+		(unsigned long)info > 2 &&
> >>+		info->si_code > SI_USER;
> >>    
> >>
> >In what case is != SI_KERNEL OK?
> >  
> >
> Fault signals rarely have an si_code of SI_KERNEL (0x80); they generally
> have a small integer to describe what the fault was really about
> (SEGV_MAPERR, etc).  All si_codes > SI_USER (0) are defined to have come
> from the kernel.  Hm, I see there's a macro, SI_FROMKERNEL, for doing
> this test.

Yeah you're right, was just crafting an email recanting that bit ;-)
Here's what I had tested here:

	force_info = (sig_kernel_sync(sig) && !sigismember(&t->blocked, sig)
		      && (unsigned long)info > 2 && SI_FROMKERNEL(info));


> Updcted patch attached.

Oops, missed the patch.  Anyway, here's my tweak to your patch.  I
preserved your Signed-off-by.  Thanks again for report plus patch.

thanks,
-chris
--

If we're sending a signal relating to a faulting instruction, then
always generate siginfo for that signal.

If the user has some unrelated process which has managed to consume
the user's entire allocation of siginfo, then signals will start being
delivered without siginfo.  Some programs absolutely depend on getting
siginfo for signals like SIGSEGV, and get very confused if they see a
SEGV without siginfo.

Such signals cannot be blocked (they're immediately fatal if they
are), and therefore cannot be queued.  There's therefore no risk of
resource starvation.

Signed-off-by: Jeremy Fitzhardinge <jeremy@goop.org>
Signed-off-by: Chris Wright <chrisw@osdl.org>

===== kernel/signal.c 1.153 vs edited =====
--- 1.153/kernel/signal.c	2005-01-30 22:33:46 -08:00
+++ edited/kernel/signal.c	2005-02-23 15:39:37 -08:00
@@ -136,6 +136,10 @@ static kmem_cache_t *sigqueue_cachep;
 #define SIG_KERNEL_IGNORE_MASK (\
         M(SIGCONT)   |  M(SIGCHLD)   |  M(SIGWINCH)  |  M(SIGURG)    )
 
+#define SIG_KERNEL_SYNC_MASK (\
+	M(SIGSEGV)   |  M(SIGBUS)    | M(SIGILL)     |  M(SIGFPE)    | \
+	M(SIGTRAP) )
+
 #define sig_kernel_only(sig) \
 		(((sig) < SIGRTMIN)  && T(sig, SIG_KERNEL_ONLY_MASK))
 #define sig_kernel_coredump(sig) \
@@ -144,6 +148,8 @@ static kmem_cache_t *sigqueue_cachep;
 		(((sig) < SIGRTMIN)  && T(sig, SIG_KERNEL_IGNORE_MASK))
 #define sig_kernel_stop(sig) \
 		(((sig) < SIGRTMIN)  && T(sig, SIG_KERNEL_STOP_MASK))
+#define sig_kernel_sync(sig) \
+		(((sig) < SIGRTMIN)  && T(sig, SIG_KERNEL_SYNC_MASK))
 
 #define sig_user_defined(t, signr) \
 	(((t)->sighand->action[(signr)-1].sa.sa_handler != SIG_DFL) &&	\
@@ -260,12 +266,12 @@ next_signal(struct sigpending *pending, 
 	return sig;
 }
 
-static struct sigqueue *__sigqueue_alloc(struct task_struct *t, int flags)
+static struct sigqueue *__sigqueue_alloc(struct task_struct *t, int flags, int force_info)
 {
 	struct sigqueue *q = NULL;
 
-	if (atomic_read(&t->user->sigpending) <
-			t->signal->rlim[RLIMIT_SIGPENDING].rlim_cur)
+	if (force_info || atomic_read(&t->user->sigpending) <
+			  t->signal->rlim[RLIMIT_SIGPENDING].rlim_cur)
 		q = kmem_cache_alloc(sigqueue_cachep, flags);
 	if (q) {
 		INIT_LIST_HEAD(&q->list);
@@ -776,7 +782,7 @@ static int send_signal(int sig, struct s
 			struct sigpending *signals)
 {
 	struct sigqueue * q = NULL;
-	int ret = 0;
+	int force_info, ret = 0;
 
 	/*
 	 * fast-pathed signals for kernel-internal things like SIGSTOP
@@ -785,6 +791,11 @@ static int send_signal(int sig, struct s
 	if ((unsigned long)info == 2)
 		goto out_set;
 
+	/* Always attempt to send siginfo with an unblocked
+	   fault-generated signal. */
+	force_info = (sig_kernel_sync(sig) && !sigismember(&t->blocked, sig)
+		      && (unsigned long)info > 2 && SI_FROMKERNEL(info));
+
 	/* Real-time signals must be queued if sent by sigqueue, or
 	   some other real-time mechanism.  It is implementation
 	   defined whether kill() does so.  We attempt to do so, on
@@ -793,7 +804,7 @@ static int send_signal(int sig, struct s
 	   make sure at least one signal gets delivered and don't
 	   pass on the info struct.  */
 
-	q = __sigqueue_alloc(t, GFP_ATOMIC);
+	q = __sigqueue_alloc(t, GFP_ATOMIC, force_info);
 	if (q) {
 		list_add_tail(&q->list, &signals->list);
 		switch ((unsigned long) info) {
@@ -1316,7 +1327,7 @@ struct sigqueue *sigqueue_alloc(void)
 {
 	struct sigqueue *q;
 
-	if ((q = __sigqueue_alloc(current, GFP_KERNEL)))
+	if ((q = __sigqueue_alloc(current, GFP_KERNEL, 0)))
 		q->flags |= SIGQUEUE_PREALLOC;
 	return(q);
 }

Re: [PATCH] Always send siginfo for synchronous signals

[Jeremy Fitzhardinge]

Chris Wright wrote:

>>/proc/N/status will tell you that a process has
>>a signal pending, but it won't tell you how many are pending).
>>    
>>
>
>Suggestion for good place to display that info?
>  
>
I guess another line in /proc/N/status:

    SigQue: 0 0 0 0 0 0 0 123 0 0 1238 0 0 0 0 0 ... 0 0 1

or something, but I haven't really thought about it.

>>In fact, bugs with these symptoms have been reported against Valgrind
>>from time to time for years, and its only recently I worked out what's
>>going on (mostly because I introduced a bug which caused Valgrind to do
>>it to itself).
>>    
>>
>This code is pretty new (since 2.6.8-rc1, last June), so I expect some
>other issue in the years past.
>  
>
There was always a limit on the number of pending queue siginfo
signals.  It used to be system-wide rather than per-user though.

    J

[PATCH] show RLIMIT_SIGPENDING usage in /proc/PID/status

[Roland McGrath]

Jeremy mentioned the aggravation of not being able to tell when your
processes are using up signal queue entries and hitting the
RLIMIT_SIGPENDING limit.  This patch adds a line to /proc/PID/status
showing how many queue items are in use, and allowed, for your uid.

I can certainly see the appeal of having a display of the number of queued
items specific to each process, and even the items within the process
broken down per signal number.  However, those are not things that are
directly counted, and ascertaining them requires iterating through the
queue.  This patch instead gives what can be readily determined in constant
time using the accounting already done.  I'm not sure something more
complex is warranted just to facilitate one particular debugging need.
With this, you can see quickly that this particular problem has come up.
Then examination of each process's SigPnd/ShdPnd lines ought to give you an
indication of which processes have any queued RT signals sitting around for
a long time, and you can then attack those programs directly, though there
is no way after the fact to determine how many queued signals with the same
number a given process has (short of killing it and seeing the usage drop).

Note you may still have a mystery if the leaking programs are not leaving
pending RT signals queued, but rather preallocating queue items via
timer_create.  That usage is not readily apparent in any /proc information.


Signed-off-by: Roland McGrath <roland@redhat.com>

--- linux-2.6/fs/proc/array.c
+++ linux-2.6/fs/proc/array.c
@@ -239,6 +239,7 @@ static inline char * task_sig(struct tas
 {
 	sigset_t pending, shpending, blocked, ignored, caught;
 	int num_threads = 0;
+	unsigned long qsize = 0, qlim = 0;
 
 	sigemptyset(&pending);
 	sigemptyset(&shpending);
@@ -255,11 +256,14 @@ static inline char * task_sig(struct tas
 		blocked = p->blocked;
 		collect_sigign_sigcatch(p, &ignored, &caught);
 		num_threads = atomic_read(&p->signal->count);
+		qsize = atomic_read(&p->user->sigpending);
+		qlim = p->signal->rlim[RLIMIT_SIGPENDING].rlim_cur;
 		spin_unlock_irq(&p->sighand->siglock);
 	}
 	read_unlock(&tasklist_lock);
 
 	buffer += sprintf(buffer, "Threads:\t%d\n", num_threads);
+	buffer += sprintf(buffer, "SigQ:\t%lu/%lu\n", qsize, qlim);
 
 	/* render them all */
 	buffer = render_sigset_t("SigPnd:\t", &pending, buffer);

Re: [PATCH] show RLIMIT_SIGPENDING usage in /proc/PID/status

[Chris Wright]

* Roland McGrath (roland@redhat.com) wrote:
> Jeremy mentioned the aggravation of not being able to tell when your
> processes are using up signal queue entries and hitting the
> RLIMIT_SIGPENDING limit.  This patch adds a line to /proc/PID/status
> showing how many queue items are in use, and allowed, for your uid.

Two questions:  1) This changes the interface for consumers of
/proc/[pid]/status data, do we care?  Adding new line like this should
be safe enough. 2) Perhaps we should do /proc/[pid]/rlimit/ type dir
for each value?  This has been asked for before.

thanks,
-chris
-- 
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net

Re: [PATCH] show RLIMIT_SIGPENDING usage in /proc/PID/status

[Roland McGrath]

> Two questions: 1) This changes the interface for consumers of
> /proc/[pid]/status data, do we care?  Adding new line like this should be
> safe enough.

As far as I can tell, noone fretted about the addition of Threads:,
ShdPnd:, etc., which were not always there.

> 2) Perhaps we should do /proc/[pid]/rlimit/ type dir for each value?
>    This has been asked for before.

Is the request to see the limit settings, or the current usage, or both?
What kind of format are you suggesting?  I don't see a need for something
with a million little files.  Also, for some of the limits the correct
current usage count is not trivial to ascertain.  (And for others like
RLIMIT_FSIZE and RLIMIT_CORE, it is of course not meaningful at all.)


Thanks,
Roland

Re: [PATCH] show RLIMIT_SIGPENDING usage in /proc/PID/status

[Chris Wright]

* Roland McGrath (roland@redhat.com) wrote:
> > Two questions: 1) This changes the interface for consumers of
> > /proc/[pid]/status data, do we care?  Adding new line like this should be
> > safe enough.
> 
> As far as I can tell, noone fretted about the addition of Threads:,
> ShdPnd:, etc., which were not always there.

Sounds good ;-)

> > 2) Perhaps we should do /proc/[pid]/rlimit/ type dir for each value?
> >    This has been asked for before.
> 
> Is the request to see the limit settings, or the current usage, or both?
> What kind of format are you suggesting?  I don't see a need for something
> with a million little files.  Also, for some of the limits the correct
> current usage count is not trivial to ascertain.  (And for others like
> RLIMIT_FSIZE and RLIMIT_CORE, it is of course not meaningful at all.)

Probably just one file per rlimit with usage, cur, max.

thanks,
-chris
-- 
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net

[PATCH] set RLIMIT_SIGPENDING limit based on RLIMIT_NPROC

[Roland McGrath]

While looking into the issues Jeremy had with the RLIMIT_SIGPENDING limit,
it occurred to me that the normal setting of this limit is bizarrely low.
The initial hard limit setting (MAX_SIGPENDING) was taken from the old
max_queued_signals parameter, which was for the entire system in aggregate.
But even as a per-user limit, the 1024 value is incongruously low for this.
On my machine, RLIMIT_NPROC allows me 8192 processes, but only 1024 queued
signals, i.e. fewer even than one pending signal in each process.  (To me,
this really puts in doubt the sensibility of using a per-user limit for
this rather than a per-process one, i.e. counted in sighand_struct or
signal_struct, which could have a much smaller reasonable value.  I don't
recall the rationale for making this new limit per-user in the first place.)

This patch sets the default RLIMIT_SIGPENDING limit at boot time, using the
calculation that decides the default RLIMIT_NPROC limit.  This uses the
same value for those two limits, which I think is still pretty conservative
on the RLIMIT_SIGPENDING value.


Thanks,
Roland


Signed-off-by: Roland McGrath <roland@redhat.com>

--- linux-2.6/include/asm-generic/resource.h
+++ linux-2.6/include/asm-generic/resource.h
@@ -51,7 +51,7 @@
 	[RLIMIT_MEMLOCK]	= {   MLOCK_LIMIT,   MLOCK_LIMIT },	\
 	[RLIMIT_AS]		= { RLIM_INFINITY, RLIM_INFINITY },	\
 	[RLIMIT_LOCKS]		= { RLIM_INFINITY, RLIM_INFINITY },	\
-	[RLIMIT_SIGPENDING]	= { MAX_SIGPENDING, MAX_SIGPENDING },	\
+	[RLIMIT_SIGPENDING]	= { 		0,	       0 },	\
 	[RLIMIT_MSGQUEUE]	= { MQ_BYTES_MAX, MQ_BYTES_MAX },	\
 }
 
--- linux-2.6/include/linux/signal.h
+++ linux-2.6/include/linux/signal.h
@@ -8,8 +8,6 @@
 
 #ifdef __KERNEL__
 
-#define MAX_SIGPENDING	1024
-
 /*
  * Real Time signals may be queued.
  */
--- linux-2.6/kernel/fork.c
+++ linux-2.6/kernel/fork.c
@@ -129,6 +129,8 @@ void __init fork_init(unsigned long memp
 
 	init_task.signal->rlim[RLIMIT_NPROC].rlim_cur = max_threads/2;
 	init_task.signal->rlim[RLIMIT_NPROC].rlim_max = max_threads/2;
+	init_task.signal->rlim[RLIMIT_SIGPENDING] =
+		init_task.signal->rlim[RLIMIT_NPROC];
 }
 
 static struct task_struct *dup_task_struct(struct task_struct *orig)

Re: [PATCH] set RLIMIT_SIGPENDING limit based on RLIMIT_NPROC

[Chris Wright]

* Roland McGrath (roland@redhat.com) wrote:
> While looking into the issues Jeremy had with the RLIMIT_SIGPENDING limit,
> it occurred to me that the normal setting of this limit is bizarrely low.
> The initial hard limit setting (MAX_SIGPENDING) was taken from the old
> max_queued_signals parameter, which was for the entire system in aggregate.
> But even as a per-user limit, the 1024 value is incongruously low for this.

But the old default system-wide limit was 1024.  And you could have
spawned 8k processes then as well.  So I don't think this matters much.

> On my machine, RLIMIT_NPROC allows me 8192 processes, but only 1024 queued
> signals, i.e. fewer even than one pending signal in each process.  (To me,
> this really puts in doubt the sensibility of using a per-user limit for
> this rather than a per-process one, i.e. counted in sighand_struct or
> signal_struct, which could have a much smaller reasonable value.  I don't
> recall the rationale for making this new limit per-user in the first place.)

I don't either, the archives show using per-user as default choice
(never saw a discussion otherwise).  Users can easily queue signals to
themselves (using multiple processes or not), and there was some concern
that somebody actually wanted to be able queue up to 1024 (since it's
what was allowed in the past).

> This patch sets the default RLIMIT_SIGPENDING limit at boot time, using the
> calculation that decides the default RLIMIT_NPROC limit.  This uses the
> same value for those two limits, which I think is still pretty conservative
> on the RLIMIT_SIGPENDING value.

It's an rlimit, so easily setable in userspace at login session time.  I
think we could raise it if people start complaining it's too low (hasn't
seemed to be a problem yet).

thanks,
-chris
-- 
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net

Re: [PATCH] set RLIMIT_SIGPENDING limit based on RLIMIT_NPROC

[Jeremy Fitzhardinge]

Chris Wright wrote:

>It's an rlimit, so easily setable in userspace at login session time.  I
>think we could raise it if people start complaining it's too low (hasn't
>seemed to be a problem yet).
>
Know any shells which support setting it?  Indeed, glibc doesn't seem to
know about it.

    J

Re: [PATCH] set RLIMIT_SIGPENDING limit based on RLIMIT_NPROC

[Chris Wright]

* Jeremy Fitzhardinge (jeremy@goop.org) wrote:
> Chris Wright wrote:
> 
> >It's an rlimit, so easily setable in userspace at login session time.  I
> >think we could raise it if people start complaining it's too low (hasn't
> >seemed to be a problem yet).
> >
> Know any shells which support setting it?  Indeed, glibc doesn't seem to
> know about it.

Hrm, my bash and glibc are both aware, you might need an update?

thanks,
-chris
-- 
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net

Re: [PATCH] Always send siginfo for synchronous signals

[Jeremy Fitzhardinge]

[-- Attachment #1: Type: text/plain, Size: 32 bytes --]

Erm, this updated patch.

    J

[-- Attachment #2: always-send-siginfo-for-faults.patch --]
[-- Type: text/x-patch, Size: 3444 bytes --]

If we're sending a signal relating to a faulting instruction, then
always generate siginfo for that signal.  

If the user has some unrelated process which has managed to consume
the user's entire allocation of siginfo, then signals will start being
delivered without siginfo.  Some programs absolutely depend on getting
siginfo for signals like SIGSEGV, and get very confused if they see a
SEGV without siginfo.

Such signals cannot be blocked (they're immediately fatal if they
are), and therefore cannot be queued.  There's therefore no risk of
resource starvation.

Signed-off-by: Jeremy Fitzhardinge <jeremy@goop.org>

Index: local-2.6/kernel/signal.c
===================================================================
--- local-2.6.orig/kernel/signal.c	2005-02-22 20:35:30.000000000 -0800
+++ local-2.6/kernel/signal.c	2005-02-23 14:57:23.000000000 -0800
@@ -136,6 +136,10 @@ static kmem_cache_t *sigqueue_cachep;
 #define SIG_KERNEL_IGNORE_MASK (\
         M(SIGCONT)   |  M(SIGCHLD)   |  M(SIGWINCH)  |  M(SIGURG)    )
 
+#define SIG_KERNEL_SYNC_MASK (\
+	M(SIGSEGV)   |  M(SIGBUS)    | M(SIGILL)     |  M(SIGFPE)    | \
+	M(SIGTRAP) )
+
 #define sig_kernel_only(sig) \
 		(((sig) < SIGRTMIN)  && T(sig, SIG_KERNEL_ONLY_MASK))
 #define sig_kernel_coredump(sig) \
@@ -144,6 +148,8 @@ static kmem_cache_t *sigqueue_cachep;
 		(((sig) < SIGRTMIN)  && T(sig, SIG_KERNEL_IGNORE_MASK))
 #define sig_kernel_stop(sig) \
 		(((sig) < SIGRTMIN)  && T(sig, SIG_KERNEL_STOP_MASK))
+#define sig_kernel_sync(sig) \
+		(((sig) < SIGRTMIN)  && T(sig, SIG_KERNEL_SYNC_MASK))
 
 #define sig_user_defined(t, signr) \
 	(((t)->sighand->action[(signr)-1].sa.sa_handler != SIG_DFL) &&	\
@@ -260,11 +266,12 @@ next_signal(struct sigpending *pending, 
 	return sig;
 }
 
-static struct sigqueue *__sigqueue_alloc(struct task_struct *t, int flags)
+static struct sigqueue *__sigqueue_alloc(struct task_struct *t, int flags, int always)
 {
 	struct sigqueue *q = NULL;
 
-	if (atomic_read(&t->user->sigpending) <
+	if (always || 
+	    atomic_read(&t->user->sigpending) <
 			t->signal->rlim[RLIMIT_SIGPENDING].rlim_cur)
 		q = kmem_cache_alloc(sigqueue_cachep, flags);
 	if (q) {
@@ -777,6 +784,7 @@ static int send_signal(int sig, struct s
 {
 	struct sigqueue * q = NULL;
 	int ret = 0;
+	int always;
 
 	/*
 	 * fast-pathed signals for kernel-internal things like SIGSTOP
@@ -785,6 +793,13 @@ static int send_signal(int sig, struct s
 	if ((unsigned long)info == 2)
 		goto out_set;
 
+	/* Always attempt to send siginfo with an unblocked
+	   fault-generated signal. */
+	always = sig_kernel_sync(sig) &&
+		!sigismember(&t->blocked, sig) &&
+		(unsigned long)info > 2 &&
+		SI_FROMKERNEL(info);
+
 	/* Real-time signals must be queued if sent by sigqueue, or
 	   some other real-time mechanism.  It is implementation
 	   defined whether kill() does so.  We attempt to do so, on
@@ -793,7 +808,7 @@ static int send_signal(int sig, struct s
 	   make sure at least one signal gets delivered and don't
 	   pass on the info struct.  */
 
-	q = __sigqueue_alloc(t, GFP_ATOMIC);
+	q = __sigqueue_alloc(t, GFP_ATOMIC, always);
 	if (q) {
 		list_add_tail(&q->list, &signals->list);
 		switch ((unsigned long) info) {
@@ -1316,7 +1331,7 @@ struct sigqueue *sigqueue_alloc(void)
 {
 	struct sigqueue *q;
 
-	if ((q = __sigqueue_alloc(current, GFP_KERNEL)))
+	if ((q = __sigqueue_alloc(current, GFP_KERNEL, 0)))
 		q->flags |= SIGQUEUE_PREALLOC;
 	return(q);
 }

[PATCH] override RLIMIT_SIGPENDING for non-RT signals

[Roland McGrath]

Indeed, I think your patch does not go far enough.  I can read POSIX to say
that the siginfo_t data must be available when `kill' was used, as well.
This patch makes it allocate the siginfo_t, even when that exceeds
{RLIMIT_SIGPENDING}, for any non-RT signal (< SIGRTMIN) not sent by
sigqueue (actually, any signal that couldn't have been faked by a sigqueue
call).  Of course, in an extreme memory shortage situation, you are SOL and
violate POSIX a little before you die horribly from being out of memory anyway.

The LEGACY_QUEUE logic already ensures that, for non-RT signals, at most
one is ever on the queue.  So there really is no risk at all of unbounded
resource consumption; the usage can reach {RLIMIT_SIGPENDING} + 31, is all.

It's already the case that the limit can be exceeded by (in theory) up to
{RLIMIT_NPROC}-1 in race conditions because the bump and the limit check
are not atomic.  (Obviously you can only get anywhere near that many with
assloads of preemption, but exceeding it by a few is not too unlikely.)
This patch also fixes that accounting so that it should not be possible to
exceed {RLIMIT_SIGPENDING} + SIGRTMIN-1 queue items per user in races.


Thanks,
Roland


Signed-off-by: Roland McGrath <roland@redhat.com>

--- linux-2.6/kernel/signal.c
+++ linux-2.6/kernel/signal.c
@@ -260,19 +260,23 @@ next_signal(struct sigpending *pending, 
 	return sig;
 }
 
-static struct sigqueue *__sigqueue_alloc(struct task_struct *t, int flags)
+static struct sigqueue *__sigqueue_alloc(struct task_struct *t, int flags,
+					 int override_rlimit)
 {
 	struct sigqueue *q = NULL;
 
-	if (atomic_read(&t->user->sigpending) <
+	atomic_inc(&t->user->sigpending);
+	if (override_rlimit ||
+	    atomic_read(&t->user->sigpending) <=
 			t->signal->rlim[RLIMIT_SIGPENDING].rlim_cur)
 		q = kmem_cache_alloc(sigqueue_cachep, flags);
-	if (q) {
+	if (unlikely(q == NULL)) {
+		atomic_dec(&t->user->sigpending);
+	} else {
 		INIT_LIST_HEAD(&q->list);
 		q->flags = 0;
 		q->lock = NULL;
 		q->user = get_uid(t->user);
-		atomic_inc(&q->user->sigpending);
 	}
 	return(q);
 }
@@ -793,7 +797,9 @@ static int send_signal(int sig, struct s
 	   make sure at least one signal gets delivered and don't
 	   pass on the info struct.  */
 
-	q = __sigqueue_alloc(t, GFP_ATOMIC);
+	q = __sigqueue_alloc(t, GFP_ATOMIC, (sig < SIGRTMIN &&
+					     ((unsigned long) info < 2 ||
+					      info->si_code >= 0)));
 	if (q) {
 		list_add_tail(&q->list, &signals->list);
 		switch ((unsigned long) info) {
@@ -1316,7 +1322,7 @@ struct sigqueue *sigqueue_alloc(void)
 {
 	struct sigqueue *q;
 
-	if ((q = __sigqueue_alloc(current, GFP_KERNEL)))
+	if ((q = __sigqueue_alloc(current, GFP_KERNEL, 0)))
 		q->flags |= SIGQUEUE_PREALLOC;
 	return(q);
 }

Re: [PATCH] override RLIMIT_SIGPENDING for non-RT signals

[Chris Wright]

* Roland McGrath (roland@redhat.com) wrote:
> Indeed, I think your patch does not go far enough.  I can read POSIX to say
> that the siginfo_t data must be available when `kill' was used, as well.

How?  I only see reference to filling in SI_USER for rt signals?
Just curious...(I've only got SuSv3 and some crusty old POSIX rt docs).

> This patch makes it allocate the siginfo_t, even when that exceeds
> {RLIMIT_SIGPENDING}, for any non-RT signal (< SIGRTMIN) not sent by
> sigqueue (actually, any signal that couldn't have been faked by a sigqueue
> call).  Of course, in an extreme memory shortage situation, you are SOL and
> violate POSIX a little before you die horribly from being out of memory anyway.

> The LEGACY_QUEUE logic already ensures that, for non-RT signals, at most
> one is ever on the queue.  So there really is no risk at all of unbounded
> resource consumption; the usage can reach {RLIMIT_SIGPENDING} + 31, is all.

Good point.  Although it's RLIMIT_SIGPENDING + (31 * user_nprocs).  So
that could be 31 * 8k, for example.

thanks,
-chris
-- 
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net

Re: [PATCH] override RLIMIT_SIGPENDING for non-RT signals

[Roland McGrath]

> * Roland McGrath (roland@redhat.com) wrote:
> > Indeed, I think your patch does not go far enough.  I can read POSIX to say
> > that the siginfo_t data must be available when `kill' was used, as well.
> 
> How?  I only see reference to filling in SI_USER for rt signals?
> Just curious...(I've only got SuSv3 and some crusty old POSIX rt docs).

There is stuff about a SA_SIGINFO signal handler's siginfo_t argument
"shall contain" the various specified information like si_pid/si_uid values
for a kill caller.

> Good point.  Although it's RLIMIT_SIGPENDING + (31 * user_nprocs).  So
> that could be 31 * 8k, for example.

And a "good point" back to you, sir!  I think the right way to think about
this in terms of resource consumption is that sizeof(struct sigqueue)*31 is
part of the potential per-process overhead that make up the consumption
units one should have in mind when choosing how to set the RLIMIT_NPROC limit.


Thanks,
Roland

Re: [PATCH] override RLIMIT_SIGPENDING for non-RT signals

[Chris Wright]

* Roland McGrath (roland@redhat.com) wrote:
> > * Roland McGrath (roland@redhat.com) wrote:
> > > Indeed, I think your patch does not go far enough.  I can read POSIX to say
> > > that the siginfo_t data must be available when `kill' was used, as well.
> > 
> > How?  I only see reference to filling in SI_USER for rt signals?
> > Just curious...(I've only got SuSv3 and some crusty old POSIX rt docs).
> 
> There is stuff about a SA_SIGINFO signal handler's siginfo_t argument
> "shall contain" the various specified information like si_pid/si_uid values
> for a kill caller.

OK, guess it's odd corner case, since they aren't queued anyway.

> > Good point.  Although it's RLIMIT_SIGPENDING + (31 * user_nprocs).  So
> > that could be 31 * 8k, for example.
> 
> And a "good point" back to you, sir!  I think the right way to think about
> this in terms of resource consumption is that sizeof(struct sigqueue)*31 is
> part of the potential per-process overhead that make up the consumption
> units one should have in mind when choosing how to set the RLIMIT_NPROC limit.

As in dynamic, and work with the patch that you sent to redo default
sigpending as per nproc?

thanks,
-chris
-- 
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net

Re: [PATCH] override RLIMIT_SIGPENDING for non-RT signals

[Jeremy Fitzhardinge]

Roland McGrath wrote:

>Indeed, I think your patch does not go far enough.  I can read POSIX to say
>that the siginfo_t data must be available when `kill' was used, as well.
>This patch makes it allocate the siginfo_t, even when that exceeds
>{RLIMIT_SIGPENDING}, for any non-RT signal (< SIGRTMIN) not sent by
>sigqueue (actually, any signal that couldn't have been faked by a sigqueue
>call).
>
Looks OK to me.  I'll give this a try soon.

    J

Re: [PATCH] override RLIMIT_SIGPENDING for non-RT signals

[Chris Wright]

* Jeremy Fitzhardinge (jeremy@goop.org) wrote:
> Roland McGrath wrote:
> 
> >Indeed, I think your patch does not go far enough.  I can read POSIX to say
> >that the siginfo_t data must be available when `kill' was used, as well.
> >This patch makes it allocate the siginfo_t, even when that exceeds
> >{RLIMIT_SIGPENDING}, for any non-RT signal (< SIGRTMIN) not sent by
> >sigqueue (actually, any signal that couldn't have been faked by a sigqueue
> >call).
> >
> Looks OK to me.  I'll give this a try soon.

Yeah, it fixes the issue, but opens the door to larger consumption of
pending signals.  Roland, what was your final preference?  I'm kind of
leaning towards Jeremy's original patch.

thanks,
-chris
-- 
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net

Re: [PATCH] override RLIMIT_SIGPENDING for non-RT signals

[Roland McGrath]

> Yeah, it fixes the issue, but opens the door to larger consumption of
> pending signals.  Roland, what was your final preference?  I'm kind of
> leaning towards Jeremy's original patch.

It's not a matter of preference.  As I said in the first place, without my
patch we are violating POSIX, and delivering unreliable results to users.


Thanks,
Roland

Re: [PATCH] override RLIMIT_SIGPENDING for non-RT signals

[Chris Wright]

* Roland McGrath (roland@redhat.com) wrote:
> > Yeah, it fixes the issue, but opens the door to larger consumption of
> > pending signals.  Roland, what was your final preference?  I'm kind of
> > leaning towards Jeremy's original patch.
> 
> It's not a matter of preference.  As I said in the first place, without my
> patch we are violating POSIX, and delivering unreliable results to users.

Right, and as you also mentioned, it's identical case to exhausting
atomic pool, in either case we're out of resources, and in both cases the
machine may recover and be functional.  And sneaking around the rlimit
can cost ~4k per-process, which is why I'd consider the edge case a
reasonable loss.  (heck, maybe 4k is fine considering task size, and
mlock limits, etc).

thanks,
-chris
-- 
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net