linux-kernel - Re: [PATCH] proc: allow killing processes via file descriptors

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20181119154923.GW3505@e103592.cambridge.arm.com>
Date:   Mon, 19 Nov 2018 15:49:25 +0000
From:   Dave Martin <Dave.Martin@....com>
To:     Christian Brauner <christian@...uner.io>
Cc:     ebiederm@...ssion.com, linux-kernel@...r.kernel.org,
        serge@...lyn.com, jannh@...gle.com, luto@...nel.org,
        akpm@...ux-foundation.org, oleg@...hat.com, cyphar@...har.com,
        viro@...iv.linux.org.uk, linux-fsdevel@...r.kernel.org,
        linux-api@...r.kernel.org, dancol@...gle.com, timmurray@...gle.com,
        Kees Cook <keescook@...omium.org>
Subject: Re: [PATCH] proc: allow killing processes via file descriptors

On Sun, Nov 18, 2018 at 12:17:51PM +0100, Christian Brauner wrote:
> With this patch an open() call on /proc/<pid> will give userspace a handle
> to struct pid of the process associated with /proc/<pid>. This allows to
> maintain a stable handle on a process.
> I have been discussing various approaches extensively during technical
> conferences this year culminating in a long argument with Eric at Linux
> Plumbers. The general consensus was that having a handle on a process
> will be something that is very simple and easy to maintain with the
> option of being extensible via a more advanced api if the need arises. I
> believe that this patch is the most simple, dumb, and therefore
> maintainable solution.
> 
> The need for this has arisen in order to reliably kill a process without
> running into issues of the pid being recycled as has been described in the
> rejected patch [1]. To fulfill the need described in that patchset a new

It would certainly be good to fix this.  IIUC, things like pkill(1) are
a gamble today and can probably kill a process that doesn't fulfil the
match criteria due to PID recycling.  (If not, I'd certainly like to
understand how that is prevented.)

> ioctl() PROC_FD_SIGNAL is added. It can be used to send signals to a
> process via a file descriptor:
> 
> int fd = open("/proc/1234", O_DIRECTORY | O_CLOEXEC);
> ioctl(fd, PROC_FD_SIGNAL, SIGKILL);
> close(fd);
> 
> Note, the stable handle will allow us to carefully extend this feature in
> the future.

A concern here would be that an fd-based shadow API may need to be
created, duplicating the whole PID-based API that already exists.

However, so long as the PID is stabilised against recycling, an ordinary
kill() call seems fine as a way to kill the target process, and no new
API or permission model seems to be needed.

It occurs to me that a mechanism for holding a reference on a third
process already exists: ptrace.

Suppose we were to have something like

	ptrace(PTRACE_MONITOR, pid, 0, 0);

that subscribes the caller for the same set of notifications via wait()
as the process' real parent gets, and prevents PID recycling until the
zombie is consumed be everyone who is subscribed.

Multiple PTRACE_MONITOR attachments could be allowed for a given target,
in addition to the real parent and regular ptrace-parent (if any).

There are a couple of wrinkles:

 * ptrace() operates on tasks, not processes, so the precise semantics
of PTRACE_MONITOR attachment would need a bit of thought.

 * Odd mechanisms for discovering PIDs, like the unix(7) SCM_CREDENTIALS
message might need a special variant to get a PTRACE_MONITOR attachment
along with the message.  This variant behaviour would need to be opt-in
for the recipient.

OTOH, extending ptrace may bring problems of its own :/

Cheers
---Dave