Re: [PATCH 01/13] devpts: Teach /dev/ptmx to find the associated devpts via path lookup

[Andy Lutomirski]

On Fri, Apr 8, 2016 at 2:29 PM, Eric W. Biederman <ebiederm@xxxxxxxxxxxx> wrote:
> Andy Lutomirski <luto@xxxxxxxxxxxxxx> writes:
>
>> On Apr 8, 2016 12:05 PM, "Linus Torvalds" <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
>>>
>>> On Fri, Apr 8, 2016 at 11:51 AM, Eric W. Biederman
>>> <ebiederm@xxxxxxxxxxxx> wrote:
>>> >
>>> > Given that concern under the rule we don't break userspace we have to
>>> > check the permissions of /dev/pts/ptmx when we are creating a new pty,
>>> > on a instance of devpts that was created with newinstance.
>>>
>>> The rule is that we don't break existing installations.
>>>
>>> If somebody has root and installs a "ptmx" node in an existing mount
>>> space next to a pts subdirectory, that's not a security issue, nor is
>>> it going to break any existing installation.
>>
>> What Eric's saying is that you don't have to be root for this.
>>
>> But Eric, I think there might be a better mitigation.  For a ptmx
>> chardev, just fail the open if the chardev's vfsmount or the devpts's
>> vfsmount doesn't belong to the same userns as the devpts's superblock.
>> After all, setting this attack up requires the caps on one of the
>> vfsmounts, and if you have those caps you could attack your own devpts
>> instance quite easily.  Would that work?
>
> I don't think so.  For one it depends on getting s_user_ns which should
> happen but is not there yet.  For another the way you describe
> it you would break the case of
>
>         unshare(CLONE_NEWUSER);
>         unshare(CLONE_NEWNS);
>         open("/dev/ptmx");
>
> Which is actually more likely to break userspace than anything else we
> have considered.  I know people actually do that.
>

Hmm, you're right.  Never mind.

--Andy