Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 23 Aug 2014 19:52:44 +0200
From:      Jilles Tjoelker <jilles@stack.nl>
To:        Konstantin Belousov <kostikbel@gmail.com>
Cc:        svn-src-head@freebsd.org, svn-src-all@freebsd.org, Roger Pau Monn? <royger@FreeBSD.org>, src-committers@freebsd.org, Bryan Drewery <bdrewery@FreeBSD.org>
Subject:   Re: svn commit: r265003 - head/secure/usr.sbin/sshd
Message-ID:  <20140823175244.GA29648@stack.nl>
In-Reply-To: <20140822153139.GN2737@kib.kiev.ua>
References:  <53F4B381.5010205@FreeBSD.org> <20140820151310.GB2737@kib.kiev.ua> <53F4BC9B.3090405@FreeBSD.org> <53F4BEB1.6070000@FreeBSD.org> <53F4C022.5050804@FreeBSD.org> <20140821080541.GE2737@kib.kiev.ua> <53F5D42E.9080908@FreeBSD.org> <20140821123246.GH2737@kib.kiev.ua> <20140822134353.GA21891@stack.nl> <20140822153139.GN2737@kib.kiev.ua>

next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, Aug 22, 2014 at 06:31:39PM +0300, Konstantin Belousov wrote:
> On Fri, Aug 22, 2014 at 03:43:53PM +0200, Jilles Tjoelker wrote:
> > This is good and necessary for SA_SIGINFO (because of the type of the
> > SIG_DFL and SIG_IGN constants, and because POSIX says so in the
> > description of SA_RESETHAND in the sigaction() page). However, there
> > seems no reason to clear the other flags, which have no effect when the
> > disposition is SIG_DFL or SIG_IGN but have historically always been
> > preserved. (Slight exception: if kernel code erroneously loops on
> > ERESTART, it will eat CPU time iff SA_RESTART is set, independent of the
> > signal's disposition.)
> Well, I already committed the patch with several bugs fixed comparing
> with what was mailed, before your feedback arrived.

> Do you consider it is important enough to revert the resetting of other
> flags ?  In particular, your note about the traditional historic
> behaviour makes me wonder.

I consider it important enough. Clearing the other flags is not
POSIX-compliant and might break applications. For example, I can imagine
an application modifying a struct sigaction with sa_handler == SIG_DFL
from a sigaction() call.

> I do not see why SA_SIGINFO is so special that it must be reset,
> while other flags are not.  The absence of the cases where the
> default/ignored disposition is affected by the flags seems rather
> arbitrary.

The difference is that SA_SIGINFO changes the disposition field from
sa_handler to sa_sigaction, and it is not unambiguously clear how
SIG_DFL and SIG_IGN are represented in sa_sigaction. Note that
sa_handler and sa_sigaction may or may not share storage, and
implementations may or may not support (void (*)(int, siginfo_t *, void
*))SIG_DFL.

For example, when I wrote system() using posix_spawn() in
https://lists.freebsd.org/pipermail/freebsd-hackers/2012-July/040065.html
I needed to know whether SIGINT and SIGQUIT were ignored or not. I wrote

]	if ((intact.sa_flags & SA_SIGINFO) != 0 ||
]	    intact.sa_handler != SIG_IGN)
]		(void)sigaddset(&defmask, SIGINT);
]	if ((quitact.sa_flags & SA_SIGINFO) != 0 ||
]	    quitact.sa_handler != SIG_IGN)
]		(void)sigaddset(&defmask, SIGQUIT);

in the assumption that there is always an actual handler if SA_SIGINFO
is set. I did not really like this code and eventually system() was
changed to use vfork() directly instead of posix_spawn(), but I think it
is correct.

Note that this means that it is sometimes necessary to install a handler
function that will never be called. Per POSIX, SA_SIGINFO must be set
for sigwaitinfo() to be guaranteed to return siginfo_t data, and the
only way to do this is to specify a handler function, even though it
will never be called because the signal is masked. (A never-called
handler function also needs to be specified when using sigwait-like
functions with signals that default to ignore.)

The other flags do not affect the representation of the disposition, and
can therefore remain set without problem.

-- 
Jilles Tjoelker



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20140823175244.GA29648>