From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <oleg@redhat.com>
X-Google-Smtp-Source: ACJfBosbGixNzJ1w+05OnSrJAscReL9qTChE3gbIwc6A2T777BCh6vOi7v/gKReKhol/+vaL1g/o
ARC-Seal: i=1; a=rsa-sha256; t=1516212293; cv=none;
        d=google.com; s=arc-20160816;
        b=TQa9IBcXQtxQRiMIWKnzeh4VkwwquCfT9eJp/lSB4sR0rDat4lpbxZG3L/mmzuzgmV
         /jpiFg65N8yCVFxSTkf6gpD7r1mt5QGRfBnq1OHieT1/sQY6Kj81cPg8eUFSsx92zyuS
         U2OOXd6d0QQYTvZhIceGG4dMfYHQlqHHSx+25qeQYS0XJRcizDm2TUHpLkjC6VZggsS0
         LdWB5D2SEafw1Yemfta/W1KPTmNzc4FDTsqUbLiXSUMZhpuYmX7DJY3fqUBLo7ibjffr
         KL7Rge+N330PqvFHM7hj8mR9lkC8qTFMbJPF670mo56f4NQxSE18isJVEFTYi96O1FSz
         3h9A==
ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816;
        h=user-agent:in-reply-to:content-disposition:mime-version:references
         :message-id:subject:cc:to:from:date:arc-authentication-results;
        bh=ENtYML27unxk53yf/94umGgRyWABpH7tsRSyJWkdzpU=;
        b=EcZFwLTkSVcY/FMDXzLPAE1HnJ2X0y+NiN5gAQp8WvjOmpiF56wPOvES0DcNGkcy3+
         fW9uILGVVWh61VU39h+iiLM7fNqKaMxMtpGyeoUg3CFCqbdXfpEA0aG7+7RTiA+4nL5O
         vBT54e0SJT3B6hT5gnvlEDT9MbMI1g+axT25Im4wMW92UgxZX6lQ9UfTSNFJDdMNpNaU
         rjXbWobHgIHwyOC4VkA2cSDdy8HaPdah6fivdP4HIdVykh95OfPkFnQ2zGWu93QlAIjb
         1aTvDz0EkiUnbQJJFTeQF9iacqnn4FZcPmCtUil7du+amJpOzLiL0mqktZpvAkKQW4jT
         fyfg==
ARC-Authentication-Results: i=1; mx.google.com;
       spf=pass (google.com: domain of oleg@redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=oleg@redhat.com;
       dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com
Authentication-Results: mx.google.com;
       spf=pass (google.com: domain of oleg@redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=oleg@redhat.com;
       dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com
Date: Wed, 17 Jan 2018 19:04:50 +0100
From: Oleg Nesterov <oleg@redhat.com>
To: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: Kirill Tkhai <ktkhai@virtuozzo.com>, gregkh@linuxfoundation.org,
	jslaby@suse.com, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 1/3] Revert "do_SAK: Don't recursively take the
 tasklist_lock"
Message-ID: <20180117180450.GA8181@redhat.com>
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>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <87tvvke5p0.fsf@xmission.com>
User-Agent: Mutt/1.5.24 (2015-08-30)
X-getmail-retrieved-from-mailbox: INBOX
X-GMAIL-THRID: =?utf-8?q?1589843366609121628?=
X-GMAIL-MSGID: =?utf-8?q?1589863821844820168?=
X-Mailing-List: linux-kernel@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>

On 01/17, Eric W. Biederman wrote:
>
> Oleg Nesterov <oleg@redhat.com> writes:
>
> > On 01/17, Eric W. Biederman wrote:
> >
> >> Kirill Tkhai <ktkhai@virtuozzo.com> 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.

Oleg.