[PATCH] fix sigqueue accounting for posix-timers broken by new RLIMIT_SIGPENDING tracking code

[PATCH] fix sigqueue accounting for posix-timers broken by new RLIMIT_SIGPENDING tracking code

[Roland McGrath]

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 <roland@redhat.com>

--- 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)

Re: [PATCH] fix sigqueue accounting for posix-timers broken by new RLIMIT_SIGPENDING tracking code

[Chris Wright]

* Roland McGrath (roland@redhat.com) 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

Re: [PATCH] fix sigqueue accounting for posix-timers broken by new RLIMIT_SIGPENDING tracking code

[Chris Wright]

* Chris Wright (chrisw@osdl.org) wrote:
> * Roland McGrath (roland@redhat.com) 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

Re: [PATCH] fix sigqueue accounting for posix-timers broken by new RLIMIT_SIGPENDING tracking code

[Roland McGrath]

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

Re: [PATCH] fix sigqueue accounting for posix-timers broken by new RLIMIT_SIGPENDING tracking code

[Chris Wright]

* Roland McGrath (roland@redhat.com) 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