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

[Andrew Lutomirski]

On Tue, Jun 11, 2013 at 11:38 AM,  <vcaputo@xxxxxxxxxxxxxxxxx> wrote:
> On Tue, Jun 11, 2013 at 09:48:22AM -0700, Andy Lutomirski wrote:
>> On 06/10/2013 06:23 PM, vcaputo@xxxxxxxxxxxxxxxxx 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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/