Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1371063 > unrolled thread

[PATCH 00/13] devpts: New instances for every mount

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 00/13] devpts: New instances for every mount ebiederm@xmission.com (Eric W. Biederman) - 2016-04-05 02:20 +0200
    [PATCH 04/13] devpts: Stop rolling devpts_remount by hand in devpts_mount "Eric W. Biederman" <ebiederm@xmission.com> - 2016-04-05 03:50 +0200
    [PATCH 05/13] devpts: Fail early (if appropriate) on overmount "Eric W. Biederman" <ebiederm@xmission.com> - 2016-04-05 03:50 +0200
    [PATCH 08/13] devpts: Make devpts_kill_sb safe if fsi is NULL "Eric W. Biederman" <ebiederm@xmission.com> - 2016-04-05 03:50 +0200
    [PATCH 03/13] devpts: Cleanup newinstance parsing "Eric W. Biederman" <ebiederm@xmission.com> - 2016-04-05 03:50 +0200
    [PATCH 06/13] devpts: Use the same default mode for both /dev/ptmx and dev/pts/ptmx "Eric W. Biederman" <ebiederm@xmission.com> - 2016-04-05 03:50 +0200
    [PATCH 01/13] devpts: Teach /dev/ptmx to find the associated devpts via path lookup "Eric W. Biederman" <ebiederm@xmission.com> - 2016-04-05 03:50 +0200
      [PATCH 09/13] devpts: Move the creation of /dev/pts/ptmx into fill_super "Eric W. Biederman" <ebiederm@xmission.com> - 2016-04-05 03:50 +0200
      [PATCH 02/13] devpts: More obvious check for the system devpts in pty allocation "Eric W. Biederman" <ebiederm@xmission.com> - 2016-04-05 03:50 +0200
      [PATCH 13/13] devpts: Kill the DEVPTS_MULTIPLE_INSTANCE config option "Eric W. Biederman" <ebiederm@xmission.com> - 2016-04-05 03:50 +0200
      [PATCH 07/13] devpts: Move parse_mount_options into fill_super "Eric W. Biederman" <ebiederm@xmission.com> - 2016-04-05 03:50 +0200
      [PATCH 12/13] devpts: Always return a distinct instance when mounting "Eric W. Biederman" <ebiederm@xmission.com> - 2016-04-05 04:00 +0200
      [PATCH 11/13] vfs: Implement mount_super_once "Eric W. Biederman" <ebiederm@xmission.com> - 2016-04-05 04:00 +0200
      [PATCH 10/13] devpts: Simplify devpts_mount by using mount_nodev "Eric W. Biederman" <ebiederm@xmission.com> - 2016-04-05 04:00 +0200
      Re: [PATCH 01/13] devpts: Teach /dev/ptmx to find the associated  devpts via path lookup Al Viro <viro@ZenIV.linux.org.uk> - 2016-04-05 05:00 +0200
        Re: [PATCH 01/13] devpts: Teach /dev/ptmx to find the associated  devpts via path lookup Al Viro <viro@ZenIV.linux.org.uk> - 2016-04-05 05:10 +0200
          Re: [PATCH 01/13] devpts: Teach /dev/ptmx to find the associated devpts via path lookup ebiederm@xmission.com (Eric W. Biederman) - 2016-04-08 21:10 +0200
      Re: [PATCH 01/13] devpts: Teach /dev/ptmx to find the associated  devpts via path lookup Linus Torvalds <torvalds@linux-foundation.org> - 2016-04-07 18:10 +0200
        Re: [PATCH 01/13] devpts: Teach /dev/ptmx to find the associated devpts via path lookup ebiederm@xmission.com (Eric W. Biederman) - 2016-04-08 21:10 +0200
          Re: [PATCH 01/13] devpts: Teach /dev/ptmx to find the associated  devpts via path lookup Linus Torvalds <torvalds@linux-foundation.org> - 2016-04-08 21:10 +0200
            Re: [PATCH 01/13] devpts: Teach /dev/ptmx to find the associated devpts via path lookup ebiederm@xmission.com (Eric W. Biederman) - 2016-04-08 22:20 +0200
            Re: [PATCH 01/13] devpts: Teach /dev/ptmx to find the associated  devpts via path lookup Andy Lutomirski <luto@amacapital.net> - 2016-04-08 22:50 +0200
              Re: [PATCH 01/13] devpts: Teach /dev/ptmx to find the associated devpts via path lookup ebiederm@xmission.com (Eric W. Biederman) - 2016-04-08 23:50 +0200
                Re: [PATCH 01/13] devpts: Teach /dev/ptmx to find the associated  devpts via path lookup Andy Lutomirski <luto@amacapital.net> - 2016-04-09 00:00 +0200
                Re: [PATCH 01/13] devpts: Teach /dev/ptmx to find the associated  devpts via path lookup Linus Torvalds <torvalds@linux-foundation.org> - 2016-04-09 00:00 +0200
                  Re: [PATCH 01/13] devpts: Teach /dev/ptmx to find the associated devpts via path lookup ebiederm@xmission.com (Eric W. Biederman) - 2016-04-09 01:20 +0200
          Re: [PATCH 01/13] devpts: Teach /dev/ptmx to find the associated  devpts via path lookup One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk> - 2016-04-09 15:20 +0200
            Re: [PATCH 01/13] devpts: Teach /dev/ptmx to find the associated devpts via path lookup "H. Peter Anvin" <hpa@zytor.com> - 2016-04-09 16:20 +0200
              Re: [PATCH 01/13] devpts: Teach /dev/ptmx to find the associated devpts via path lookup ebiederm@xmission.com (Eric W. Biederman) - 2016-04-09 17:00 +0200
                Re: [PATCH 01/13] devpts: Teach /dev/ptmx to find the associated devpts via path lookup "H. Peter Anvin" <hpa@zytor.com> - 2016-04-10 00:50 +0200
                  Re: [PATCH 01/13] devpts: Teach /dev/ptmx to find the associated  devpts via path lookup Linus Torvalds <torvalds@linux-foundation.org> - 2016-04-10 02:10 +0200
                    Re: [PATCH 01/13] devpts: Teach /dev/ptmx to find the associated devpts via path lookup "H. Peter Anvin" <hpa@zytor.com> - 2016-04-10 02:20 +0200
                      Re: [PATCH 01/13] devpts: Teach /dev/ptmx to find the associated  devpts via path lookup Linus Torvalds <torvalds@linux-foundation.org> - 2016-04-10 02:20 +0200
                        Re: [PATCH 01/13] devpts: Teach /dev/ptmx to find the associated  devpts via path lookup Andy Lutomirski <luto@amacapital.net> - 2016-04-10 02:50 +0200
                          Re: [PATCH 01/13] devpts: Teach /dev/ptmx to find the associated devpts via path lookup "H. Peter Anvin" <hpa@zytor.com> - 2016-04-11 17:10 +0200
                            Re: [PATCH 01/13] devpts: Teach /dev/ptmx to find the associated  devpts via path lookup Al Viro <viro@ZenIV.linux.org.uk> - 2016-04-12 03:40 +0200
                          Re: [PATCH 01/13] devpts: Teach /dev/ptmx to find the associated  devpts via path lookup Andy Lutomirski <luto@amacapital.net> - 2016-04-11 22:20 +0200
                            Re: [PATCH 01/13] devpts: Teach /dev/ptmx to find the associated devpts via path lookup "H. Peter Anvin" <hpa@zytor.com> - 2016-04-11 22:30 +0200
                              Re: [PATCH 01/13] devpts: Teach /dev/ptmx to find the associated devpts via path lookup ebiederm@xmission.com (Eric W. Biederman) - 2016-04-12 01:50 +0200
                                Re: [PATCH 01/13] devpts: Teach /dev/ptmx to find the associated  devpts via path lookup Linus Torvalds <torvalds@linux-foundation.org> - 2016-04-12 02:10 +0200
                                  Re: [PATCH 01/13] devpts: Teach /dev/ptmx to find the associated devpts via path lookup ebiederm@xmission.com (Eric W. Biederman) - 2016-04-12 02:30 +0200
                                    Re: [PATCH 01/13] devpts: Teach /dev/ptmx to find the associated  devpts via path lookup Linus Torvalds <torvalds@linux-foundation.org> - 2016-04-12 03:20 +0200
                                    Re: [PATCH 01/13] devpts: Teach /dev/ptmx to find the associated devpts via path lookup "H. Peter Anvin" <hpa@zytor.com> - 2016-04-12 03:20 +0200
                                      Re: [PATCH 01/13] devpts: Teach /dev/ptmx to find the associated devpts via path lookup ebiederm@xmission.com (Eric W. Biederman) - 2016-04-12 03:40 +0200
                                        Re: [PATCH 01/13] devpts: Teach /dev/ptmx to find the associated  devpts via path lookup Al Viro <viro@ZenIV.linux.org.uk> - 2016-04-12 03:50 +0200
                                    Re: [PATCH 01/13] devpts: Teach /dev/ptmx to find the associated  devpts via path lookup Al Viro <viro@ZenIV.linux.org.uk> - 2016-04-12 03:40 +0200
                                      Re: [PATCH 01/13] devpts: Teach /dev/ptmx to find the associated devpts via path lookup ebiederm@xmission.com (Eric W. Biederman) - 2016-04-12 04:30 +0200
                            Re: [PATCH 01/13] devpts: Teach /dev/ptmx to find the associated devpts via path lookup ebiederm@xmission.com (Eric W. Biederman) - 2016-04-11 22:30 +0200
                            Re: [PATCH 01/13] devpts: Teach /dev/ptmx to find the associated  devpts via path lookup Andy Lutomirski <luto@amacapital.net> - 2016-04-12 19:50 +0200
                              Re: [PATCH 01/13] devpts: Teach /dev/ptmx to find the associated  devpts via path lookup Linus Torvalds <torvalds@linux-foundation.org> - 2016-04-12 20:20 +0200
                                Re: [PATCH 01/13] devpts: Teach /dev/ptmx to find the associated  devpts via path lookup "H. Peter Anvin" <hpa@zytor.com> - 2016-04-12 21:20 +0200
                                  [PATCH 01/16] devpts: Attempting to get it right ebiederm@xmission.com (Eric W. Biederman) - 2016-04-15 17:50 +0200
                                    [PATCH 08/16] devpts: Stop rolling devpts_remount by hand in devpts_mount "Eric W. Biederman" <ebiederm@xmission.com> - 2016-04-15 17:50 +0200
                                    [PATCH 05/16] vfs: Allow unlink, and rename on expirable file mounts "Eric W. Biederman" <ebiederm@xmission.com> - 2016-04-15 17:50 +0200
                                    [PATCH 04/16] devpts: Teach /dev/ptmx to automount the appropriate devpts via path lookup "Eric W. Biederman" <ebiederm@xmission.com> - 2016-04-15 17:50 +0200
                                    [PATCH 09/16] devpts: Fail early (if appropriate) on overmount "Eric W. Biederman" <ebiederm@xmission.com> - 2016-04-15 17:50 +0200
                                    [PATCH 10/16] devpts: Move parse_mount_options into fill_super "Eric W. Biederman" <ebiederm@xmission.com> - 2016-04-15 17:50 +0200
                                    [PATCH 06/16] devpts: More obvious check for the system devpts in pty allocation "Eric W. Biederman" <ebiederm@xmission.com> - 2016-04-15 17:50 +0200
                                    [PATCH 01/16] devpts: Use the same default mode for both /dev/ptmx and dev/pts/ptmx "Eric W. Biederman" <ebiederm@xmission.com> - 2016-04-15 17:50 +0200
                                      [PATCH 12/16] devpts: Move the creation of /dev/pts/ptmx into fill_super "Eric W. Biederman" <ebiederm@xmission.com> - 2016-04-15 18:00 +0200
                                      [PATCH 11/16] devpts: Make devpts_kill_sb safe if fsi is NULL "Eric W. Biederman" <ebiederm@xmission.com> - 2016-04-15 18:00 +0200
                                      [PATCH 16/16] devpts: Kill the DEVPTS_MULTIPLE_INSTANCE config option "Eric W. Biederman" <ebiederm@xmission.com> - 2016-04-15 18:00 +0200
                                      [PATCH 15/16] devpts: Always return a distinct instance when mounting "Eric W. Biederman" <ebiederm@xmission.com> - 2016-04-15 18:00 +0200
                                      [PATCH 13/16] devpts: Simplify devpts_mount by using mount_nodev "Eric W. Biederman" <ebiederm@xmission.com> - 2016-04-15 18:00 +0200
                                      [PATCH 14/16] vfs: Implement mount_super_once "Eric W. Biederman" <ebiederm@xmission.com> - 2016-04-15 18:00 +0200
                          Re: [PATCH 01/13] devpts: Teach /dev/ptmx to find the associated devpts via path lookup ebiederm@xmission.com (Eric W. Biederman) - 2016-04-12 02:10 +0200
                            Re: [PATCH 01/13] devpts: Teach /dev/ptmx to find the associated  devpts via path lookup Linus Torvalds <torvalds@linux-foundation.org> - 2016-04-12 02:10 +0200
                              Re: [PATCH 01/13] devpts: Teach /dev/ptmx to find the associated devpts via path lookup ebiederm@xmission.com (Eric W. Biederman) - 2016-04-12 02:40 +0200
                                Re: [PATCH 01/13] devpts: Teach /dev/ptmx to find the associated  devpts via path lookup Linus Torvalds <torvalds@linux-foundation.org> - 2016-04-12 03:00 +0200
                    Re: [PATCH 01/13] devpts: Teach /dev/ptmx to find the associated devpts via path lookup ebiederm@xmission.com (Eric W. Biederman) - 2016-04-11 22:20 +0200

Page 2 of 4 — ← Prev page 1 [2] 3 4  Next page →

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

Linus Torvalds <torvalds@linux-foundation.org> writes:

> On Fri, Apr 8, 2016 at 11:51 AM, Eric W. Biederman
> <ebiederm@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.

Anyone can do that with "mount --bind".  All it takes is root in a user
namespace.  I can get root in a user namespace as no one special.

So someone may have set such a thing up, and it may now be possible
to defeat such a regime as anyone.

In practice I suspect all such cases are handled by actually hiding the
mount of devpts in another mount namespace.

> The whole point of the patch is that yes, we change semantics. A
> change of semantics means that people will see situations where the
> behavior is different. But that's not "breaking user space", that's
> just "ok, you can see a difference".

If we don't want to care about this case, and if someone complains about
a security regression readd my permission checks I am fine with that.

But I don't want to let a possibility of breaking someone (that I don't
know how to test for, and would be silent breakage) slip through.

Eric

[toc] | [prev] | [next] | [standalone]

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

On Apr 8, 2016 12:05 PM, "Linus Torvalds" <torvalds@linux-foundation.org> wrote:
>
> On Fri, Apr 8, 2016 at 11:51 AM, Eric W. Biederman
> <ebiederm@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?

>
> The whole point of the patch is that yes, we change semantics. A
> change of semantics means that people will see situations where the
> behavior is different. But that's not "breaking user space", that's
> just "ok, you can see a difference".
>
>                 Linus

[toc] | [prev] | [next] | [standalone]

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

Andy Lutomirski <luto@amacapital.net> writes:

> On Apr 8, 2016 12:05 PM, "Linus Torvalds" <torvalds@linux-foundation.org> wrote:
>>
>> On Fri, Apr 8, 2016 at 11:51 AM, Eric W. Biederman
>> <ebiederm@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

[toc] | [prev] | [next] | [standalone]

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

On Fri, Apr 8, 2016 at 2:29 PM, Eric W. Biederman <ebiederm@xmission.com> wrote:
> Andy Lutomirski <luto@amacapital.net> writes:
>
>> On Apr 8, 2016 12:05 PM, "Linus Torvalds" <torvalds@linux-foundation.org> wrote:
>>>
>>> On Fri, Apr 8, 2016 at 11:51 AM, Eric W. Biederman
>>> <ebiederm@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.
>

Hmm, you're right.  Never mind.

--Andy

[toc] | [prev] | [next] | [standalone]

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

On Fri, Apr 8, 2016 at 2:29 PM, Eric W. Biederman <ebiederm@xmission.com> wrote:
>
> 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.

.. but you could just check that the ptmx node is actually the same
superblock that the pts directory is mounted on. If it's a bind mount,
that wouldn't be true.

But more fundamentally I still don't actually understand why you even
really care.

We get the wrong pts case *today*. We'd get a different wrong pts
namespace when somebody tries to do odd things. Why would we care? It
would be a _better_ guess.

I don't see the security issue. If you do tricks to get pty's in
another group, what's the problem? You have to do it consciously, and
I don't see what the downside is. You get what you ask for, and I
don't see a new attack surface.

The whole "somebody used chmod on /dev/pts/" argument sounds bogus.
That's an insane thing to do. If you want a private namespace, you
make *all* of /dev private, you don't go "oh, I'll just make the pts
subdirectory private".

In other words, your whole scenario sounds totally made up to begin
with. And even if it happens, I don't see what would be so disastrous
about it.

I mean, right now, /dev/ptmx is world read-write in the root container
and everybody gets access to the same underlying set of ptys. And
that's not some horrible security issue. It's how things are
*supposed* to work.

So I really don't see the argument. You guys are just making shit up.

                Linus

[toc] | [prev] | [next] | [standalone]

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

Linus Torvalds <torvalds@linux-foundation.org> writes:

> But more fundamentally I still don't actually understand why you even
> really care.

At this point I care because there is a failure of communication.
Until this email no one has ever said:  "Ok that actually could happen
but we don't actually care."

Right now I am a bit paranoid because I have seen a few too many cases
where some little detail was glossed over and someone clever turned it
into a great big CVE they could drive a truck through.  So I am once
bitten twice shy and all of that.

> We get the wrong pts case *today*. We'd get a different wrong pts
> namespace when somebody tries to do odd things. Why would we care? It
> would be a _better_ guess.
>
> I don't see the security issue. If you do tricks to get pty's in
> another group, what's the problem? You have to do it consciously, and
> I don't see what the downside is. You get what you ask for, and I
> don't see a new attack surface.
>
> The whole "somebody used chmod on /dev/pts/" argument sounds bogus.
> That's an insane thing to do. If you want a private namespace, you
> make *all* of /dev private, you don't go "oh, I'll just make the pts
> subdirectory private".

Oh I pretty much agree it is an insane thing to do.  At the same time I
know that people can make a lot of little sane decisions that can lead
to an insane situation, so just because it is insane I can't rule
it out automatically.

The actual sane thing to do, and what I think most of userspace does
at this point is to create it's own mount namespace so nothing is
visible to outsiders.

> In other words, your whole scenario sounds totally made up to begin
> with. And even if it happens, I don't see what would be so disastrous
> about it.

In general I agree.  The scenario is made up.  I would be surprised if
it happens.

> I mean, right now, /dev/ptmx is world read-write in the root container
> and everybody gets access to the same underlying set of ptys. And
> that's not some horrible security issue. It's how things are
> *supposed* to work.

I agree.

> So I really don't see the argument. You guys are just making shit up.

I don't see why we have the linux extension of supporting anything
except mode 0666 on /dev/ptmx or /dev/pts/ptmx.  This is really about
not breaking that linux extension by overlooking some little detail.

On the attack analysis front the worst thing I can see happening is a
denial of service attack.  I see two possible denial of service attacks.
One possible attack creates a pty and prevents devpts from being
unmounted.  Another possible attack creates all possible ptys on a
devpts instance, and prevents legitimate tty creations from happening.

At the end of the day as you say it would be a pretty crazy person who
isolated a mount of devpts with just the permissions of /dev/pts/ptmx.
So if we don't want to care knowing those stupid attacks above are
possible I am happy not to care.  They don't look all that serious to
me.

Eric

[toc] | [prev] | [next] | [standalone]

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

> If anyone has a better idea on how userspace should connect the master
> pty file descriptor the slave file descriptor, I would be willing to
> implement that instead.

If we are willing to go away from the existing mess of a tty interface
inflicted on us by BSD and then mashed up by POSIX then a syscall of

  int err = ptypair(int fd[2], int perms, int flags);

[where flags is the O_ ones we usually need to cover (CLOEXEC etc) and
maybe even some kind of "private" flag to say don't even expose it via
devpts).

would do remarkably sane things to the majoirty of use cases as it breaks
the dependence on grantpt and also the historic screwup that pty pairs
aren't allocated atomically with both file handles returned as pipe()
does.

Alan

[toc] | [prev] | [next] | [standalone]

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

On April 9, 2016 6:09:09 AM PDT, One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk> wrote:
>
>> If anyone has a better idea on how userspace should connect the
>master
>> pty file descriptor the slave file descriptor, I would be willing to
>> implement that instead.
>
>If we are willing to go away from the existing mess of a tty interface
>inflicted on us by BSD and then mashed up by POSIX then a syscall of
>
>  int err = ptypair(int fd[2], int perms, int flags);
>
>[where flags is the O_ ones we usually need to cover (CLOEXEC etc) and
>maybe even some kind of "private" flag to say don't even expose it via
>devpts).
>
>would do remarkably sane things to the majoirty of use cases as it
>breaks
>the dependence on grantpt and also the historic screwup that pty pairs
>aren't allocated atomically with both file handles returned as pipe()
>does.
>
>Alan

We don't even need to do that if we'd be willing to change the user space interface... if we could rely on the POSIX interface then posix_openpt() could simply open /dev/pts/ptmx and everything would just work.

The trick here is how to make it work in the presence of some extremely bad practices in existing userspace.
-- 
Sent from my Android device with K-9 Mail. Please excuse brevity and formatting.

[toc] | [prev] | [next] | [standalone]

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

"H. Peter Anvin" <hpa@zytor.com> writes:

> On April 9, 2016 6:09:09 AM PDT, One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk> wrote:
>>
>>> If anyone has a better idea on how userspace should connect the
>>master
>>> pty file descriptor the slave file descriptor, I would be willing to
>>> implement that instead.
>>
>>If we are willing to go away from the existing mess of a tty interface
>>inflicted on us by BSD and then mashed up by POSIX then a syscall of
>>
>>  int err = ptypair(int fd[2], int perms, int flags);
>>
>>[where flags is the O_ ones we usually need to cover (CLOEXEC etc) and
>>maybe even some kind of "private" flag to say don't even expose it via
>>devpts).
>>
>>would do remarkably sane things to the majoirty of use cases as it
>>breaks
>>the dependence on grantpt and also the historic screwup that pty pairs
>>aren't allocated atomically with both file handles returned as pipe()
>>does.
>>
>>Alan
>
> We don't even need to do that if we'd be willing to change the user
> space interface... if we could rely on the POSIX interface then
> posix_openpt() could simply open /dev/pts/ptmx and everything would
> just work.

At a quick skim it does look like userspace uses posix_openpt for the
most part.  Certainly portable apps that can run on FreeBSD do.
And just grepping through binaries all of the ones I have checked so far
are calling posix_openpt.

Peter if you or someone could start updating the userspace version of
posix_openpt to use /dev/pts/ptmx when available over /dev/ptmx in
parallel to the kernel work to always allow mount of devpts to give
distinct instances that would be great.

> The trick here is how to make it work in the presence of some
> extremely bad practices in existing userspace.

Yeah.  I am going to look and see if I can move this controversial bit
to a separate patch so we can discuss it more conviniently.

Eric

[toc] | [prev] | [next] | [standalone]

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

On April 9, 2016 7:45:46 AM PDT, ebiederm@xmission.com wrote:
>"H. Peter Anvin" <hpa@zytor.com> writes:
>
>> On April 9, 2016 6:09:09 AM PDT, One Thousand Gnomes
><gnomes@lxorguk.ukuu.org.uk> wrote:
>>>
>>>> If anyone has a better idea on how userspace should connect the
>>>master
>>>> pty file descriptor the slave file descriptor, I would be willing
>to
>>>> implement that instead.
>>>
>>>If we are willing to go away from the existing mess of a tty
>interface
>>>inflicted on us by BSD and then mashed up by POSIX then a syscall of
>>>
>>>  int err = ptypair(int fd[2], int perms, int flags);
>>>
>>>[where flags is the O_ ones we usually need to cover (CLOEXEC etc)
>and
>>>maybe even some kind of "private" flag to say don't even expose it
>via
>>>devpts).
>>>
>>>would do remarkably sane things to the majoirty of use cases as it
>>>breaks
>>>the dependence on grantpt and also the historic screwup that pty
>pairs
>>>aren't allocated atomically with both file handles returned as pipe()
>>>does.
>>>
>>>Alan
>>
>> We don't even need to do that if we'd be willing to change the user
>> space interface... if we could rely on the POSIX interface then
>> posix_openpt() could simply open /dev/pts/ptmx and everything would
>> just work.
>
>At a quick skim it does look like userspace uses posix_openpt for the
>most part.  Certainly portable apps that can run on FreeBSD do.
>And just grepping through binaries all of the ones I have checked so
>far
>are calling posix_openpt.
>
>Peter if you or someone could start updating the userspace version of
>posix_openpt to use /dev/pts/ptmx when available over /dev/ptmx in
>parallel to the kernel work to always allow mount of devpts to give
>distinct instances that would be great.
>
>> The trick here is how to make it work in the presence of some
>> extremely bad practices in existing userspace.
>
>Yeah.  I am going to look and see if I can move this controversial bit
>to a separate patch so we can discuss it more conviniently.
>
>Eric

On the flipside, if we were to allow ourselves to break userspace, at this point I would suggest making /dev/pts/ptmx have a different device number and make the legacy /dev/ptmx print a warning message, after which it can at least eventually be deleted.

This might not be a bad idea anyway.
-- 
Sent from my Android device with K-9 Mail. Please excuse brevity and formatting.

[toc] | [prev] | [next] | [standalone]

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

On Sat, Apr 9, 2016 at 3:37 PM, H. Peter Anvin <hpa@zytor.com> wrote:
>
> On the flipside, if we were to allow ourselves to break userspace, at this point I would suggest making /dev/pts/ptmx have a different device number and make the legacy /dev/ptmx print a warning message, after which it can at least eventually be deleted.

You don't need a different device number.

The /dev/pts/ptmx file may look like it's the same node as /dev/ptmx,
but it is trivial to recognize as the pts one:

     if (dentry->d_sb->s_magic == DEVPTS_SUPER_MAGIC)

and you're done.

But nobody actually uses /dev/pts/ptmx, because it has never had sane
permissions.

So the fact is, /dev/ptmx is what people use, and we're not breaking userspace.

But when we fix bad semantics (and always just looking up the initial
pts mount really is crazy semantics) that doesn't mean that we have to
bend over backwards to not make the changed semantics visible. We
don't _break_ user space, but we also don't care about some random
test-program that checks for particular semantics.

And I can pretty much _guarantee_ that nobody has ever done the "let's
bind-mount a 'ptmx' node in a /dev directory, and then expect that to
bind to some _other_ pts thing than the one in /dev/pts/".

Except as a test-program, or possibly as a "why the f*ck doesn't this
work? Oh, I need to use the single-instance thing because the
multi-instance pts thing is broken. Damn shitty implementation".

                Linus

[toc] | [prev] | [next] | [standalone]

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

On April 9, 2016 5:01:27 PM PDT, Linus Torvalds <torvalds@linux-foundation.org> wrote:
>On Sat, Apr 9, 2016 at 3:37 PM, H. Peter Anvin <hpa@zytor.com> wrote:
>>
>> On the flipside, if we were to allow ourselves to break userspace, at
>this point I would suggest making /dev/pts/ptmx have a different device
>number and make the legacy /dev/ptmx print a warning message, after
>which it can at least eventually be deleted.
>
>You don't need a different device number.
>
>The /dev/pts/ptmx file may look like it's the same node as /dev/ptmx,
>but it is trivial to recognize as the pts one:
>
>     if (dentry->d_sb->s_magic == DEVPTS_SUPER_MAGIC)
>
>and you're done.
>
>But nobody actually uses /dev/pts/ptmx, because it has never had sane
>permissions.
>
>So the fact is, /dev/ptmx is what people use, and we're not breaking
>userspace.
>
>But when we fix bad semantics (and always just looking up the initial
>pts mount really is crazy semantics) that doesn't mean that we have to
>bend over backwards to not make the changed semantics visible. We
>don't _break_ user space, but we also don't care about some random
>test-program that checks for particular semantics.
>
>And I can pretty much _guarantee_ that nobody has ever done the "let's
>bind-mount a 'ptmx' node in a /dev directory, and then expect that to
>bind to some _other_ pts thing than the one in /dev/pts/".
>
>Except as a test-program, or possibly as a "why the f*ck doesn't this
>work? Oh, I need to use the single-instance thing because the
>multi-instance pts thing is broken. Damn shitty implementation".
>
>                Linus

Fixing the default permissions is trivial, of course.  The intent from the beginning was to make a ptmx -> pts/ptmx, but user space never did...
-- 
Sent from my Android device with K-9 Mail. Please excuse brevity and formatting.

[toc] | [prev] | [next] | [standalone]

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

On Sat, Apr 9, 2016 at 5:06 PM, H. Peter Anvin <hpa@zytor.com> wrote:
>
> Fixing the default permissions is trivial, of course.  The intent from the beginning was to make a ptmx -> pts/ptmx, but user space never did...

That wasn't my point.

Because the permissions have never been usable, I pretty much
guarantee that no current user space uses /dev/pts/ptmx.

So that node is almost entirely irrelevant. Us fixing the permissions
at this point isn't going to make it any more relevant, we might as
well ignore it.

Which all means that the way forward really is to just make /dev/ptmx
work. It's not going away, and it _is_ fairly easy to fix.

But I don't think the fix should care about permissions - and we might
as well leave the existing pts/ptmx node with broken permissions.
Because we've never been actually interested in looking up
/dev/pts/ptmx - all we actually care about is to look up which devpts
instance it is.

And that's not about the ptmx node, that's really about the
mount-point. So the right thing to do - conceptually - is *literally*
to just say "ok, what is mounted at 'pts'". Note how at no point do we
want to _open_ anything.

That's why I said that conceptually we could just open /proc/mounts.
Because *that* is really the operation we care about. We don't care
about lookup, and we don't care about permissions on the ptmx node.
Those are completely and utterly irrelevant to what we're actually
after.

So I think the permission thing is not just extra code with more
failure points. I think it's conceptually entirely the wrong thing to
do, and just confuses people into thinking that we're doing something
that we aren't.

                Linus

[toc] | [prev] | [next] | [standalone]

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

On Sat, Apr 9, 2016 at 5:16 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Sat, Apr 9, 2016 at 5:06 PM, H. Peter Anvin <hpa@zytor.com> wrote:
>>
>> Fixing the default permissions is trivial, of course.  The intent from the beginning was to make a ptmx -> pts/ptmx, but user space never did...
>
> That wasn't my point.
>
> Because the permissions have never been usable, I pretty much
> guarantee that no current user space uses /dev/pts/ptmx.
>
> So that node is almost entirely irrelevant. Us fixing the permissions
> at this point isn't going to make it any more relevant, we might as
> well ignore it.
>
> Which all means that the way forward really is to just make /dev/ptmx
> work. It's not going away, and it _is_ fairly easy to fix.
>
> But I don't think the fix should care about permissions - and we might
> as well leave the existing pts/ptmx node with broken permissions.
> Because we've never been actually interested in looking up
> /dev/pts/ptmx - all we actually care about is to look up which devpts
> instance it is.
>
> And that's not about the ptmx node, that's really about the
> mount-point. So the right thing to do - conceptually - is *literally*
> to just say "ok, what is mounted at 'pts'". Note how at no point do we
> want to _open_ anything.
>
> That's why I said that conceptually we could just open /proc/mounts.
> Because *that* is really the operation we care about. We don't care
> about lookup, and we don't care about permissions on the ptmx node.
> Those are completely and utterly irrelevant to what we're actually
> after.
>
> So I think the permission thing is not just extra code with more
> failure points. I think it's conceptually entirely the wrong thing to
> do, and just confuses people into thinking that we're doing something
> that we aren't.

What we *do* want to do, though, is to prevent the following:

Root (or a container manager or whatever) does:

mknod /foo/ptmx c 5 2
chmod 600 /foo/ptmx
chmod 666 /dev/ptmx
mount -t devpts -o newinstance none /foo/pts

Evil user does:

$ unshare -urm
# mount --bind /dev /mnt/foo
# mount --bind /foo/pts /mnt/foo/pts
# open /mnt/foo/ptmx

The issue is that the evil user has the ability to open /mnt/foo/ptmx
(because it's 666), and the relative path 'pts' points to /foo/pts,
which the evil user should *not* be able to access.  IOW, with a naive
implementation, we can match up the ptmx node with the wrong devpts
instance because the evil user unshared their mount namespace and
screwed around.

I don't immediately see how to fix this without playing permission games.

--Andy

[toc] | [prev] | [next] | [standalone]

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

On April 9, 2016 6:27:36 PM PDT, Linus Torvalds <torvalds@linux-foundation.org> wrote:
>On Apr 9, 2016 5:45 PM, "Andy Lutomirski" <luto@amacapital.net> wrote:
>>
>>
>> What we *do* want to do, though, is to prevent the following:
>
>I don't see the point. Why do you bring up this insane scenario that
>nobody
>can possibly care about?
>
>So you actually have any reason to believe somebody does that?
>
>I already asked about that earlier, and the silence was deafening.
>
>    Linus

Here is an entire different approach, I don't know if it is sane or not: when *mounting* the devpts filesystem, it could automagically create the bins mount for ptmx in the parent of its mount point.  Presumably the would be a mount option to disable that behavior.

Does anyone see an obvious problem with that?
-- 
Sent from my Android device with K-9 Mail. Please excuse brevity and formatting.

[toc] | [prev] | [next] | [standalone]

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

On Mon, Apr 11, 2016 at 07:48:28AM -0700, H. Peter Anvin wrote:
 
> Here is an entire different approach, I don't know if it is sane or not: when *mounting* the devpts filesystem, it could automagically create the bins mount for ptmx in the parent of its mount point.  Presumably the would be a mount option to disable that behavior.
> 
> Does anyone see an obvious problem with that?

Yes.  ->mount() doesn't (and fucking *shouldn't*) know anything about the
mountpoint to be.  Not to mention that the same superblock can easily end
up being visible in many places, etc.  This is insane.

[toc] | [prev] | [next] | [standalone]

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

On Sat, Apr 9, 2016 at 6:27 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Apr 9, 2016 5:45 PM, "Andy Lutomirski" <luto@amacapital.net> wrote:
>>
>>
>> What we *do* want to do, though, is to prevent the following:
>
> I don't see the point. Why do you bring up this insane scenario that nobody
> can possibly care about?
>
> So you actually have any reason to believe somebody does that?
>
> I already asked about that earlier, and the silence was deafening.

I have no idea, but I'm generally uncomfortable with magical things
that bypass normal security policy.

That being said, here's an idea for fixing this, at least in the long
run.  Add a new devpts mount option "no_ptmx_redirect" that turns off
this behavior for the super in question.  That is, opening /dev/ptmx
if "pts/ptmx" points to something with no_ptmx_redirect set will fail.
Distros shipping new kernels could be encouraged to (finally!) make
/dev/ptmx a symlink and set this option.

We just might be able to get away with spelling that option "newinstance".

[toc] | [prev] | [next] | [standalone]

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

On April 11, 2016 1:12:22 PM PDT, Andy Lutomirski <luto@amacapital.net> wrote:
>On Sat, Apr 9, 2016 at 6:27 PM, Linus Torvalds
><torvalds@linux-foundation.org> wrote:
>>
>> On Apr 9, 2016 5:45 PM, "Andy Lutomirski" <luto@amacapital.net>
>wrote:
>>>
>>>
>>> What we *do* want to do, though, is to prevent the following:
>>
>> I don't see the point. Why do you bring up this insane scenario that
>nobody
>> can possibly care about?
>>
>> So you actually have any reason to believe somebody does that?
>>
>> I already asked about that earlier, and the silence was deafening.
>
>I have no idea, but I'm generally uncomfortable with magical things
>that bypass normal security policy.
>
>That being said, here's an idea for fixing this, at least in the long
>run.  Add a new devpts mount option "no_ptmx_redirect" that turns off
>this behavior for the super in question.  That is, opening /dev/ptmx
>if "pts/ptmx" points to something with no_ptmx_redirect set will fail.
>Distros shipping new kernels could be encouraged to (finally!) make
>/dev/ptmx a symlink and set this option.
>
>We just might be able to get away with spelling that option
>"newinstance".

What about the idea of making the bind mount automatic?
-- 
Sent from my Android device with K-9 Mail. Please excuse brevity and formatting.

[toc] | [prev] | [next] | [standalone]

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

"H. Peter Anvin" <hpa@zytor.com> writes:

> On April 11, 2016 1:12:22 PM PDT, Andy Lutomirski <luto@amacapital.net> wrote:
>>On Sat, Apr 9, 2016 at 6:27 PM, Linus Torvalds
>><torvalds@linux-foundation.org> wrote:
>>>
>>> On Apr 9, 2016 5:45 PM, "Andy Lutomirski" <luto@amacapital.net>
>>wrote:
>>>>
>>>>
>>>> What we *do* want to do, though, is to prevent the following:
>>>
>>> I don't see the point. Why do you bring up this insane scenario that
>>nobody
>>> can possibly care about?
>>>
>>> So you actually have any reason to believe somebody does that?
>>>
>>> I already asked about that earlier, and the silence was deafening.
>>
>>I have no idea, but I'm generally uncomfortable with magical things
>>that bypass normal security policy.
>>
>>That being said, here's an idea for fixing this, at least in the long
>>run.  Add a new devpts mount option "no_ptmx_redirect" that turns off
>>this behavior for the super in question.  That is, opening /dev/ptmx
>>if "pts/ptmx" points to something with no_ptmx_redirect set will fail.
>>Distros shipping new kernels could be encouraged to (finally!) make
>>/dev/ptmx a symlink and set this option.
>>
>>We just might be able to get away with spelling that option
>>"newinstance".
>
> What about the idea of making the bind mount automatic?

We could almost do that cleanly by playing with the /dev/ptmx dentry
and implementing a d_automount method.  That still needs the crazy path
based lookup without permission checks.

Unfortunately the filesystem not the device owns the dentry operations.

My practical concern if we worked through the implementation details
would be how would it interact with people who bind mount /dev/pts/ptmx
on top of /dev/ptmx.  We might get some strange new errors.

Eric

[toc] | [prev] | [next] | [standalone]

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

On Mon, Apr 11, 2016 at 4:37 PM, Eric W. Biederman
<ebiederm@xmission.com> wrote:
>
> My practical concern if we worked through the implementation details
> would be how would it interact with people who bind mount /dev/pts/ptmx
> on top of /dev/ptmx.  We might get some strange new errors.

Yes, please don't let's play "clever" games. The semantics should be
fairly straightforward.

I still don't understand why people think that you shouldn't be able
to access a 'pts' subsystem that is accessible to others. If you can
bind-mount the pts directory somewhere, then you can damn well already
see that pts mount, claiming that somehow it should be sacred ground
and you shouldn't be able to access it with a ptmx node outside of it
is just insane.

So people have been bringing that up as an issue, but nobody has ever
actually been able to articulate why anybody should ever care.

Now people are just making up random odd semantics. Nobody has ever
explained why the _simple_ "ptmx binds to the pts directory next to
it" is actually problem. Even for a bind mount, you have to be able to
open the point you're mounting, so we know that the "attacker" already
had access to the pts subdirectory.

If somebody wants to keep the pts mount private, they should damn well
keep it _private_. I don't understand peoples "oh, you can access it
but you can't access it".excuses.

              Linus

[toc] | [prev] | [next] | [standalone]

Page 2 of 4 — ← Prev page 1 [2] 3 4  Next page →

Back to top | Article view | linux.kernel

csiph-web