How can kill(-1, 0) return EPERM?

[Konstantin Belousov]

On Fri, Nov 29, 2019 at 07:45:09PM +0300, Dmitry Marakasov wrote:
> * Dmitry Marakasov (amdmi3 at amdmi3.ru) wrote:
> 
> > I'm helping to investigate some userspace issue [1], where kill(-1, SIGKILL)
> > fails with EPERM. I've managed to isolate this case in a small program:
> > 
> > 
> > ```
> > #include <err.h>
> > #include <errno.h>
> > #include <signal.h>
> > #include <stdio.h>
> > #include <string.h>
> > #include <unistd.h>
> > 
> > int main() {
> >     if (setuid(66) == -1)  // uucp, just for the test
> >         err(1, "setuid");
> > 
> >     int res = kill(-1, 0);  // <- fails with EPERM
> >     fprintf(stderr, "kill(-1, 0) result=%d, errno=%s\n", res, strerror(errno));
> > 
> >     return 0;
> > }
> > ```
> > 
> > when run from root on 12.1 kill call fails with EPERM. However I cannot
> > comprehend what it is caused by and how it's even possible: kill(2) manpage
> > says that with pid=-1 kill should only send (and in this case of sig=0,
> > /not/ send) signals to the processes belonging to the current uid, so there
> > should be no permission problems. I've also looked into the kernel code
> > (sys_kill, killpg1), and it matches to what manpage says, I see no way
> > for it to return EPERM: sys_kill() should fall through to the switch, call
> > killpg1() with all=1 and killpg1() if(all) branch may only set `ret` to
> > either 0 or ESRCH. Am I missing something, or is there a problem somewhere?
> 
> It looks like I have misread the `else if' path of this core.
> 
>     if (all) {
>         /*
>          * broadcast
>          */
>         sx_slock(&allproc_lock);
>         FOREACH_PROC_IN_SYSTEM(p) {
>             if (p->p_pid <= 1 || p->p_flag & P_SYSTEM ||
>                 p == td->td_proc || p->p_state == PRS_NEW) {
>                 continue;
>             }
>             PROC_LOCK(p);
>             err = p_cansignal(td, p, sig);
>             if (err == 0) {
>                 if (sig)
>                     pksignal(p, sig, ksi);
>                 ret = err;
>             }
>             else if (ret == ESRCH)
>                 ret = err;
>             PROC_UNLOCK(p);
>         }
>         sx_sunlock(&allproc_lock);
>     } ...
> 
> so it's clear now where EPERM comes from. However it looks like the
> behavior contradicts the manpage - there are no signs of check that
> the signalled process has the same uid as the caller.

I am not sure what you mean by 'signs of check'.  Look at p_cansignal()
and cr_cansignal() implementation.