Andy Lutomirski <l...@amacapital.net> writes:
> On Apr 8, 2016 12:05 PM, "Linus Torvalds" <torva...@linux-foundation.org>
> wrote:
>>
>> On Fri, Apr 8, 2016 at 11:51 AM, Eric W. Biederman
>> <ebied...@xmission.com> 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.
Also using any property from a mount namespace or a vfs mount is usually
an error, as it is an inconsistent model.
Plus I don't think what you are suggesting would make anything simpler
or easier to reason about. It only costs me about 3 lines of code to
perform the permission checks. The complaint is that they exist at all.
Eric
