[RFC,PATCH 2/2] posix timers: don't discard the signal if the timer was explicitly destroyed

[RFC,PATCH 2/2] posix timers: don't discard the signal if the timer was explicitly destroyed

[Oleg Nesterov]

I am not sure this patch is really needed, please review.

The previous patch adds the user-visible change. It is not clear to me why
should we cancel the pending signal sent by the timer after timer_delete().
Suppose the signal is blocked, pending, the user checks sys_rt_sigpending(),
destroys the timer and then doesn't see the signal.

Add the "boolean force" parameter to sigqueue_free(). Unless it is true, we
don't cancel the pending signal but just clear SIGQUEUE_PREALLOC, so that
"struct sigqueue" will be freed when dequeued, like the "regular" sigqueue.

So, with this patch exit/exec discard the signal, but timer_delete() doesn't.

Signed-off-by: Oleg Nesterov <oleg@tv-sign.ru>

--- 25/include/linux/sched.h~2_PT_ONLY_ON_EXEC	2008-03-08 17:35:43.000000000 +0300
+++ 25/include/linux/sched.h	2008-03-08 17:35:43.000000000 +0300
@@ -1709,7 +1709,7 @@ extern int send_sig(int, struct task_str
 extern void zap_other_threads(struct task_struct *p);
 extern int kill_proc(pid_t, int, int);
 extern struct sigqueue *sigqueue_alloc(void);
-extern void sigqueue_free(struct sigqueue *);
+extern void sigqueue_free(struct sigqueue *q, int force);
 extern int send_sigqueue(struct sigqueue *,  struct task_struct *, int group);
 extern int do_sigaction(int, struct k_sigaction *, struct k_sigaction *);
 extern int do_sigaltstack(const stack_t __user *, stack_t __user *, unsigned long);
--- 25/kernel/signal.c~2_PT_ONLY_ON_EXEC	2008-04-22 16:40:45.000000000 +0400
+++ 25/kernel/signal.c	2008-04-22 18:42:34.000000000 +0400
@@ -1255,7 +1255,7 @@ static void sigqueue_cancel(struct sigqu
 	recalc_sigpending();
 }
 
-void sigqueue_free(struct sigqueue *q)
+void sigqueue_free(struct sigqueue *q, int force)
 {
 	unsigned long flags;
 	spinlock_t *lock = &current->sighand->siglock;
@@ -1267,12 +1267,17 @@ void sigqueue_free(struct sigqueue *q)
 	 * q->list to serialize with collect_signal().
 	 */
 	spin_lock_irqsave(lock, flags);
-	if (!list_empty(&q->list))
-		sigqueue_cancel(q);
+	q->flags &= ~SIGQUEUE_PREALLOC;
+	if (!list_empty(&q->list)) {
+		if (force)
+			sigqueue_cancel(q);
+		else
+			q = NULL;
+	}
 	spin_unlock_irqrestore(lock, flags);
 
-	q->flags &= ~SIGQUEUE_PREALLOC;
-	__sigqueue_free(q);
+	if (q)
+		__sigqueue_free(q);
 }
 
 int send_sigqueue(struct sigqueue *q, struct task_struct *t, int group)
--- 25/kernel/posix-timers.c~2_PT_ONLY_ON_EXEC	2008-03-08 17:41:18.000000000 +0300
+++ 25/kernel/posix-timers.c	2008-04-22 18:35:50.000000000 +0400
@@ -441,7 +441,7 @@ static struct k_itimer * alloc_posix_tim
 
 #define IT_ID_SET	1
 #define IT_ID_NOT_SET	0
-static void release_posix_timer(struct k_itimer *tmr, int it_id_set)
+static void release_posix_timer(struct k_itimer *tmr, int it_id_set, int force)
 {
 	if (it_id_set) {
 		unsigned long flags;
@@ -449,7 +449,7 @@ static void release_posix_timer(struct k
 		idr_remove(&posix_timers_id, tmr->it_id);
 		spin_unlock_irqrestore(&idr_lock, flags);
 	}
-	sigqueue_free(tmr->sigq);
+	sigqueue_free(tmr->sigq, force);
 	if (unlikely(tmr->it_process) &&
 	    tmr->it_sigev_notify == (SIGEV_SIGNAL|SIGEV_THREAD_ID))
 		put_task_struct(tmr->it_process);
@@ -580,7 +580,7 @@ sys_timer_create(const clockid_t which_c
 
 out:
 	if (error)
-		release_posix_timer(new_timer, it_id_set);
+		release_posix_timer(new_timer, it_id_set, 0);
 
 	return error;
 }
@@ -861,7 +861,7 @@ retry_delete:
 		timer->it_process = NULL;
 	}
 	unlock_timer(timer, flags);
-	release_posix_timer(timer, IT_ID_SET);
+	release_posix_timer(timer, IT_ID_SET, 0);
 	return 0;
 }
 
@@ -890,7 +890,7 @@ retry_delete:
 		timer->it_process = NULL;
 	}
 	unlock_timer(timer, flags);
-	release_posix_timer(timer, IT_ID_SET);
+	release_posix_timer(timer, IT_ID_SET, 1);
 }
 
 /*

Re: [RFC,PATCH 2/2] posix timers: don't discard the signal if the timer was explicitly destroyed

[Oleg Nesterov]

On 04/22, Roland McGrath wrote:
>
> > I am not sure this patch is really needed, please review.
> 
> I don't think this is worth doing.
> 
> > The previous patch adds the user-visible change. It is not clear to me why
> > should we cancel the pending signal sent by the timer after timer_delete().
> > Suppose the signal is blocked, pending, the user checks sys_rt_sigpending(),
> > destroys the timer and then doesn't see the signal.
> 
> So?  POSIX says it's unspecified what happens to such a signal,

Ah, good!

> so an
> application can't rely on it one way or the other.  I don't see any reason
> to complicate it further.

Agreed, please ignore this patch.

Thanks! I really hoped you will nack this, I hate the additional complications
too.

Oleg.

Re: [RFC,PATCH 2/2] posix timers: don't discard the signal if the timer was explicitly destroyed

[Roland McGrath]

> I am not sure this patch is really needed, please review.

I don't think this is worth doing.

> The previous patch adds the user-visible change. It is not clear to me why
> should we cancel the pending signal sent by the timer after timer_delete().
> Suppose the signal is blocked, pending, the user checks sys_rt_sigpending(),
> destroys the timer and then doesn't see the signal.

So?  POSIX says it's unspecified what happens to such a signal, so an
application can't rely on it one way or the other.  I don't see any reason
to complicate it further.


Thanks,
Roland