The introduction of RLIMIT_SIGPENDING and the associated tracking code was
broken for the case of preallocated sigqueue elements, i.e. posix-timers.
It wrongly includes the timer's preallocated sigqueue structs in the count
towards the per-user when allocating them, but (rightly) does not decrement
the count when they are freed.
This patch fixes it by keeping preallocated sigqueue structs out of the
per-user accounting altogether.
Thanks,
Roland
Signed-off-by: Roland McGrath <[email protected]>
--- vanilla-linux-2.6/kernel/signal.c.orig 2004-09-09 22:07:56.492060454 -0700
+++ vanilla-linux-2.6/kernel/signal.c 2004-09-09 21:55:45.604292751 -0700
@@ -264,7 +264,7 @@ next_signal(struct sigpending *pending,
return sig;
}
-static struct sigqueue *__sigqueue_alloc(void)
+static struct sigqueue *__sigqueue_alloc(int flags)
{
struct sigqueue *q = NULL;
@@ -273,10 +273,13 @@ static struct sigqueue *__sigqueue_alloc
q = kmem_cache_alloc(sigqueue_cachep, GFP_ATOMIC);
if (q) {
INIT_LIST_HEAD(&q->list);
- q->flags = 0;
q->lock = NULL;
- q->user = get_uid(current->user);
- atomic_inc(&q->user->sigpending);
+ q->user = NULL;
+ q->flags = flags;
+ if (!(flags & SIGQUEUE_PREALLOC)) {
+ q->user = get_uid(current->user);
+ atomic_inc(&q->user->sigpending);
+ }
}
return(q);
}
@@ -1331,11 +1334,7 @@ kill_proc(pid_t pid, int sig, int priv)
struct sigqueue *sigqueue_alloc(void)
{
- struct sigqueue *q;
-
- if ((q = __sigqueue_alloc()))
- q->flags |= SIGQUEUE_PREALLOC;
- return(q);
+ return __sigqueue_alloc(SIGQUEUE_PREALLOC);
}
void sigqueue_free(struct sigqueue *q)
* Roland McGrath ([email protected]) wrote:
>
> The introduction of RLIMIT_SIGPENDING and the associated tracking code was
> broken for the case of preallocated sigqueue elements, i.e. posix-timers.
> It wrongly includes the timer's preallocated sigqueue structs in the count
> towards the per-user when allocating them, but (rightly) does not decrement
> the count when they are freed.
Are you sure? IOW, are you seeing a leak of the count? The sigqueue
structure has a lifetime the same as the timer, IIRC. So each time the
signal is sent/received there's no accounting, because it's reused.
But timer create/delete does sigqueue_alloc -> __sigqueue_alloc which
does the inc. and sigqueue_free -> __sigqueue_free which does the dec.
I'm fairly sure I had tested this.
thanks,
-chris
--
Linux Security Modules http://lsm.immunix.org http://lsm.bkbits.net
* Chris Wright ([email protected]) wrote:
> * Roland McGrath ([email protected]) wrote:
> >
> > The introduction of RLIMIT_SIGPENDING and the associated tracking code was
> > broken for the case of preallocated sigqueue elements, i.e. posix-timers.
> > It wrongly includes the timer's preallocated sigqueue structs in the count
> > towards the per-user when allocating them, but (rightly) does not decrement
> > the count when they are freed.
>
> Are you sure? IOW, are you seeing a leak of the count? The sigqueue
> structure has a lifetime the same as the timer, IIRC. So each time the
> signal is sent/received there's no accounting, because it's reused.
> But timer create/delete does sigqueue_alloc -> __sigqueue_alloc which
> does the inc. and sigqueue_free -> __sigqueue_free which does the dec.
> I'm fairly sure I had tested this.
Yeah, I re-read the code and re-ran my tests just now to verify. Current
code is correct. In fact, this patch is wrong. It unbalances the inc/dec
such that sigqueue_alloc() won't inc, yet final sigqueue_free (at timer
destruction) will still dec. Please don't apply this patch.
thanks,
-chris
--
Linux Security Modules http://lsm.immunix.org http://lsm.bkbits.net
D'oh. You are right. I misread the code in the middle of the night. I
came across the new issue of hitting the sigpending limit while verifying a
test case for timers leaking. I failed to realize that it's the timer leak
itself that translates into a sigpending count leak.
Indeed, ignore that patch, and wait for the posix-timers leak fix coming soon.
Thanks,
Roland
* Roland McGrath ([email protected]) wrote:
> D'oh. You are right. I misread the code in the middle of the night. I
> came across the new issue of hitting the sigpending limit while verifying a
> test case for timers leaking. I failed to realize that it's the timer leak
> itself that translates into a sigpending count leak.
>
> Indeed, ignore that patch, and wait for the posix-timers leak fix coming soon.
Cool, thanks for the follow up.
thanks,
-chris
--
Linux Security Modules http://lsm.immunix.org http://lsm.bkbits.net