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: <alpine.LFD.1.10.0805211245131.3081@woody.linux-foundation.org>
Date:	Wed, 21 May 2008 13:07:50 -0700 (PDT)
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Roland McGrath <roland@...hat.com>
cc:	Oleg Nesterov <oleg@...sign.ru>,
	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 Wed, 21 May 2008, Roland McGrath wrote:
> 
> Removing the entry without fixing the pending set is the bug we're trying
> to fix.  That's what it does now, and it's wrong.

Well, I think we could/should look at the bigger picture.

The whole (and only!) reason for this problem in the first place is that 
the generic signal-handling code is simply not designed for signals going 
away. Because they simply don't. This whole sys_timer_delete() thing is 
purely based on that problem.

So we have a few options here, I think.

One is the approach that Oleg has taken, which is to try to remove the 
signals. Quite frankly, I don't much like it, because it's against all the 
normal signal handler behaviour. And being against all the normal signal 
handler behaviour, it violates the assumptions we have about signals being 
sticky, which is why it then has problems with both the task sigpending 
bit and the per-signal pending bits.

We can continue with that approach, but judging by the issues, I suspect 
it basically involves having to do back-pointers from the signal info to 
the queues they are on, and even then we'd always have the issues with 
code that simply assumes that signals are sticky. The simple fact is, we 
have lots of system call code that does

	if (signal_pending())
		return -ERESTARTNOHAND;

etc, so even if we don't actually *take* the signal, a cancelled signal 
would still result in a spurious restart or worse - an EINTR.

But there are other approaches. For example, afaik, this really is more 
about execve() than about anything else. How about we just do a special 
case in "flush_signal_handlers()" - rather than try to make it an issue of 
releasing the POSIX timer.

We know those posix timer things are special. We know they go away at 
execve() time. There's this unlucky race condition that only happens at 
execve time, and only because we must flush the pending signal handlers in 
a way that we normally *don't* flush any other signals.

So we could easily make the POSIX timer code just mark the signals it 
sends, and then at execve() time (in "flush_signal_handlers()") we walk 
the signal queues of that thread and get rid of those signal entries.

And at *that* point it is trivial to clear re-calculate the pending bit 
entirely for that thread, because we know there is nothing else going on.

But doing it in general, when there may be multiple active threads, and 
one of them does an "timer_delete()" system call, that's actually hard. 
The other threads may be doing other things, and may validly have that 
timer pending.

Hmm?

I bet there are other approaches to this. But I think "sigqueue_free()" is 
fundamentally hard to fix to do what we want to do, without making signals 
do something that they normally don't have to do.

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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ