All of lore.kernel.org
 help / color / mirror / Atom feed
From: Pedro Alves <palves@redhat.com>
To: Oleg Nesterov <oleg@redhat.com>,
	Denys Vlasenko <vda.linux@googlemail.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Dmitry Vyukov <dvyukov@google.com>,
	Alexander Potapenko <glider@google.com>,
	Eric Dumazet <edumazet@google.com>,
	Jan Kratochvil <jan.kratochvil@redhat.com>,
	Julien Tinnes <jln@google.com>, Kees Cook <keescook@google.com>,
	Kostya Serebryany <kcc@google.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	"Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com>,
	Robert Swiecki <swiecki@google.com>,
	Roland McGrath <roland@hack.frob.com>,
	syzkaller@googlegroups.com,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	"gdb@sourceware.org" <gdb@sourceware.org>
Subject: Re: [PATCH 1/2] wait/ptrace: always assume __WALL if the child is traced
Date: Mon, 26 Oct 2015 12:08:56 +0000	[thread overview]
Message-ID: <562E17D8.4000108@redhat.com> (raw)
In-Reply-To: <20151025155440.GB2043@redhat.com>

On 10/25/2015 03:54 PM, Oleg Nesterov wrote:
> On 10/22, Denys Vlasenko wrote:
>>
>> On Wed, Oct 21, 2015 at 11:47 PM, Oleg Nesterov <oleg@redhat.com> wrote:
>>> On 10/21, Denys Vlasenko wrote:
>>>>
>>>> On 10/21/2015 09:59 PM, Denys Vlasenko wrote:
>>>>> On 10/21/2015 12:31 AM, Andrew Morton wrote:
>>>>>> Well, to fix this a distro needs to roll out a new kernel.  Or a new
>>>>>> init(8).  Is there any reason to believe that distributing/deploying a
>>>>>> new kernel is significantly easier for everyone?  Because fixing init
>>>>>> sounds like a much preferable solution to this problem.
>>>>>
>>>>> People will continue to write new init(8) implementations,
>>>>> and they will miss this obscure case.
>>>>>
>>>>> Before this bug was found, it was considered possible to use
>>>>> a shell script as init process. What now, every shell needs to add
>>>>> __WALL to its waitpids?
>>>
>>> Why not? I think it can safely use __WALL too.
>>
>> Because having any userspace program which can happen to be init,
>> which includes all shells out there in the wild
>> (bash, dash, ash, ksh, zsh, msh, hush,...)
>> learn about __WALL is wrong: apart from this wart, they do not have
>> to use any Linux-specific code. It can all be perfectly legitimate POSIX.
> 
> Yes, this is true. I meant that they could safely use __WALL to, but I
> understand that this change can be painful.
> 
>>> Sure. But people do the things which were never intended to be
>>> used all the time. We simply can not know if this "feature"
>>> already has a creative user or not.
>>
>> It won't be unfixable: they will just have to switch from PTRACE_TRACEME
>> to PTRACE_ATTACH.
>>
>> As of now we do not know any people craz^W creative enough
>> to create a cross between init and strace. If such specimens would
>> materialize, don't they deserve to have to make that change?
> 
> This also applies to people who use bash/whatever as /sbin/init and allow
> the untrusted users to run the exploits ;) I do not know who is more crazy.
> 
> In any case, the real question is whether we should change the kernel to
> fix the problem, or ask the distros to fix their init's. In the former
> case 1/2 looks simpler/safer to me than the change in ptrace_traceme(),
> and you seem to agree that 1/2 is not that bad.

A risk here seems to be that waitpid will start returning unexpected
(thread) PIDs to parent processes, and it's not unreasonable to assume
that e.g., a program asserts that waitpid either returns error or a
known (process) PID.

That's not an init-only issue, but something that might affect any
process that runs a child that happens to decide to
call PTRACE_TRACEME.

The ptrace man page says:

 "A process can initiate a trace by calling fork(2) and having the resulting
 child do a PTRACE_TRACEME, followed (typically) by an execve(2)."

Given that, can we instead make the kernel error out on PTRACE_TRACEME issued
from a non-leader thread?  Then between PTRACE_TRACEME and the parent's
waitpid, __WALL or !__WALL should make no difference.

(Also, in the original test case, if the child gets/raises a signal or execs
before exiting, the bash/init/whatever process won't be issuing PTRACE_CONT,
and the child will thus end up stuck (though should be SIGKILLable,
I believe).  All this because PTRACE_TRACEME is broken by design by making it
be the child's choice whether to be traced...)

Thanks,
Pedro Alves


  reply	other threads:[~2015-10-26 12:09 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-20 17:17 [PATCH 0/2] wait/ptrace: always assume __WALL if the child is traced Oleg Nesterov
2015-10-20 17:17 ` [PATCH 1/2] " Oleg Nesterov
2015-10-20 22:31   ` Andrew Morton
2015-10-21  3:27     ` Vasily Averin
2015-10-21 17:41     ` Oleg Nesterov
2015-10-21 19:47       ` Andrew Morton
2015-10-21 20:44         ` Oleg Nesterov
2015-10-21 19:59     ` Denys Vlasenko
2015-10-21 20:31       ` Denys Vlasenko
2015-10-21 21:47         ` Oleg Nesterov
2015-10-21 23:27           ` Denys Vlasenko
2015-10-25 15:54             ` Oleg Nesterov
2015-10-26 12:08               ` Pedro Alves [this message]
2015-10-28 16:11                 ` Oleg Nesterov
2015-10-28 15:43                   ` Pedro Alves
2015-10-28 19:02                     ` Oleg Nesterov
2015-10-22 13:51   ` Denys Vlasenko
2015-10-20 17:17 ` [PATCH 2/2] wait: allow sys_waitid() to use __WNOTHREAD/__WCLONE/__WALL Oleg Nesterov
2015-10-20 17:36 ` [PATCH 0/2] wait/ptrace: always assume __WALL if the child is traced Oleg Nesterov
2015-10-22 14:40 ` Pedro Alves
2015-10-25 15:42   ` Oleg Nesterov

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=562E17D8.4000108@redhat.com \
    --to=palves@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=dvlasenk@redhat.com \
    --cc=dvyukov@google.com \
    --cc=edumazet@google.com \
    --cc=gdb@sourceware.org \
    --cc=glider@google.com \
    --cc=jan.kratochvil@redhat.com \
    --cc=jln@google.com \
    --cc=kcc@google.com \
    --cc=keescook@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mtk.manpages@gmail.com \
    --cc=oleg@redhat.com \
    --cc=roland@hack.frob.com \
    --cc=swiecki@google.com \
    --cc=syzkaller@googlegroups.com \
    --cc=torvalds@linux-foundation.org \
    --cc=vda.linux@googlemail.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.