This is the mail archive of the glibc-bugs@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]

[Bug libc/22145] ttyname() gives up too early in the face of namespaces


https://sourceware.org/bugzilla/show_bug.cgi?id=22145

--- Comment #26 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "GNU C Library master sources".

The branch, release/2.26/master has been updated
       via  069c3dd05abc91fced6e1e119e425c361ad97644 (commit)
       via  89e75d5eda37bf513d5f219a571d7b39b26277c3 (commit)
       via  4f987759d1108f5be622d7511d984d31fbc05178 (commit)
       via  f43ead291c501dcfd9381e99cacb118b33344b04 (commit)
       via  bd81a9d1e99f04d0b664542ddec2d270789f7ec1 (commit)
       via  1f0ba053ed6d065b2261f49d77bdcf4a34255689 (commit)
       via  58a63062a04c05a0e0633e0cdbf315a57cc88b5d (commit)
      from  5da0de4de5e8b9576475432b94fa0a486fd9389f (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=069c3dd05abc91fced6e1e119e425c361ad97644

commit 069c3dd05abc91fced6e1e119e425c361ad97644
Author: Florian Weimer <fweimer@redhat.com>
Date:   Sat Nov 18 14:34:46 2017 +0100

    tst-ttyname: Fix namespace setup for Fedora

    On Fedora, the previous initialization sequence did not work and
    resulted in failures like:

    info:  entering chroot 1
    info:    testcase: basic smoketest
    info:      ttyname: PASS {name="/dev/pts/5", errno=0}
    info:      ttyname_r: PASS {name="/dev/pts/5", ret=0, errno=0}
    error: ../sysdeps/unix/sysv/linux/tst-ttyname.c:122: write (setroups,
"deny"): Operation not permitted
    info:  entering chroot 2
    error: ../sysdeps/unix/sysv/linux/tst-ttyname.c:122: write (setroups,
"deny"): Operation not permitted
    error: 2 test failures

    Reviewed-by: Christian Brauner <christian.brauner@ubuntu.com>
    (cherry picked from commit 8db7f48cb74670829df037b2d037df3f36b71ecd)

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=89e75d5eda37bf513d5f219a571d7b39b26277c3

commit 89e75d5eda37bf513d5f219a571d7b39b26277c3
Author: Luke Shumaker <lukeshu@parabola.nu>
Date:   Wed Nov 15 20:39:22 2017 +0100

    linux ttyname{_r}: Add tests

    Add a new tst-ttyname test that includes several named sub-testcases.

    This patch is ordered after the patches with the fixes that it tests for
(to
    avoid breaking `git bisect`), but for reference, here's how each relevant
change
    so far affected the testcases in this commit, starting with
    15e9a4f378c8607c2ae1aa465436af4321db0e23:

      |                                 | before  |         | make checks |
don't |
      |                                 | 15e9a4f | 15e9a4f | consistent  |
bail  |
     
|---------------------------------+---------+---------+-------------+-------|
      | basic smoketest                 | PASS    | PASS    | PASS        |
PASS  |
      | no conflict, no match           | PASS[1] | PASS    | PASS        |
PASS  |
      | no conflict, console            | PASS    | FAIL!   | FAIL        |
PASS! |
      | conflict, no match              | FAIL    | PASS!   | PASS        |
PASS  |
      | conflict, console               | FAIL    | FAIL    | FAIL        |
PASS! |
      | with readlink target            | PASS    | PASS    | PASS        |
PASS  |
      | with readlink trap; fallback    | FAIL    | FAIL    | FAIL        |
PASS! |
      | with readlink trap; no fallback | FAIL    | PASS!   | PASS        |
PASS  |
      | with search-path trap           | FAIL    | FAIL    | PASS!       |
PASS  |
     
|---------------------------------+---------+---------+-------------+-------|
      |                                 | 4/9     | 5/9     | 6/9         | 9/9
  |

      [1]: 15e9a4f introduced a semantic that, under certain failure
           conditions, ttyname sets errno=ENODEV, where previously it didn't
           set errno; it's not quite fair to hold "before 15e9a4f" ttyname to
           those new semantics.  This testcase actually fails, but would have
           passed if we tested for the old the semantics.

    Each of the failing tests before 15e9a4f are all essentially the same bug:
that
    it returns a PTY slave with the correct minor device number, but from the
wrong
    devpts filesystem instance.

    15e9a4f sought to fix this, but missed several of the cases that can cause
this
    to happen, and also broke the case where both the erroneous PTY and the
correct
    PTY exist.

    Acked-by: Christian Brauner <christian.brauner@ubuntu.com>
    (cherry picked from commit d9611e308592355718b36fe085b7b61aa52911e5)

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=4f987759d1108f5be622d7511d984d31fbc05178

commit 4f987759d1108f5be622d7511d984d31fbc05178
Author: Luke Shumaker <lukeshu@parabola.nu>
Date:   Wed Nov 15 20:36:44 2017 +0100

    linux ttyname{_r}: Don't bail prematurely [BZ #22145]

    Commit 15e9a4f378c8607c2ae1aa465436af4321db0e23 introduced logic for
ttyname()
    sending back ENODEV to signal that we can't get a name for the TTY because
we
    inherited it from a different mount namespace.

    However, just because we inherited it from a different mount namespace and
it
    isn't available at its original path, doesn't mean that its name is
unknowable;
    we can still try to find it by allowing the normal fall back on iterating
    through devices.

    An example scenario where this happens is with "/dev/console" in
containers.
    It's a common practice among container managers to allocate a PTY
master/slave
    pair in the host's mount namespace (the slave having a path like
"/dev/pty/$X"),
    bind mount the slave to "/dev/console" in the container's mount namespace,
and
    send the slave FD to a process in the container. Inside of the
    container, the slave-end isn't available at its original path
("/dev/pts/$X"),
    since the container mount namespace has a separate devpts instance from the
host
    (that path may or may not exist in the container; if it does exist, it's
not the
     same PTY slave device). Currently ttyname{_r} sees that the file at the
    original "/dev/pts/$X" path doesn't match the FD passed to it, and fails
early
    and gives up, even though if it kept searching it would find the TTY at
    "/dev/console". Fix that; don't have the ENODEV path force an early return
    inhibiting the fall-back search.

    This change is based on the previous patch that adds use of is_mytty in
    getttyname and getttyname_r. Without that change, this effectively reverts
    15e9a4f, which made us disregard the false similarity of file pointed to by
    "/proc/self/fd/$Y", because if it doesn't bail prematurely then that file
    ("/dev/pts/$X") will just come up again anyway in the fall-back search.

    Reviewed-by: Christian Brauner <christian.brauner@ubuntu.com>
    (cherry picked from commit a09dfc19edcbac3f96d5410529b724db0a583879)

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=f43ead291c501dcfd9381e99cacb118b33344b04

commit f43ead291c501dcfd9381e99cacb118b33344b04
Author: Luke Shumaker <lukeshu@parabola.nu>
Date:   Fri Dec 22 15:27:57 2017 +0100

    linux ttyname{_r}: Make tty checks consistent

    In the ttyname and ttyname_r routines on Linux, at several points it needs
to
    check if a given TTY is the TTY we are looking for. It used to be that this
    check was (to see if `maybe` is `mytty`):

           __xstat64(_STAT_VER, maybe_filename, &maybe) == 0
        #ifdef _STATBUF_ST_RDEV
           && S_ISCHR(maybe.st_mode) && maybe.st_rdev == mytty.st_rdev
        #else
           && maybe.st_ino == mytty.st_ino && maybe.st_dev == mytty.st_dev
        #endif

    This check appears in several places.

    Then, one of the changes made in commit
15e9a4f378c8607c2ae1aa465436af4321db0e23
    was to change that check to:

           __xstat64(_STAT_VER, maybe_filename, &maybe) == 0
        #ifdef _STATBUF_ST_RDEV
           && S_ISCHR(maybe.st_mode) && maybe.st_rdev == mytty.st_rdev
        #endif
           && maybe.st_ino == mytty.st_ino && maybe.st_dev == mytty.st_dev

    That is, it made the st_ino and st_dev parts of the check happen even if we
have
    the st_rdev member. This is an important change, because the kernel allows
    multiple devpts filesystem instances to be created; a device file in one
devpts
    instance may share the same st_rdev with a file in another devpts instance,
but
    they aren't the same file.

    This check appears twice in each file (ttyname.c and ttyname_r.c), once (in
    ttyname and __ttyname_r) to check if a candidate file found by inspecting
/proc
    is the desired TTY, and once (in getttyname and getttyname_r) to check if a
    candidate file found by searching /dev is the desired TTY. However, 15e9a4f
    only updated the checks for files found via /proc; but the concern about
    collisions between devpts instances is just as valid for files found via
/dev.

    So, update all 4 occurrences the check to be consistent with the version of
the
    check introduced in 15e9a4f. Make it easy to keep all 4 occurrences of the
    check consistent by pulling it in to a static inline function, is_mytty.

    Reviewed-by: Christian Brauner <christian.brauner@ubuntu.com>
    (cherry picked from commit 2fbce9c2031e70b6bd67876accfc34b0ec492878)

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=bd81a9d1e99f04d0b664542ddec2d270789f7ec1

commit bd81a9d1e99f04d0b664542ddec2d270789f7ec1
Author: Luke Shumaker <lukeshu@parabola.nu>
Date:   Wed Nov 15 20:33:11 2017 +0100

    linux ttyname: Change return type of is_pty from int to bool

    is_pty returning a bool is fine since there's no possible outcome other
than
    true or false, and bool is used throughout the codebase.

    Reviewed-by: Christian Brauner <christian.brauner@ubuntu.com>
    (cherry picked from commit d10d6cab168ffa26ef6a506655ee5dc8537c8ed7)

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=1f0ba053ed6d065b2261f49d77bdcf4a34255689

commit 1f0ba053ed6d065b2261f49d77bdcf4a34255689
Author: Luke Shumaker <lukeshu@parabola.nu>
Date:   Wed Nov 15 20:31:32 2017 +0100

    linux ttyname: Update a reference to kernel docs for kernel 4.10

    Linux 4.10 moved many of the documentation files around.

    4.10 came out between the time the patch adding the comment (commit
    15e9a4f378c8607c2ae1aa465436af4321db0e23) was submitted and the time
    it was applied (in February, January, and March 2017; respectively).

    Reviewed-by: Christian Brauner <christian.brauner@ubuntu.com>
    (cherry picked from commit 9b5a87502d048905c383b65c51768f4a1db8c685)

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=58a63062a04c05a0e0633e0cdbf315a57cc88b5d

commit 58a63062a04c05a0e0633e0cdbf315a57cc88b5d
Author: Luke Shumaker <lukeshu@parabola.nu>
Date:   Wed Nov 15 20:28:40 2017 +0100

    manual: Update to mention ENODEV for ttyname and ttyname_r

    Commit 15e9a4f378c8607c2ae1aa465436af4321db0e23 introduced ENODEV as a
possible
    error condition for ttyname and ttyname_r. Update the manual to mention
this GNU
    extension.

    Reviewed-by: Christian Brauner <christian.brauner@ubuntu.com>
    (cherry picked from commit 495a56fdeb05d20a88304ff5da577d23a8e81ae1)

-----------------------------------------------------------------------

Summary of changes:
 ChangeLog                             |   44 +++
 NEWS                                  |    1 +
 manual/terminal.texi                  |    5 +
 sysdeps/unix/sysv/linux/Makefile      |    3 +-
 sysdeps/unix/sysv/linux/tst-ttyname.c |  570 +++++++++++++++++++++++++++++++++
 sysdeps/unix/sysv/linux/ttyname.c     |   63 ++---
 sysdeps/unix/sysv/linux/ttyname.h     |   18 +-
 sysdeps/unix/sysv/linux/ttyname_r.c   |   65 ++---
 8 files changed, 682 insertions(+), 87 deletions(-)
 create mode 100644 sysdeps/unix/sysv/linux/tst-ttyname.c

-- 
You are receiving this mail because:
You are on the CC list for the bug.

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