This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] getpt: use /dev/pts/ptmx as default ptmx master


Christian Brauner <christian.brauner@canonical.com> writes:

> On Thu, Mar 15, 2018 at 03:02:14PM -0500, Eric W. Biederman wrote:
>> Christian Brauner <christian.brauner@ubuntu.com> writes:
>> 
>> > For a long time now Linux has placed the ptmx character device directly
>> > under the devpts mount at /dev/pts/ptmx. The /dev/ptmx path today is
>> > usually either a symlink, an additional character device or a bind-mount.
>> >
>> > The idea has always been to slowly fade-out /dev/ptmx and switch to using
>> > /dev/pts/ptmx exclusively. The kernel currently maintains code to retain
>> > backwards compatibility for anyone going through /dev/ptmx.
>> 
>> It depends on who you talk to Linus is against the slow fade-out.
>> 
>> > Specifically, if the ptmx device is opened through /dev/ptmx the kernel
>> > will look for a "pts" directory in the same directory where the /dev/ptmx
>> > device node resides. This implies that the devpts mount at /dev/pts and the
>> > /dev/ptmx mount need to have a common ancestor directory. This assumption
>> > is usually fulfilled when a symlink or separate device node is used.
>> > However, this assumption will be broken when /dev/ptmx is a bind-mount of
>> > /dev/pts/ptmx because they are located on different devices. For a detailed
>> > analysis of this problem please refer to my upstream patch [1].
>> 
>> We just finished merging the patches that causes this to be a problem
>> for TIOCGTPEER.
>> 
>> > It is time to start switching to using /dev/pts/ptmx and use /dev/ptmx as a
>> > fallback only. As far as I can tell, we have three cases to reason
>> > about:
>> 
>> If we had always had /dev/pts/ptmx there definitely would have been
>> advantages.  What advantages does it have now to switch to opening
>> /dev/pts/ptmx?  Especially given that the default permissions are 0000,
>> so opening /dev/pts/ptmx will fail on most ordinary configurations
>> today.  Only in containers is /dev/pts/ptmx the prefered device to open.
>> 
>> There is a strong argument for using TIOCGTPEER (this fixes possible
>> races).  There is a strong argument for having each mount of devpts
>> be a distinct filesystem (this fixes mount options from a chroot b0rking
>> the main system).
>> 
>> I don't see a similarly strong argument for asking glibc to always open
>> /dev/pts/ptmx if available.  It would simplify things, but we have
>> already dealt with the issues.  So I don't see any real world issues
>> that it fixes.
>
> Thanks for stopping by Eric, if you think it's not worth it I'm not
> going to pursue this further.
>
> My main reason for pushing the /dev/pts/ptmx first, /dev/ptmx second was
> that all bugs we ever had that caused /proc/<pid>/fd/<nr> symlinks to be
> messed were caused by going through /dev/ptmx causing the kernel to go
> through "needless" lookup logic in case the devpts filesystem couldn't
> immediately be found where the ptmx device was.

If we sell this on we are much less likely to encounter kernel bugs,
because the code paths are fundamentally simpler.  Then I will agree.
But let's keep it clear that is why this patch is being prompted.

The counter case is that by glibc and everything else always using
/dev/ptmx we get a large test base for that code path.

It would make my life easier if everyone always used /dev/pts/ptmx
or posix_granpt.  Because in truth what it took to fix /dev/ptmx
and TIOCGPTPERR in the kernel is not something many people the
expertise to double check.

Using /dev/pts/ptmx when it has the proper permissions is probably
even a slightly faster.  But I don't think it is anyone's bottleneck
performance wise.

Pro: Fewer bugs.  Cons: Slightly less backwards compatible.
Personally *Shrug*.

If the fewer bugs and less chance of a security issue with the interface
because of that holds sway.  Please merge this.  Otherwise don't worry
about it.

Eric


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]