LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
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>
Subject: Re: [PATCH][RFC] fix PTRACE_KILL
Date: Fri, 27 Aug 2021 11:54:57 -0700	[thread overview]
Message-ID: <CAHk-=whnUy1UHTt6dpgCdBgz4tu1JCVmrN8ouJ==fpDkT+kwpw@mail.gmail.com> (raw)
In-Reply-To: <YSXRL4Gt4SVLa+Hl@zeniv-ca.linux.org.uk>

[ 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

  reply	other threads:[~2021-08-27 18:55 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-16 21:50 Al Viro
2021-08-17 18:21 ` Eric W. Biederman
2021-08-25  5:12   ` Al Viro
2021-08-27 18:54     ` Linus Torvalds [this message]
2021-08-27 22:05       ` Eric W. Biederman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAHk-=whnUy1UHTt6dpgCdBgz4tu1JCVmrN8ouJ==fpDkT+kwpw@mail.gmail.com' \
    --to=torvalds@linux-foundation.org \
    --cc=ebiederm@xmission.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=oleg@redhat.com \
    --cc=viro@zeniv.linux.org.uk \
    --subject='Re: [PATCH][RFC] fix PTRACE_KILL' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).