linux-kernel - Re: [PATCH 1/3] signals: sigqueue_free: don't free sigqueue if it is queued

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: <20080522115553.GB117@tv-sign.ru>
Date:	Thu, 22 May 2008 15:55:53 +0400
From:	Oleg Nesterov <oleg@...sign.ru>
To:	Roland McGrath <roland@...hat.com>
Cc:	Linus Torvalds <torvalds@...ux-foundation.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Austin Clements <amdragon+kernelbugzilla@....edu>,
	Ingo Molnar <mingo@...e.hu>, john stultz <johnstul@...ibm.com>,
	Michael Kerrisk <mtk.manpages@...glemail.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/3] signals: sigqueue_free: don't free sigqueue if it is queued

On 05/22, Oleg Nesterov wrote:
>
> On 05/21, Roland McGrath wrote:
> >
> > -void flush_sigqueue(struct sigpending *queue)
> > +static void __flush_sigqueue(struct sigpending *queue, int timers)
> >  {
> >  	struct sigqueue *q;
> >  
> >  	sigemptyset(&queue->signal);
> >  	while (!list_empty(&queue->list)) {
> >  		q = list_entry(queue->list.next, struct sigqueue , list);
> > +		if (timers && q->info.si_code != SI_TIMER)
> > +			continue;
> >  		list_del_init(&q->list);
> >  		__sigqueue_free(q);
> >  	}
> >  }
> 
> This is not enough. Again, we remove and free sigqueue but don't discard
> the pending signal. (and we must take into account other rt signals with
> the same si_signo if we want to discard the signal).
> 
> Oh, this problem is unexpectedly nasty. It is trivial and minor, we can
> solve it in may ways, but personally I can't find a simple/clean way.
> 
> Let's look at my first attempt,
> 
> 	http://marc.info/?l=linux-kernel&m=120888210417700
> 
> the patch was "almost" correct.
> We can add the "bool cancel" parameter to sigqueue_free(), true when
> called from exec (or exit_itimers). In that case SIGQUEUE_SHARED_PENDING
> is enough: the pending signal was either sent to current, or it is group
> wide. Not nice too of course, but afaics a bit simpler.

I take my words back. It is not simpler. How about

	void xxx(struct sigpending *pending)
	{
		struct sigqueue *q;
		sigset_t drop, retain;

		sigemptyset(&drop);
		sigemptyset(&retain);

		list_for_each_entry_safe(q) {
			int sig = q->info.si_signo;

			// it is better to add another SIGQUEUE_ flag...
			if (q->info.si_code == SI_TIMER) {
				list_del_init(&q->list);
				__sigqueue_free(q);
				sigaddset(&drop, sig);
			} else
				sigaddset(&retain, sig);
		}

		// pseudo code
		pending->signal &= ~(drop & ~retain);
	}

?

This helper is called somewhere near flush_signal_handlers() or
de_thread()->exit_itimers().

We still need the "sigqueue_free: don't free sigqueue if it is queued"
patch of course.

Oleg.

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