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: <20080521115414.GA147@tv-sign.ru>
Date:	Wed, 21 May 2008 15:54:14 +0400
From:	Oleg Nesterov <oleg@...sign.ru>
To:	Roland McGrath <roland@...hat.com>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Austin Clements <amdragon+kernelbugzilla@....edu>,
	Ingo Molnar <mingo@...e.hu>, john stultz <johnstul@...ibm.com>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	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/20, Roland McGrath wrote:
>
> > This patch doesn't change the behaviour of sys_timer_delete() and friends,
> > just makes it more correct and allows us to introduce other SIGQUEUE_ flags
> > passed to the receiver.
>
> To clarify, this certainly does change the behavior.
> There are two changes.
>
> Firstly, a pending timer-firing signal currently gets its siginfo_t
> zeroed out synchronously by timer_delete and now will have its info
> preserved.  That change alone is a potential problem for userland, so
> it should not go in without the following changes to prevent userland
> from seeing the signal at all.  (Currently userland may see a spurious
> signal, but its si_code and si_value don't indicate a timer firing.
> With the correct info, userland might try to use a pointer from
> si_value that was freed around the time it called timer_delete.)

Yes, yes, I meant doesn't change the behaviour in a sense that the
signal is still "visible" to the application.

> Second, a pending timer-firing signal currently stops counting towards
> the RLIMIT_SIGPENDING limit immediately upon a timer_delete call and
> now will keep counting toward that limit until it gets dequeued and
> discarded.  This change is not necessarily a problem.  (POSIX does not
> specify how we decide when resources are too short to create a new
> timer or queue a signal.)  But it deserves mention somewhere.
> Applications can just get fixed to always unblock the signal number or
> flush old signals out with sigwait, if they are averse to timer
> signals with intact siginfo_t arriving after timer_delete.

Yes, thanks, I didn't think about this.

> For this reason, I don't think this set of changes should be
> considered for any -stable branch.

Yes sure.


> > 	q->flags |= SIGQUEUE_CANCELLED;
> > 	spin_lock_irqsave(lock, flags);
> > 	q->flags &= ~SIGQUEUE_PREALLOC;
>
> Just make it:
>
> 	spin_lock_irqsave(lock, flags);
> 	q->flags |= SIGQUEUE_CANCELLED;
> 	q->flags &= ~SIGQUEUE_PREALLOC;
>
> and we needn't wax philosophical about the meaning of locking rules.  That
> patch would have my ACK, but I concur with Linus about the undesireability
> of the plain = version.

OK, will do tomorrow, but...

Oh well. I just realized SIGQUEUE_CANCELLED breaks sys_sigpending() ?

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/