[Bug libc/22145] ttyname() gives up too early in the face of namespaces
cvs-commit at gcc dot gnu.org
sourceware-bugzilla@sourceware.org
Wed Nov 15 21:27:00 GMT 2017
https://sourceware.org/bugzilla/show_bug.cgi?id=22145
--- Comment #23 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, master has been updated
via d9611e308592355718b36fe085b7b61aa52911e5 (commit)
via a09dfc19edcbac3f96d5410529b724db0a583879 (commit)
via 2fbce9c2031e70b6bd67876accfc34b0ec492878 (commit)
via d10d6cab168ffa26ef6a506655ee5dc8537c8ed7 (commit)
via 9b5a87502d048905c383b65c51768f4a1db8c685 (commit)
via 495a56fdeb05d20a88304ff5da577d23a8e81ae1 (commit)
from 78cde19f622cab74e3953c3d0139d51e1076108e (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=d9611e308592355718b36fe085b7b61aa52911e5
commit d9611e308592355718b36fe085b7b61aa52911e5
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>
https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=a09dfc19edcbac3f96d5410529b724db0a583879
commit a09dfc19edcbac3f96d5410529b724db0a583879
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>
https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=2fbce9c2031e70b6bd67876accfc34b0ec492878
commit 2fbce9c2031e70b6bd67876accfc34b0ec492878
Author: Luke Shumaker <lukeshu@parabola.nu>
Date: Wed Nov 15 20:34:30 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>
https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=d10d6cab168ffa26ef6a506655ee5dc8537c8ed7
commit d10d6cab168ffa26ef6a506655ee5dc8537c8ed7
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>
https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=9b5a87502d048905c383b65c51768f4a1db8c685
commit 9b5a87502d048905c383b65c51768f4a1db8c685
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>
https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=495a56fdeb05d20a88304ff5da577d23a8e81ae1
commit 495a56fdeb05d20a88304ff5da577d23a8e81ae1
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>
-----------------------------------------------------------------------
Summary of changes:
ChangeLog | 26 ++
manual/terminal.texi | 5 +
sysdeps/unix/sysv/linux/Makefile | 3 +-
sysdeps/unix/sysv/linux/tst-ttyname.c | 625 +++++++++++++++++++++++++++++++++
sysdeps/unix/sysv/linux/ttyname.c | 59 ++--
sysdeps/unix/sysv/linux/ttyname.h | 18 +-
sysdeps/unix/sysv/linux/ttyname_r.c | 61 +---
7 files changed, 715 insertions(+), 82 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.
More information about the Glibc-bugs
mailing list