From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Google-Smtp-Source: ACJfBoti/UYQrhdbPt6bU320vKZpbaAIvRI4syWpuSBN4eRB321BF0DTI/yORgVhaQXjviuiTc59 ARC-Seal: i=1; a=rsa-sha256; t=1516214306; cv=none; d=google.com; s=arc-20160816; b=TCMNlox+68MbSEKPhy9cB99XnAPUcfo2PGdtOaq2ZxIzSiVm9yImi9AgWh3cEss4ij JGjEBnqmzspjYoN11jBUqFU/xkBi9qluTM5LMgtjTZlkxOJUf0NnVmvc1X5j+OP2gFqP 8Ylmj3Trz9hDLINqsodj8/cfm16hRB8S1mpPT7kem2xvQuezIbExcwrP6Fyv/W06gFYr 0cfeUqYMJEM8YGtNNH7VQjk+TVG/w0DWlbC6jEMeBY3Dv3aBxT07uTlCvgT+l9aeeGzP Zj5OEfZezVp3y/3NdjO1kKC5Tc1VjcLRBfVrY5KXITWt4iQSP+gr8rG6iuxX2Pt2Nveg DA+Q== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=subject:mime-version:user-agent:message-id:in-reply-to:date :references:cc:to:from:arc-authentication-results; bh=KVr4UJEspwc+qJRrExFesd0/Itw4gwsLf520NwPJKfo=; b=j66WvYhHfNu9tiWTlltOD72e+2GKh38RJpjnOFlpV4Z6hBXteowd/4xbj0yo7lZyjT CAJcJFO733wOYjiihxp33boAQufRTFG79aEHENi46N0Q1h2ceIbW734L1yYd+9dboaLx KNka/oGFWg+DK++pUqz3FiiH400+MZtM1ZgFQHTxgZialcs08yDIyQ/xqMKYd0dugm60 e/przRmTDJF69ffKiNoVDE8KrtyyIR4jtdqcYUhuGrGotLjU+r9RJ4a9BFJzRyixKVbw se1k9ZfnyjZYepoEUioXLNqp8UI7Ak3nhgXu1CEJraF1VJ69LLnZelpKkXZ5Rp0yQ9Rr 5rXg== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of ebiederm@xmission.com designates 166.70.13.231 as permitted sender) smtp.mailfrom=ebiederm@xmission.com Authentication-Results: mx.google.com; spf=pass (google.com: domain of ebiederm@xmission.com designates 166.70.13.231 as permitted sender) smtp.mailfrom=ebiederm@xmission.com From: ebiederm@xmission.com (Eric W. Biederman) To: Oleg Nesterov Cc: Kirill Tkhai , gregkh@linuxfoundation.org, jslaby@suse.com, linux-kernel@vger.kernel.org References: <151619233415.5683.18062849657787533510.stgit@localhost.localdomain> <151619277281.5683.16110625178528288163.stgit@localhost.localdomain> <87shb4floe.fsf@xmission.com> <20180117173415.GA7964@redhat.com> <87tvvke5p0.fsf@xmission.com> <20180117180450.GA8181@redhat.com> Date: Wed, 17 Jan 2018 12:37:33 -0600 In-Reply-To: <20180117180450.GA8181@redhat.com> (Oleg Nesterov's message of "Wed, 17 Jan 2018 19:04:50 +0100") Message-ID: <87zi5ccowi.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=1ebsbR-0004s1-Dj;;;mid=<87zi5ccowi.fsf@xmission.com>;;;hst=in02.mta.xmission.com;;;ip=97.121.73.102;;;frm=ebiederm@xmission.com;;;spf=neutral X-XM-AID: U2FsdGVkX19Z2xgpvlMtzIkjJFtzGUrEqpEgLRkWozo= X-SA-Exim-Connect-IP: 97.121.73.102 X-SA-Exim-Mail-From: ebiederm@xmission.com X-Spam-Report: * -1.0 ALL_TRUSTED Passed through trusted hosts only via SMTP * 0.7 XMSubLong Long Subject * 0.0 TVD_RCVD_IP Message was received from an IP address * 0.0 T_TM2_M_HEADER_IN_MSG BODY: No description available. * 0.8 BAYES_50 BODY: Bayes spam probability is 40 to 60% * [score: 0.5000] * -0.0 DCC_CHECK_NEGATIVE Not listed in DCC * [sa07 1397; Body=1 Fuz1=1 Fuz2=1] * 0.0 T_TooManySym_01 4+ unique symbols in subject * 0.0 T_TooManySym_02 5+ unique symbols in subject X-Spam-DCC: XMission; sa07 1397; Body=1 Fuz1=1 Fuz2=1 X-Spam-Combo: ;Oleg Nesterov X-Spam-Relay-Country: X-Spam-Timing: total 298 ms - load_scoreonly_sql: 0.03 (0.0%), signal_user_changed: 2.6 (0.9%), b_tie_ro: 1.90 (0.6%), parse: 0.77 (0.3%), extract_message_metadata: 12 (4.1%), get_uri_detail_list: 2.9 (1.0%), tests_pri_-1000: 6 (1.9%), tests_pri_-950: 1.17 (0.4%), tests_pri_-900: 0.97 (0.3%), tests_pri_-400: 26 (8.9%), check_bayes: 25 (8.5%), b_tokenize: 8 (2.5%), b_tok_get_all: 9 (3.0%), b_comp_prob: 2.4 (0.8%), b_tok_touch_all: 3.9 (1.3%), b_finish: 0.58 (0.2%), tests_pri_0: 241 (81.0%), check_dkim_signature: 0.50 (0.2%), check_dkim_adsp: 2.7 (0.9%), tests_pri_500: 4.1 (1.4%), rewrite_mail: 0.00 (0.0%) Subject: Re: [PATCH v2 1/3] Revert "do_SAK: Don't recursively take the tasklist_lock" 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 in02.mta.xmission.com) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: =?utf-8?q?1589843366609121628?= X-GMAIL-MSGID: =?utf-8?q?1589865933181454657?= X-Mailing-List: linux-kernel@vger.kernel.org List-ID: Oleg Nesterov writes: > On 01/17, Eric W. Biederman wrote: >> >> Oleg Nesterov writes: >> >> > On 01/17, Eric W. Biederman wrote: >> > >> >> Kirill Tkhai writes: >> >> >> >> > This reverts commit 20ac94378de5. >> >> > >> >> > send_sig() does not take tasklist_lock for a long time, >> >> > so this commit and the problem it solves are not relevant >> >> > anymore. >> >> > >> >> > Also, the problem of force_sig() is it clears SIGNAL_UNKILLABLE >> >> > flag, thus even global init may be killed by __do_SAK(), >> >> > which is definitely not the expected behavior. >> >> >> >> Actually it is. >> >> >> >> SAK should kill everything that has the tty open. If init opens the tty >> >> I am so sorry, it can not operate correctly. init should not have your >> >> tty open. >> > >> > OK, but then we need "force" in other places too. __do_SAK() does send_sig(SIGKILL) >> > in do_each_pid_task(PIDTYPE_SID) and if signal->tty == tty. >> > >> > Plus force_sig() is not rcu-friendly. >> > >> > So I personally agree with this change. Whether we want to kill the global init >> > or not should be discussed, if we want to do this __do_SAK() should use >> > SEND_SIG_FORCED and this is what Kirill is going to do (iiuc), but this needs >> > another patch. >> >> To operate correctly, do_SAK() needs to kill everything that has the tty >> open. Unless we can make that guarantee I don't see the point of >> changing do_SAK. > > OK, but how this connects to this change? > > Again, this force_sig() doesn't match other send_sig()'s in __do_SAK(), > and Kirill is going to turn them all into send_sig_info(SEND_SIG_FORCED). > Just we need to discuss whether we need to skip the global init or not > but this is another story. > > So why do you dislike this change? > > force_sig() should die anyway. At least in its current form, it should not > be used unless task == current. But this is off-topic. I see that as a fair criticism of force_sig, and a good argument to use send_sig(SIGKILL, SEND_SIG_FORCED). Which will kill the global init. What I don't like is a bunch of patches to introduce races and make something more racy that should be a logical atomic operation to kill all of the processes that have a certain tty open so that on the next open there will be exactly one process with the tty open. I guess it is a super vhangup. The purported purpose of SAK is for security. Breaking security for performance is not ok. See what that just did to intel. So we either need to say do_SAK is broken. In which case the proper fix is to just delete the thing. Or we need not to ensure the final implemenation is an atomic kill of everything that has the tty open. I think if these patches can justify using rcu with races in the current do_SAK implementation than I think do_SAK can just die. Removing do_SAK would be a much better way of ensuring do_SAK does not have long lock hold times. Races in do_SAK do not justify saying it is ok to introduce more races in do_SAK. Either do_SAK is not fit for purpose or it is. Eric