Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <YRrdvKEu2JQxLI5n@zeniv-ca.linux.org.uk> <877dgkvsog.fsf@disp2133> <YSXRL4Gt4SVLa+Hl@zeniv-ca.linux.org.uk>
In-Reply-To: <YSXRL4Gt4SVLa+Hl@zeniv-ca.linux.org.uk>
From:   Linus Torvalds <torvalds@linux-foundation.org>
Date:   Fri, 27 Aug 2021 11:54:57 -0700
Message-ID: <CAHk-=whnUy1UHTt6dpgCdBgz4tu1JCVmrN8ouJ==fpDkT+kwpw@mail.gmail.com>
Subject: Re: [PATCH][RFC] fix PTRACE_KILL
To:     Al Viro <viro@zeniv.linux.org.uk>
Cc:     "Eric W. Biederman" <ebiederm@xmission.com>,
        Oleg Nesterov <oleg@redhat.com>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

[ Sorry, this got missed by other stuff in my inbox ]

On Tue, Aug 24, 2021 at 10:12 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> The change I'd proposed makes PTRACE_KILL deliver SIGKILL regardless of
> the target state; yours is arguably what we should've done from the very
> beginning (what we used to have prior to 0.99pl14 and what all other
> Unices had been doing all along), but it's a visible change wrt error
> returns and I don't see any sane way to paper over that part.
>
> Linus, what would you prefer?  I've no strong preferences here...

Honestly, I have no huge preferences either, simply because this has
clearly been broken so long that people who care have worked around
the breakage already, or it just didn't matter enough.

Your patch looks fine to me, although looking at it I get the feeling
that we might as well do it inside "ptrace_check_attach()", and that
we should have just passed that function the request (instead of that
odd "ignore_state" argument).

ptrace_check_attach() already gets that tasklist_lock that the code
requires, and could easily do the PTRACE_KILL checking. So I think
that might be an additional cleanup.

But your patch is targeted and doesn't look wrong to me either.

I don't think Eric's patch works, because if I read that one right, it
would actually do that "wait_task_inactive()" which is very much
against the whole point of PTRACE_KILL.  We want to kill the target
asap, and regardless of where it is stuck.

              Linus