linux-kernel - Re: adopt(pid_t pid) syscall proposal [patch included]

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] [day] [month] [year] [list]

Message-ID: <CAObL_7GG8sMhx_vC7WMEqPgrVTr6_A=dsuO5cSWPfPRo7hsS0w@mail.gmail.com>
Date:	Tue, 11 Jun 2013 11:47:10 -0700
From:	Andrew Lutomirski <luto@....edu>
To:	vcaputo@...generation.com
Cc:	linux-kernel@...r.kernel.org
Subject: Re: adopt(pid_t pid) syscall proposal [patch included]

On Tue, Jun 11, 2013 at 11:38 AM,  <vcaputo@...generation.com> wrote:
> On Tue, Jun 11, 2013 at 09:48:22AM -0700, Andy Lutomirski wrote:
>> On 06/10/2013 06:23 PM, vcaputo@...generation.com wrote:
>> >+                    if (!uid_eq(cred->euid, tcred->suid) &&
>> >+                        !uid_eq(cred->euid, tcred->uid)  &&
>> >+                        !uid_eq(cred->uid,  tcred->suid) &&
>> >+                        !uid_eq(cred->uid,  tcred->uid) &&
>> >+                        !ns_capable(cred->user_ns, CAP_KILL)) {
>> >+                            ret = -EPERM;
>> >+                            goto out_unlock;
>> >+                    }
>> >+
>>
>> That check's far too permissive.
>
> I suspected, but that's easily improved, I just wanted to get this out
> there and see what people thought, start the discussion, as well as
> get my use case functional.  Although I'm curious, what is your cause
> for concern with the existing checks?

For example, "!uid_eq(cred->euid, tcred->uid)" means that you can
adopt setuid things.  Looking at ptrace may be a good start.

>
>>
>> This sounds like it will break anything that uses wait and expects its
>> children to not be stolen out from under it.
>
> This thought crossed my mind, and originally I intended to restrict
> adoption to orphans who had become children of init.  It seemed like
> more general use might be desirable so I left that out.  If this
> really is a possibility worth preventing we could put that restriction
> on it.  To the best of my knowledge, nothing informs init of its
> becoming parent of an orphan, so we should be able to steal the orphan
> back without harm.  This still enables the use case of screen/tmux
> reattachment.
>
>>
>> Also, you'll have problems with screen -x or the default tmux shareable
>> configuration.  It sounds like this is better done in userspace.
>
> It just needs some determination of "session leader" applied to which
> attach adopts the backend before invoking adopt(), that logic goes in
> userspace.  I imagine the implementations of both screen and tmux
> already have such determinations happening for other reasons.

What I mean is: why not just teach your userspace tool to do this
without kernel help?

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/