Re: How can kill(-1, 0) return EPERM?
On Mon, Dec 02, 2019 at 02:11:14AM +0300, Dmitry Marakasov wrote: > * Konstantin Belousov (kostik...@gmail.com) 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 > > > > > > #include > > > > > > #include > > > > > > #include > > > > > > #include > > > > > > #include > > > > > > > > > > > > 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(_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(_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. > > > > > > I've meant that according to the manpage > > > > > > If pid is -1: > > > If the user has super-user privileges, the signal is sent to > > > all > > > processes excluding system processes (with P_SYSTEM flag > > > set), > > > process with ID 1 (usually init(8)), and the process sending > > > the > > > signal. If the user is not the super user, the signal is > > > sent to > > > all processes with the same uid as the user excluding the > > > process > > > sending the signal. No error is returned if any process > > > could be > > > signaled. > > > > > > IMO there should be an additional check in this condition: > > > > > > if (p->p_pid <= 1 || p->p_flag & P_SYSTEM || > > > p == td->td_proc || p->p_state == PRS_NEW) { > > > continue; > > > } > > > > > > E.g. something like > > > > > > if (p->p_pid <= 1 || p->p_flag & P_SYSTEM || > > > p == td->td_proc || p->p_state == PRS_NEW || > > > (td->td_ucred->cr_ruid != 0 && > > > p->td_ucred->cr_ruid != td->td_ucred->cr_ruid) { > > > continue; > > > } > > > > > > e.g. it should not even attempt to signal processes with other uids. > > Why ? You are trying to outguess p_cansignal(), which could deny > > action for much more reasons, so you would get EPERM still, e.g. if the > > target is suid. Or, p_cansignal() also might allow to send the signal > > even for mismatched uids, again look at it code. > > Exactly because of that - p_cansignal behaves in it's own way, which > doesn't match the indended/documented kill(2) behavior. You're right > in a sence that plain uid check is not sufficient though. > > > I might guess that your complain is really about a
Re: How can kill(-1, 0) return EPERM?
* Konstantin Belousov (kostik...@gmail.com) 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 > > > > > #include > > > > > #include > > > > > #include > > > > > #include > > > > > #include > > > > > > > > > > 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(_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(_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. > > > > I've meant that according to the manpage > > > > If pid is -1: > > If the user has super-user privileges, the signal is sent to > > all > > processes excluding system processes (with P_SYSTEM flag set), > > process with ID 1 (usually init(8)), and the process sending > > the > > signal. If the user is not the super user, the signal is sent > > to > > all processes with the same uid as the user excluding the > > process > > sending the signal. No error is returned if any process could > > be > > signaled. > > > > IMO there should be an additional check in this condition: > > > > if (p->p_pid <= 1 || p->p_flag & P_SYSTEM || > > p == td->td_proc || p->p_state == PRS_NEW) { > > continue; > > } > > > > E.g. something like > > > > if (p->p_pid <= 1 || p->p_flag & P_SYSTEM || > > p == td->td_proc || p->p_state == PRS_NEW || > > (td->td_ucred->cr_ruid != 0 && > > p->td_ucred->cr_ruid != td->td_ucred->cr_ruid) { > > continue; > > } > > > > e.g. it should not even attempt to signal processes with other uids. > Why ? You are trying to outguess p_cansignal(), which could deny > action for much more reasons, so you would get EPERM still, e.g. if the > target is suid. Or, p_cansignal() also might allow to send the signal > even for mismatched uids, again look at it code. Exactly because of that - p_cansignal behaves in it's own way, which doesn't match the indended/documented kill(2) behavior. You're right in a sence that plain uid check is not sufficient though. > I might guess that your complain is really about a different aspect > of it. If you look at the posix description of the EPERM error from > kill(2) (really kill(3)), it says > [EPERM] The process does not have permission to send the signal to > any receiving process. > In other words, we should not return EPERM if we signalled at least one > of the process. > > Is this
Re: How can kill(-1, 0) return EPERM?
On Sun, Dec 01, 2019 at 03:24:11AM +0300, Dmitry Marakasov wrote: > * Konstantin Belousov (kostik...@gmail.com) 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 > > > > #include > > > > #include > > > > #include > > > > #include > > > > #include > > > > > > > > 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(_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(_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. > > I've meant that according to the manpage > > If pid is -1: > If the user has super-user privileges, the signal is sent to all > processes excluding system processes (with P_SYSTEM flag set), > process with ID 1 (usually init(8)), and the process sending the > signal. If the user is not the super user, the signal is sent to > all processes with the same uid as the user excluding the process > sending the signal. No error is returned if any process could be > signaled. > > IMO there should be an additional check in this condition: > > if (p->p_pid <= 1 || p->p_flag & P_SYSTEM || > p == td->td_proc || p->p_state == PRS_NEW) { > continue; > } > > E.g. something like > > if (p->p_pid <= 1 || p->p_flag & P_SYSTEM || > p == td->td_proc || p->p_state == PRS_NEW || > (td->td_ucred->cr_ruid != 0 && > p->td_ucred->cr_ruid != td->td_ucred->cr_ruid) { > continue; > } > > e.g. it should not even attempt to signal processes with other uids. Why ? You are trying to outguess p_cansignal(), which could deny action for much more reasons, so you would get EPERM still, e.g. if the target is suid. Or, p_cansignal() also might allow to send the signal even for mismatched uids, again look at it code. I might guess that your complain is really about a different aspect of it. If you look at the posix description of the EPERM error from kill(2) (really kill(3)), it says [EPERM] The process does not have permission to send the signal to any receiving process. In other words, we should not return EPERM if we signalled at least one of the process. Is this the problem ? ___ freebsd-stable@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-stable To unsubscribe, send any mail to "freebsd-stable-unsubscr...@freebsd.org"
Re: How can kill(-1, 0) return EPERM?
* Konstantin Belousov (kostik...@gmail.com) 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 > > > #include > > > #include > > > #include > > > #include > > > #include > > > > > > 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(_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(_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. I've meant that according to the manpage If pid is -1: If the user has super-user privileges, the signal is sent to all processes excluding system processes (with P_SYSTEM flag set), process with ID 1 (usually init(8)), and the process sending the signal. If the user is not the super user, the signal is sent to all processes with the same uid as the user excluding the process sending the signal. No error is returned if any process could be signaled. IMO there should be an additional check in this condition: if (p->p_pid <= 1 || p->p_flag & P_SYSTEM || p == td->td_proc || p->p_state == PRS_NEW) { continue; } E.g. something like if (p->p_pid <= 1 || p->p_flag & P_SYSTEM || p == td->td_proc || p->p_state == PRS_NEW || (td->td_ucred->cr_ruid != 0 && p->td_ucred->cr_ruid != td->td_ucred->cr_ruid) { continue; } e.g. it should not even attempt to signal processes with other uids. -- Dmitry Marakasov . 55B5 0596 FF1E 8D84 5F56 9510 D35A 80DD F9D2 F77D amd...@amdmi3.ru ..: https://github.com/AMDmi3 ___ freebsd-stable@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-stable To unsubscribe, send any mail to "freebsd-stable-unsubscr...@freebsd.org"
Re: How can kill(-1, 0) return EPERM?
On Fri, Nov 29, 2019 at 07:45:09PM +0300, Dmitry Marakasov wrote: > * Dmitry Marakasov (amd...@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 > > #include > > #include > > #include > > #include > > #include > > > > 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(_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(_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. ___ freebsd-stable@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-stable To unsubscribe, send any mail to "freebsd-stable-unsubscr...@freebsd.org"
Re: How can kill(-1, 0) return EPERM?
* Dmitry Marakasov (amd...@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 > #include > #include > #include > #include > #include > > 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(_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(_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. -- Dmitry Marakasov . 55B5 0596 FF1E 8D84 5F56 9510 D35A 80DD F9D2 F77D amd...@amdmi3.ru ..: https://github.com/AMDmi3 ___ freebsd-stable@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-stable To unsubscribe, send any mail to "freebsd-stable-unsubscr...@freebsd.org"
How can kill(-1, 0) return EPERM?
Hi! 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 #include #include #include #include #include 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? [1] https://github.com/NixOS/nix/issues/3250 -- Dmitry Marakasov . 55B5 0596 FF1E 8D84 5F56 9510 D35A 80DD F9D2 F77D amd...@amdmi3.ru ..: https://github.com/AMDmi3 ___ freebsd-stable@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-stable To unsubscribe, send any mail to "freebsd-stable-unsubscr...@freebsd.org"