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

On Tue, Sep 19, 2023 at 03:39:21PM +0300, Dmitry Chagin wrote:
> 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?
Ah, I see.  Thank you for the explanation.

What was the commit where bde added the check?