Re: git: 227d01c1bc7c - main - linux(4): On Linux SIGKILL can not be reset to default

On Tue, Sep 19, 2023 at 01:42:26PM +0300, Konstantin Belousov wrote:
> On Tue, Sep 19, 2023 at 01:05:07PM +0300, Dmitry Chagin wrote:
> > On Tue, Sep 19, 2023 at 12:05:42AM +0300, Konstantin Belousov wrote:
> > > On Mon, Sep 18, 2023 at 02:53:25PM +0000, Dmitry Chagin wrote:
> > > > The branch main has been updated by dchagin:
> > > > 
> > > > URL: https://cgit.FreeBSD.org/src/commit/?id=227d01c1bc7caf2e838ee6eef1e6a3cc81d79d1b
> > > > 
> > > > commit 227d01c1bc7caf2e838ee6eef1e6a3cc81d79d1b
> > > > Author:     Dmitry Chagin <dchagin@FreeBSD.org>
> > > > AuthorDate: 2023-09-18 14:53:01 +0000
> > > > Commit:     Dmitry Chagin <dchagin@FreeBSD.org>
> > > > CommitDate: 2023-09-18 14:53:01 +0000
> > > > 
> > > >     linux(4): On Linux SIGKILL can not be reset to default
> > > >     
> > > >     MFC after:              1 week
> > > > ---
> > > >  sys/compat/linux/linux_signal.c | 5 ++++-
> > > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/sys/compat/linux/linux_signal.c b/sys/compat/linux/linux_signal.c
> > > > index e3e3fc74a6a9..9a84700b3949 100644
> > > > --- a/sys/compat/linux/linux_signal.c
> > > > +++ b/sys/compat/linux/linux_signal.c
> > > > @@ -170,6 +170,7 @@ linux_do_sigaction(struct thread *td, int linux_sig, l_sigaction_t *linux_nsa,
> > > >  
> > > >  	if (!LINUX_SIG_VALID(linux_sig))
> > > >  		return (EINVAL);
> > > > +	sig = linux_to_bsd_signal(linux_sig);
> > > >  
> > > >  	osa = (linux_osa != NULL) ? &oact : NULL;
> > > >  	if (linux_nsa != NULL) {
> > > > @@ -180,9 +181,11 @@ linux_do_sigaction(struct thread *td, int linux_sig, l_sigaction_t *linux_nsa,
> > > >  			linux_ktrsigset(&linux_nsa->lsa_mask,
> > > >  			    sizeof(linux_nsa->lsa_mask));
> > > >  #endif
> > > > +		if ((sig == SIGKILL || sig == SIGSTOP) &&
> > > > +		    nsa->sa_handler == SIG_DFL)
> > > > +			return (EINVAL);
> > > This is excessive: look at kern_sigaction() around line 822.
> > > 
> > 
> > Well, I checkd that before, Bruce added this check a long time ago,
> > however, Bruce's change allows the handler to be set to SIG_DFL.
> > And I haven't found an explicit statement of how it should be in POSIX,
> > so I think it's Linux specific
> > 
> 
> If kern_sigaction() allows to change SIGKILL or SIGSTOP disposition from
> default, we have a serious issue.  The check above simply repeats the
> check from kern_sigaction().

Hmm, why? Native kern_sigaction check:

		if ((sig == SIGKILL || sig == SIGSTOP) &&
		    act->sa_handler != SIG_DFL) {

				^^^^^^^ this allows to set sa_handler to SIG_DFL,
					but Linux forbids this

			mtx_unlock(&ps->ps_mtx);
			PROC_UNLOCK(p);
			return (EINVAL);
		}

And I found in the POSIX:

It is unspecified whether an attempt to set the action for a signal that
cannot be caught or ignored to SIG_DFL is ignored or causes an error to
be returned with errno set to [EINVAL].

     The sigaction() function may fail if:
        [EINVAL]
	          An attempt was made to set the action to SIG_DFL for a
		  signal that cannot be caught or ignored (or both).

Linux returns EINVAL in that case, FreeBSD does not, both are valid?