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


On Thu, Mar 15, 2018 at 10:38:07AM -0400, Zack Weinberg wrote:
> On Thu, Mar 15, 2018 at 10:10 AM, Christian Brauner
> <christian.brauner@canonical.com> wrote:
> > On Thu, Mar 15, 2018 at 10:02:56AM -0400, Zack Weinberg wrote:
> >> On Thu, Mar 15, 2018 at 8:06 AM, Christian Brauner
> >> <christian.brauner@ubuntu.com> wrote:
> >> > For a long time now Linux has placed the ptmx character device directly
> >> > under the devpts mount at /dev/pts/ptmx.
> >>
> >> Exactly which kernel version started doing this?
> >
> > The article about the patch is here.
> > https://lwn.net/Articles/689539/
> 
> I'm afraid I don't know how to work out from that article which
> _release version_ of the kernel first provided ptmx inside devpts.
> 
> However, that's dated 2016 and the oldest kernel that glibc still
> supports for runtime use is 3.2, which came out in 2012, so I conclude
> we do need the fallback code, at least.
> 
> >> > It is time to start switching to using /dev/pts/ptmx and use /dev/ptmx as a
> >> > fallback only.
> >>
> >> Application code is entitled to do open ('/dev/ptmx", O_RDWR) itself
> >> rather than calling posix_openpt.  It is not OK to break those
> >> applications.  That was the recommended practice prior to the
> >> introduction of posix_openpt, and I am suspicious of posix_openpt not
> >> existing on still-reasonable portability targets.
> >>
> >> Since /dev/ptmx must stick around for the sake of those applications,
> >> I am inclined to say that libc's posix_openpt should continue using
> >> /dev/ptmx as well, in order to ensure that that configuration
> >> continues to be tested.  I am also inclined to say that, on new
> >> kernels where the devpts filesystem provides the ptmx node, using a
> >> bind-mount rather than a symlink for /dev/ptmx is a misconfiguration
> >> (and on older kernels, obviously it needs to be an actual device
> >> node).
> >
> > Neither the kernel nor this patch breaks userspace applications. Maybe
> > I'm being dense but what argument supports this assumption?
> > This patch extends __posix_openpt() to try and open(/dev/pts/ptmx) first
> > and if it fails for any reason retry with open(/dev/ptmx).
> 
> Sorry, I was unclear.
> 
> The scenario I want to avoid is, five to ten years in the future,
> someone - perhaps a sloppy container constructor - thinks that
> /dev/ptmx can be dropped.  This is not OK, even that far ahead,
> because it might break applications.  In order to prevent that from
> happening, I think glibc should continue to use only /dev/ptmx, so
> that the breakage will be immediate and obviously the fault of the
> sloppy container constructor.

By that logic adding superseeding features should never be supported
because of the possibility that someone might *accidently* drop support
for the legacy version. That doesn't make sense.

Dropping /dev/ptmx will not happen and is not intended. If that's the
core of the concern we can put a comment in there statig explictly
"Never ever remove the fallback as we support old kernels that do not
have /dev/pts/ptmx."

Furthermore, such fallbacks exist in multiple locations in glibc
already. One recent example is that glibc already supports the
TIOCGPTPEER ioctl() call to allocate the slave side file descriptor of a
pty solely on the master fd and uses unsafe path-based retrieval only as
a fallback option. Nothing in that patch implies that path-based
retrieveal will ever be removed. Neither does anything in this patch
imply that /dev/ptmx will.

Christian

> 
> I have no objection to your _kernel_ patch, I just think it's silly to
> use a bind-mount for /dev/ptmx when a symlink will work just as well
> (in fact, better).
> 
> zw


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