From owner-freebsd-stable@freebsd.org Sun Dec 1 23:21:46 2019 Return-Path: Delivered-To: freebsd-stable@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 49DAC1BC571 for ; Sun, 1 Dec 2019 23:21:46 +0000 (UTC) (envelope-from amdmi3@amdmi3.ru) Received: from ihor-5.amdmi3.ru (ihor-5.amdmi3.ru [185.238.136.130]) by mx1.freebsd.org (Postfix) with ESMTP id 47R4544DtRz4Nf7 for ; Sun, 1 Dec 2019 23:21:44 +0000 (UTC) (envelope-from amdmi3@amdmi3.ru) Received: from amdmi3.ru (localhost [IPv6:::1]) by ihor-5.amdmi3.ru (Postfix) with SMTP id 37BAC13B79; Mon, 2 Dec 2019 02:21:41 +0300 (MSK) Received: by amdmi3.ru (nbSMTP-1.00) for uid 1001 amdmi3@amdmi3.ru; Mon, 2 Dec 2019 02:21:41 +0300 (MSK) Date: Mon, 2 Dec 2019 02:11:14 +0300 From: Dmitry Marakasov To: Konstantin Belousov Cc: freebsd-stable@freebsd.org Subject: Re: How can kill(-1, 0) return EPERM? Message-ID: <20191201231114.GG4071@hades.panopticon> References: <20191129151606.GD4071@hades.panopticon> <20191129164509.GE4071@hades.panopticon> <20191129225834.GY10580@kib.kiev.ua> <20191201002411.GF4071@hades.panopticon> <20191201144813.GD10580@kib.kiev.ua> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20191201144813.GD10580@kib.kiev.ua> User-Agent: Mutt/1.12.1 (2019-06-15) X-Rspamd-Queue-Id: 47R4544DtRz4Nf7 X-Spamd-Bar: +++ Authentication-Results: mx1.freebsd.org; dkim=none; dmarc=none; spf=none (mx1.freebsd.org: domain of amdmi3@amdmi3.ru has no SPF policy when checking 185.238.136.130) smtp.mailfrom=amdmi3@amdmi3.ru X-Spamd-Result: default: False [3.83 / 15.00]; ARC_NA(0.00)[]; FROM_HAS_DN(0.00)[]; TO_DN_SOME(0.00)[]; MIME_GOOD(-0.10)[text/plain]; DMARC_NA(0.00)[amdmi3.ru]; AUTH_NA(1.00)[]; NEURAL_SPAM_MEDIUM(0.43)[0.428,0]; TO_MATCH_ENVRCPT_SOME(0.00)[]; RCPT_COUNT_TWO(0.00)[2]; NEURAL_SPAM_LONG(0.99)[0.986,0]; R_SPF_NA(0.00)[]; FREEMAIL_TO(0.00)[gmail.com]; RCVD_NO_TLS_LAST(0.10)[]; FROM_EQ_ENVFROM(0.00)[]; R_DKIM_NA(0.00)[]; SUBJECT_ENDS_QUESTION(1.00)[]; ASN(0.00)[asn:48666, ipnet:185.238.136.0/22, country:RU]; MIME_TRACE(0.00)[0:+]; IP_SCORE(0.42)[ip: (-0.17), ipnet: 185.238.136.0/22(0.47), asn: 48666(1.77), country: RU(0.01)]; RCVD_COUNT_TWO(0.00)[2] X-BeenThere: freebsd-stable@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Production branch of FreeBSD source code List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 01 Dec 2019 23:21:46 -0000 * Konstantin Belousov (kostikbel@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(&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. > > > > 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 the problem ? It's not limited with this aspect. Here are some possible cases: - no processes with the same UID - one process with the same UID which can be signalled - one process with the same UID which cannot (say, because of MAC) be signalled - case of one process with the different UID which can be signalled The kill(-1, 0) with the given UID results are/should be, correspondingly: As per documentation: ESRCH, 0, EPERM, ESRCH As per current implementation: EPERM, 0, EPERM, 0 As per implementation which relies solely on *_cansignal: ESRCH, 0, ESRCH, 0 (unrelated process signalled) As you can see, relying soleley on p_cansignel would fix one case, but break the others. We may need a trimmed down variant of p_cansignal for this. Or make the latter take the mask of which checks it should perform or skip. -- Dmitry Marakasov . 55B5 0596 FF1E 8D84 5F56 9510 D35A 80DD F9D2 F77D amdmi3@amdmi3.ru ..: https://github.com/AMDmi3