tst-ttyname: skip the test when /dev/ptmx is not available

  * sysdeps/unix/sysv/linux/tst-ttyname.c (do_in_chroot_1): Skip the
test instead of failing in case of an error returned by posix_openpt.
---
 ChangeLog                             | 5 +++++
 sysdeps/unix/sysv/linux/tst-ttyname.c | 4 +++-
 2 files changed, 8 insertions(+), 1 deletion(-)
  

* Dmitry V. Levin:

> * sysdeps/unix/sysv/linux/tst-ttyname.c (do_in_chroot_1): Skip the
> test instead of failing in case of an error returned by posix_openpt.

I think the test failure is real in this case.  I wouldn't it be?
  

On Mon, Dec 25, 2017 at 07:19:44PM +0100, Florian Weimer wrote:
> * Dmitry V. Levin:
> 
> > * sysdeps/unix/sysv/linux/tst-ttyname.c (do_in_chroot_1): Skip the
> > test instead of failing in case of an error returned by posix_openpt.
> 
> I think the test failure is real in this case.  I wouldn't it be?

No, /dev/ptmx is intentionally missing in the environment where this test
failed.  Do you suggest an explicit check for ENOENT?
  

On Mon, Dec 25, 2017 at 11:12 AM, Dmitry V. Levin <ldv@altlinux.org> wrote:
> On Mon, Dec 25, 2017 at 07:19:44PM +0100, Florian Weimer wrote:
>> * Dmitry V. Levin:
>>
>> > * sysdeps/unix/sysv/linux/tst-ttyname.c (do_in_chroot_1): Skip the
>> > test instead of failing in case of an error returned by posix_openpt.
>>
>> I think the test failure is real in this case.  I wouldn't it be?
>
> No, /dev/ptmx is intentionally missing in the environment where this test
> failed.

Why?

> Do you suggest an explicit check for ENOENT?

If this is absolutely necessary for some reason, then it should be as
narrow as possible, so yes, that would be my suggestion.

zw
  

On Mon, Dec 25, 2017 at 12:09:56PM -0800, Zack Weinberg wrote:
> On Mon, Dec 25, 2017 at 11:12 AM, Dmitry V. Levin <ldv@altlinux.org> wrote:
> > On Mon, Dec 25, 2017 at 07:19:44PM +0100, Florian Weimer wrote:
> >> * Dmitry V. Levin:
> >>
> >> > * sysdeps/unix/sysv/linux/tst-ttyname.c (do_in_chroot_1): Skip the
> >> > test instead of failing in case of an error returned by posix_openpt.
> >>
> >> I think the test failure is real in this case.  I wouldn't it be?
> >
> > No, /dev/ptmx is intentionally missing in the environment where this test
> > failed.
> 
> Why?

It's a restricted environment.

> > Do you suggest an explicit check for ENOENT?
> 
> If this is absolutely necessary for some reason, then it should be as
> narrow as possible, so yes, that would be my suggestion.

OK
  

On Mon, Dec 25, 2017 at 1:00 PM, Dmitry V. Levin <ldv@altlinux.org> wrote:
> On Mon, Dec 25, 2017 at 12:09:56PM -0800, Zack Weinberg wrote:
>> On Mon, Dec 25, 2017 at 11:12 AM, Dmitry V. Levin <ldv@altlinux.org> wrote:
>> > On Mon, Dec 25, 2017 at 07:19:44PM +0100, Florian Weimer wrote:
>> >> * Dmitry V. Levin:
>> >>
>> >> > * sysdeps/unix/sysv/linux/tst-ttyname.c (do_in_chroot_1): Skip the
>> >> > test instead of failing in case of an error returned by posix_openpt.
>> >>
>> >> I think the test failure is real in this case.  I wouldn't it be?
>> >
>> > No, /dev/ptmx is intentionally missing in the environment where this test
>> > failed.
>>
>> Why?
>
> It's a restricted environment.

Why is lack of access to pseudoterminals a useful restriction to
apply?  What does it defend against?

zw
  

On Mon, Dec 25, 2017 at 01:18:56PM -0800, Zack Weinberg wrote:
> On Mon, Dec 25, 2017 at 1:00 PM, Dmitry V. Levin <ldv@altlinux.org> wrote:
> > On Mon, Dec 25, 2017 at 12:09:56PM -0800, Zack Weinberg wrote:
> >> On Mon, Dec 25, 2017 at 11:12 AM, Dmitry V. Levin <ldv@altlinux.org> wrote:
> >> > On Mon, Dec 25, 2017 at 07:19:44PM +0100, Florian Weimer wrote:
> >> >> * Dmitry V. Levin:
> >> >>
> >> >> > * sysdeps/unix/sysv/linux/tst-ttyname.c (do_in_chroot_1): Skip the
> >> >> > test instead of failing in case of an error returned by posix_openpt.
> >> >>
> >> >> I think the test failure is real in this case.  I wouldn't it be?
> >> >
> >> > No, /dev/ptmx is intentionally missing in the environment where this test
> >> > failed.
> >>
> >> Why?
> >
> > It's a restricted environment.
> 
> Why is lack of access to pseudoterminals a useful restriction to
> apply?  What does it defend against?

The weakest point of that restricted environment is linux kernel,
so unused functionality is disabled to narrow the attack surface.
  

* Dmitry V. Levin:

> On Mon, Dec 25, 2017 at 12:09:56PM -0800, Zack Weinberg wrote:
>> On Mon, Dec 25, 2017 at 11:12 AM, Dmitry V. Levin <ldv@altlinux.org> wrote:
>> > On Mon, Dec 25, 2017 at 07:19:44PM +0100, Florian Weimer wrote:
>> >> * Dmitry V. Levin:
>> >>
>> >> > * sysdeps/unix/sysv/linux/tst-ttyname.c (do_in_chroot_1): Skip the
>> >> > test instead of failing in case of an error returned by posix_openpt.
>> >>
>> >> I think the test failure is real in this case.  I wouldn't it be?
>> >
>> > No, /dev/ptmx is intentionally missing in the environment where this test
>> > failed.
>> 
>> Why?
>
> It's a restricted environment.

I don't think the glibc test suite is supposed to pass in such an
environment.  If you don't provide /dev/null, /sys, or /proc to the
tests, some of them will fail as well.  I still think that the current
test accurately reflects the inadequacy of your test environment.
  

On Tue, Dec 26, 2017 at 02:07:45PM +0100, Florian Weimer wrote:
> * Dmitry V. Levin:
> > On Mon, Dec 25, 2017 at 12:09:56PM -0800, Zack Weinberg wrote:
> >> On Mon, Dec 25, 2017 at 11:12 AM, Dmitry V. Levin <ldv@altlinux.org> wrote:
> >> > On Mon, Dec 25, 2017 at 07:19:44PM +0100, Florian Weimer wrote:
> >> >> * Dmitry V. Levin:
> >> >>
> >> >> > * sysdeps/unix/sysv/linux/tst-ttyname.c (do_in_chroot_1): Skip the
> >> >> > test instead of failing in case of an error returned by posix_openpt.
> >> >>
> >> >> I think the test failure is real in this case.  I wouldn't it be?
> >> >
> >> > No, /dev/ptmx is intentionally missing in the environment where this test
> >> > failed.
> >> 
> >> Why?
> >
> > It's a restricted environment.
> 
> I don't think the glibc test suite is supposed to pass in such an
> environment.

It used to work perfectly for at least 15 years, and it still works
when tst-ttyname is fixed.

> If you don't provide /dev/null, /sys, or /proc to the
> tests, some of them will fail as well.

This is irrelevant to /dev/ptmx availability.

> I still think that the current
> test accurately reflects the inadequacy of your test environment.

I do not agree.

btw, as support_become_root is also unavailable in restricted
environments, an alternative fix for tst-ttyname is to skip the test
when support_become_root fails.
  

On Tue, Dec 26, 2017 at 04:53:30PM +0300, Dmitry V. Levin wrote:
> On Tue, Dec 26, 2017 at 02:07:45PM +0100, Florian Weimer wrote:
> > * Dmitry V. Levin:
> > > On Mon, Dec 25, 2017 at 12:09:56PM -0800, Zack Weinberg wrote:
> > >> On Mon, Dec 25, 2017 at 11:12 AM, Dmitry V. Levin <ldv@altlinux.org> wrote:
> > >> > On Mon, Dec 25, 2017 at 07:19:44PM +0100, Florian Weimer wrote:
> > >> >> * Dmitry V. Levin:
> > >> >>
> > >> >> > * sysdeps/unix/sysv/linux/tst-ttyname.c (do_in_chroot_1): Skip the
> > >> >> > test instead of failing in case of an error returned by posix_openpt.
> > >> >>
> > >> >> I think the test failure is real in this case.  I wouldn't it be?
> > >> >
> > >> > No, /dev/ptmx is intentionally missing in the environment where this test
> > >> > failed.
> > >> 
> > >> Why?
> > >
> > > It's a restricted environment.
> > 
> > I don't think the glibc test suite is supposed to pass in such an
> > environment.
> 
> It used to work perfectly for at least 15 years, and it still works
> when tst-ttyname is fixed.

As tst-ttyname was backported to 2.26 stable branch recently, it
introduced this test suite regression there, too, so a fix to tst-ttyname
should also be backported.

[...]
> btw, as support_become_root is also unavailable in restricted
> environments, an alternative fix for tst-ttyname is to skip the test
> when support_become_root fails.

If /dev/ptmx is available but support_become_root is unable to become
root, tst-ttyname correctly recognizes this setup as unsupported.
It would be quite logical for tst-ttyname to return EXIT_UNSUPPORTED
as soon as support_become_root failed.
  

* Dmitry V. Levin:

>> I don't think the glibc test suite is supposed to pass in such an
>> environment.
>
> It used to work perfectly for at least 15 years, and it still works
> when tst-ttyname is fixed.

This just reflects that we did not have sufficient test coverage for
the PTY code.

> btw, as support_become_root is also unavailable in restricted
> environments, an alternative fix for tst-ttyname is to skip the test
> when support_become_root fails.

But that's unrelated.  And in some environments, support_become_root
can fail, but the process was initially sufficiently privileged to run
the test.
  

On Fri, Dec 29, 2017 at 02:55:17PM +0100, Florian Weimer wrote:
> * Dmitry V. Levin:
> 
> >> I don't think the glibc test suite is supposed to pass in such an
> >> environment.
> >
> > It used to work perfectly for at least 15 years, and it still works
> > when tst-ttyname is fixed.
> 
> This just reflects that we did not have sufficient test coverage for
> the PTY code.

No doubts about that, but the kernel side of pty interface does not have
to be implemented, so we cannot allow the test to fail just because the
kernel does not provide it.

> > btw, as support_become_root is also unavailable in restricted
> > environments, an alternative fix for tst-ttyname is to skip the test
> > when support_become_root fails.
> 
> But that's unrelated.  And in some environments, support_become_root
> can fail, but the process was initially sufficiently privileged to run
> the test.

I don't think such environments exist in practice but this is
theoretically possible.
  

On Wed, 27 Dec 2017, Dmitry V. Levin wrote:

> > > > It's a restricted environment.
> > > 
> > > I don't think the glibc test suite is supposed to pass in such an
> > > environment.
> > 
> > It used to work perfectly for at least 15 years, and it still works
> > when tst-ttyname is fixed.
> 
> As tst-ttyname was backported to 2.26 stable branch recently, it
> introduced this test suite regression there, too, so a fix to tst-ttyname
> should also be backported.

I'm more concerned that we still have the failure on Ubuntu 16.04 noted in 
<https://sourceware.org/ml/libc-alpha/2017-11/msg00832.html>, than about 
failures in a particular restricted environment.  Generally, if there are 
known unresolved issues around a patch on master, that's evidence it would 
be premature to backport it to a stable branch.  (Note: I haven't actually 
verified if the failure on Ubuntu 16.04 is now present on the release 
branch.)
  

On Sun, 31 Dec 2017 20:30:16 -0500,
Joseph Myers wrote:
> On Wed, 27 Dec 2017, Dmitry V. Levin wrote:
> 
> > > > > It's a restricted environment.
> > > > 
> > > > I don't think the glibc test suite is supposed to pass in such an
> > > > environment.
> > > 
> > > It used to work perfectly for at least 15 years, and it still works
> > > when tst-ttyname is fixed.
> > 
> > As tst-ttyname was backported to 2.26 stable branch recently, it
> > introduced this test suite regression there, too, so a fix to tst-ttyname
> > should also be backported.
> 
> I'm more concerned that we still have the failure on Ubuntu 16.04 noted in 
> <https://sourceware.org/ml/libc-alpha/2017-11/msg00832.html>, than about 
> failures in a particular restricted environment.  Generally, if there are 
> known unresolved issues around a patch on master, that's evidence it would 
> be premature to backport it to a stable branch.  (Note: I haven't actually 
> verified if the failure on Ubuntu 16.04 is now present on the release 
> branch.)

Yikes!  I didn't notice that reply.  I've just downloaded the Ubuntu
16.04.3 ISO and will look in to that in the next few days.