Received: by 2002:ad5:474a:0:0:0:0:0 with SMTP id i10csp10969109imu; Thu, 6 Dec 2018 09:25:59 -0800 (PST) X-Google-Smtp-Source: AFSGD/XFVFn60oW9lIuAmPjYoCmP7tocTQP5HA++1GsXgaRJwhTLXR6Gh+xr77rwnUJeiWWkFpz2 X-Received: by 2002:a17:902:468:: with SMTP id 95mr28752888ple.3.1544117159563; Thu, 06 Dec 2018 09:25:59 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1544117159; cv=none; d=google.com; s=arc-20160816; b=KZ45ZjEZn745aX9tu5UM0t0iRwrzFGSmABLFIexokUfjGdZEd37/j+tNlKZ6Jso1MK NKrWC15PTbgx4rjo3VlSguY6AGsSkOosB9+dOPiy0vHcdxkRmrr2D2WvOC/KlXMRqx4q VpwO6Yk/E0Xy7D2CRIf/IcaVxqI7zNaWAV6Afw+2CJsLjqJlGB2CvZcutxOVt3Rfws9f qzJYYs/2S/dE4nHB3r6P98EzINTxeKVa3y1OFMEreY95Kp/y+PKwCevdS88RgwfZ8RLc 3548a7dY/PDmhYV7gX+i1LnxS9Yhf2nNDsOfojLhEquwQRcWays56pMtq5lqKVdtIFg8 HQMQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:subject:mime-version:user-agent :message-id:in-reply-to:date:references:cc:to:from; bh=xtgwlUek3wcPHwfLyF/pGrgoEPeXSdWepWnIUmu7vZI=; b=Wq8PFg3gWnHFhDIiv/WyyqXHNeKtOf3tWWa9Lrp+s9PLhcKx2aImnOVATwJSOHMRkG bJkhQPUyulE15yoXU4V6wK+euqy5cUbMja2xvS+O3gC6NALVs1MfXZjRJtm+/nwlSNQ9 Yk2mSUuVbIeNnjJaftGQ6DIhJmWrfBwSMhyHDGM000GVlJLGqMD/5jrWwYLvWOxmVeB+ SGpRpCpjrakBFThiql3vGZ217zuWke3CU/8GGvmQJlDc6J7F943Lo9zv1K20ZJv66RND BjTZINPToLLHtkPWrwyuupDHMNm08yKQuzGNep4kyflQzdZ4Tv9DmrqerUdVc2208U1X SmPA== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=xmission.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id f5si650768plo.422.2018.12.06.09.25.30; Thu, 06 Dec 2018 09:25:59 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=xmission.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1725970AbeLFRYq (ORCPT + 99 others); Thu, 6 Dec 2018 12:24:46 -0500 Received: from out03.mta.xmission.com ([166.70.13.233]:38027 "EHLO out03.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725889AbeLFRYq (ORCPT ); Thu, 6 Dec 2018 12:24:46 -0500 Received: from in01.mta.xmission.com ([166.70.13.51]) by out03.mta.xmission.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.87) (envelope-from ) id 1gUxOF-0008HB-Cg; Thu, 06 Dec 2018 10:24:43 -0700 Received: from ip68-227-174-240.om.om.cox.net ([68.227.174.240] helo=x220.xmission.com) by in01.mta.xmission.com with esmtpsa (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.87) (envelope-from ) id 1gUxOB-0007e2-6K; Thu, 06 Dec 2018 10:24:43 -0700 From: ebiederm@xmission.com (Eric W. Biederman) To: Daniel Colascione Cc: Christian Brauner , linux-kernel , Linux API , Andy Lutomirski , Arnd Bergmann , "Serge E. Hallyn" , Jann Horn , Andrew Morton , Oleg Nesterov , Aleksa Sarai , Al Viro , Linux FS Devel , Tim Murray , linux-man , Kees Cook , Florian Weimer , tglx@linutronix.de, x86@kernel.org References: <20181206121858.12215-1-christian@brauner.io> <87sgzahf7k.fsf@xmission.com> Date: Thu, 06 Dec 2018 11:24:28 -0600 In-Reply-To: (Daniel Colascione's message of "Thu, 6 Dec 2018 08:17:18 -0800") Message-ID: <878t12efg3.fsf@xmission.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-XM-SPF: eid=1gUxOB-0007e2-6K;;;mid=<878t12efg3.fsf@xmission.com>;;;hst=in01.mta.xmission.com;;;ip=68.227.174.240;;;frm=ebiederm@xmission.com;;;spf=neutral X-XM-AID: U2FsdGVkX183fo99KdM7s83XVknY4FOfpoZT6zBIXDY= X-SA-Exim-Connect-IP: 68.227.174.240 X-SA-Exim-Mail-From: ebiederm@xmission.com X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on sa07.xmission.com X-Spam-Level: X-Spam-Status: No, score=-0.2 required=8.0 tests=ALL_TRUSTED,BAYES_50, DCC_CHECK_NEGATIVE,T_TM2_M_HEADER_IN_MSG,T_TooManySym_01, T_TooManySym_02 autolearn=disabled version=3.4.2 X-Spam-Report: * -1.0 ALL_TRUSTED Passed through trusted hosts only via SMTP * 0.8 BAYES_50 BODY: Bayes spam probability is 40 to 60% * [score: 0.4999] * 0.0 T_TM2_M_HEADER_IN_MSG BODY: No description available. * -0.0 DCC_CHECK_NEGATIVE Not listed in DCC * [sa07 1397; Body=1 Fuz1=1 Fuz2=1] * 0.0 T_TooManySym_02 5+ unique symbols in subject * 0.0 T_TooManySym_01 4+ unique symbols in subject X-Spam-DCC: XMission; sa07 1397; Body=1 Fuz1=1 Fuz2=1 X-Spam-Combo: ;Daniel Colascione X-Spam-Relay-Country: X-Spam-Timing: total 3770 ms - load_scoreonly_sql: 0.09 (0.0%), signal_user_changed: 2.9 (0.1%), b_tie_ro: 1.84 (0.0%), parse: 1.22 (0.0%), extract_message_metadata: 17 (0.4%), get_uri_detail_list: 3.0 (0.1%), tests_pri_-1000: 33 (0.9%), tests_pri_-950: 2.1 (0.1%), tests_pri_-900: 1.72 (0.0%), tests_pri_-90: 48 (1.3%), check_bayes: 46 (1.2%), b_tokenize: 20 (0.5%), b_tok_get_all: 12 (0.3%), b_comp_prob: 6 (0.2%), b_tok_touch_all: 4.8 (0.1%), b_finish: 0.69 (0.0%), tests_pri_0: 475 (12.6%), check_dkim_signature: 0.62 (0.0%), check_dkim_adsp: 2.2 (0.1%), poll_dns_idle: 3172 (84.1%), tests_pri_10: 2.2 (0.1%), tests_pri_500: 3183 (84.4%), rewrite_mail: 0.00 (0.0%) Subject: Re: [PATCH v4] signal: add taskfd_send_signal() syscall X-Spam-Flag: No X-SA-Exim-Version: 4.2.1 (built Thu, 05 May 2016 13:38:54 -0600) X-SA-Exim-Scanned: Yes (on in01.mta.xmission.com) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Daniel Colascione writes: > On Thu, Dec 6, 2018 at 7:02 AM Eric W. Biederman wrote: >> >> Christian Brauner writes: >> >> > The kill() syscall operates on process identifiers (pid). After a process >> > has exited its pid can be reused by another process. If a caller sends a >> > signal to a reused pid it will end up signaling the wrong process. This >> > issue has often surfaced and there has been a push [1] to address this >> > problem. >> > >> > This patch uses file descriptors (fd) from proc/ as stable handles on >> > struct pid. Even if a pid is recycled the handle will not change. The fd >> > can be used to send signals to the process it refers to. >> > Thus, the new syscall taskfd_send_signal() is introduced to solve this >> > problem. Instead of pids it operates on process fds (taskfd). >> >> I am not yet thrilled with the taskfd naming. > > Both the old and new names were fine. Do you want to suggest a name at > this point? You can't just say "I don't like this. Guess again" > forever. Both names suck, as neither name actually describes what the function is designed to do. Most issues happen at the interface between abstractions. A name that confuses your users will just make that confusion more likely. So it is important that we do the very best with the name that we can do. We are already having questions about what happens when you perform the non-sense operation of sending a signal to a zombie. It comes up because there are races when a process may die and you are not expecting it. That is an issue with the existing signal sending API, that has caused confusion. That isn't half as confusing as the naming issue. A task in linux is a single thread. A process is all of the threads. If we are going to support both cases it doesn't make sense to hard code a single case in the name. I would be inclined to simplify things and call the syscall something like "fdkill(int fd, struct siginfo *info, int flags)". Or perhaps just "fd_send_signal(int fd, struct siginfo *info, int flags)". Then we are not overspecifying what the system call does in the name. Plus it makes it clear that the fd specifies where the signal goes. Something I see that by your reply below that you were confused about. >> Is there any plan to support sesssions and process groups? > > Such a thing could be added with flags in the future. Why complicate > this patch? Actually that isn't the way this is designed. You would have to have another kind of file descriptor. I am asking because it goes to the question of naming and what we are trying to do here. We don't need to implement that but we have already looked into this kind of extensibility. If we want the extensibility we should make room for it, or just close the door. Having the door half open and a confusing interface is a problem for users. >> I am concerned about using kill_pid_info. It does this: >> >> >> rcu_read_lock(); >> p = pid_task(pid, PIDTYPE_PID); >> if (p) >> error = group_send_sig_info(sig, info, p, PIDTYPE_TGID); >> rcu_read_unlock(); >> >> That pid_task(PIDTYPE_PID) is fine for existing callers that need bug >> compatibility. For new interfaces I would strongly prefer pid_task(PIDTYPE_TGID). > > What is the bug that PIDTYPE_PID preserves? I am not 100% certain all of the bits for this to matter have been merged yet but we are close enough that it would not be hard to make it matter. There are two strange behaviours of ordinary kill on the linux kernel that I am aware of. 1) kill(thread_id,...) where the thread_id is not the id of the first thread and the thread_id thus the pid of the process sends the signal to the entire process. Something that arguably should not happen. 2) kill(pid,...) where the original thread has exited performs the permission checks against the dead signal group leader. Which means that the permission checks for sending a signal are very likely wrong for a multi-threaded processes that calls a function like setuid. To fix the second case we are going to have to perform the permission checks on a non-zombie thread. That is something that is straight forward to make work with PIDTYPE_TGID. It is not so easy to make work with PIDTYPE_PID. I looked and it doesn't look like I have merged the logic of having PIDTYPE_TGID point to a living thread when the signal group leader exits and becomes a zombie. It isn't hard but it does require some very delicate surgery on the code, so that we don't break some of the historic confusion of threads and process groups. Eric