* [PATCH v3 2/6] linux ttyname: Update a reference to kernel docs for kernel 4.10
2017-11-08 18:01 [PATCH v3 0/6] Fixup linux ttyname and ttyname_r [BZ #22145] Luke Shumaker
@ 2017-11-08 18:01 ` Luke Shumaker
2017-11-08 18:01 ` [PATCH v3 1/6] manual: Update to mention ENODEV for ttyname and ttyname_r Luke Shumaker
` (5 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: Luke Shumaker @ 2017-11-08 18:01 UTC (permalink / raw)
To: libc-alpha; +Cc: christian.brauner
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).
v2:
- No change
v3:
- Revise commit message
- Two spaces after a period in comments
---
ChangeLog | 2 ++
sysdeps/unix/sysv/linux/ttyname.h | 3 ++-
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/ChangeLog b/ChangeLog
index dc4094bdc3..095baf9eb6 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,7 @@
2017-11-07 Luke Shumaker <lukeshu@parabola.nu>
+ * sysdeps/unix/sysv/linux/ttyname.h (is_pty): Update doc reference.
+
* manual/terminal.texi (Is It a Terminal):
Mention ENODEV for ttyname and ttyname_r.
diff --git a/sysdeps/unix/sysv/linux/ttyname.h b/sysdeps/unix/sysv/linux/ttyname.h
index 2e415e4e9c..cbcdbab607 100644
--- a/sysdeps/unix/sysv/linux/ttyname.h
+++ b/sysdeps/unix/sysv/linux/ttyname.h
@@ -21,7 +21,8 @@
#include <sys/stat.h>
/* Return true if this is a UNIX98 pty device, as defined in
- linux/Documentation/devices.txt. */
+ linux/Documentation/devices.txt (on linux < 4.10) or
+ linux/Documentation/admin-guide/devices.txt (on linux >= 4.10). */
static inline int
is_pty (struct stat64 *sb)
{
--
2.15.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3 3/6] linux ttyname: Change return type of is_pty from int to bool
2017-11-08 18:01 [PATCH v3 0/6] Fixup linux ttyname and ttyname_r [BZ #22145] Luke Shumaker
2017-11-08 18:01 ` [PATCH v3 2/6] linux ttyname: Update a reference to kernel docs for kernel 4.10 Luke Shumaker
2017-11-08 18:01 ` [PATCH v3 1/6] manual: Update to mention ENODEV for ttyname and ttyname_r Luke Shumaker
@ 2017-11-08 18:01 ` Luke Shumaker
2017-11-08 18:01 ` [PATCH v3 6/6] linux ttyname and ttyname_r: Add tests Luke Shumaker
` (3 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: Luke Shumaker @ 2017-11-08 18:01 UTC (permalink / raw)
To: libc-alpha; +Cc: christian.brauner
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.
v2:
- Introduce as part of another commit
v3:
- Split into a separate commit
---
ChangeLog | 3 +++
sysdeps/unix/sysv/linux/ttyname.h | 2 +-
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/ChangeLog b/ChangeLog
index 095baf9eb6..11709a01ac 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,8 @@
2017-11-07 Luke Shumaker <lukeshu@parabola.nu>
+ * sysdeps/unix/sysv/linux/ttyname.h (is_pty): Change return type from
+ int to bool.
+
* sysdeps/unix/sysv/linux/ttyname.h (is_pty): Update doc reference.
* manual/terminal.texi (Is It a Terminal):
diff --git a/sysdeps/unix/sysv/linux/ttyname.h b/sysdeps/unix/sysv/linux/ttyname.h
index cbcdbab607..ce4845a587 100644
--- a/sysdeps/unix/sysv/linux/ttyname.h
+++ b/sysdeps/unix/sysv/linux/ttyname.h
@@ -23,7 +23,7 @@
/* Return true if this is a UNIX98 pty device, as defined in
linux/Documentation/devices.txt (on linux < 4.10) or
linux/Documentation/admin-guide/devices.txt (on linux >= 4.10). */
-static inline int
+static inline bool
is_pty (struct stat64 *sb)
{
#ifdef _STATBUF_ST_RDEV
--
2.15.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3 1/6] manual: Update to mention ENODEV for ttyname and ttyname_r
2017-11-08 18:01 [PATCH v3 0/6] Fixup linux ttyname and ttyname_r [BZ #22145] Luke Shumaker
2017-11-08 18:01 ` [PATCH v3 2/6] linux ttyname: Update a reference to kernel docs for kernel 4.10 Luke Shumaker
@ 2017-11-08 18:01 ` Luke Shumaker
2017-11-08 18:01 ` [PATCH v3 3/6] linux ttyname: Change return type of is_pty from int to bool Luke Shumaker
` (4 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: Luke Shumaker @ 2017-11-08 18:01 UTC (permalink / raw)
To: libc-alpha; +Cc: christian.brauner
Commit 15e9a4f378c8607c2ae1aa465436af4321db0e23 introduced ENODEV as a
possible error condition for ttyname and ttyname_r. Update the manual
to mention this GNU extension.
v2:
- Fix typo: psuedo->pseudo
- Improve wording
- Fix ChangeLog item name
v3:
- Revise commit message
---
ChangeLog | 5 +++++
manual/terminal.texi | 5 +++++
2 files changed, 10 insertions(+)
diff --git a/ChangeLog b/ChangeLog
index bc8c473f09..dc4094bdc3 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2017-11-07 Luke Shumaker <lukeshu@parabola.nu>
+
+ * manual/terminal.texi (Is It a Terminal):
+ Mention ENODEV for ttyname and ttyname_r.
+
2017-11-07 Joseph Myers <joseph@codesourcery.com>
* include/float.h
diff --git a/manual/terminal.texi b/manual/terminal.texi
index 4fef5045b8..4aace48b14 100644
--- a/manual/terminal.texi
+++ b/manual/terminal.texi
@@ -109,6 +109,11 @@ The @var{filedes} is not associated with a terminal.
@item ERANGE
The buffer length @var{len} is too small to store the string to be
returned.
+
+@item ENODEV
+The @var{filedes} is associated with a terminal device that is a slave
+pseudo-terminal, but the file name associated with that device could
+not be determined. This is a GNU extension.
@end table
@end deftypefun
--
2.15.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3 6/6] linux ttyname and ttyname_r: Add tests
2017-11-08 18:01 [PATCH v3 0/6] Fixup linux ttyname and ttyname_r [BZ #22145] Luke Shumaker
` (2 preceding siblings ...)
2017-11-08 18:01 ` [PATCH v3 3/6] linux ttyname: Change return type of is_pty from int to bool Luke Shumaker
@ 2017-11-08 18:01 ` Luke Shumaker
2017-11-08 18:09 ` [PATCH v3 4/6] linux ttyname and ttyname_r: Make the TTY equivalence checks consistent Luke Shumaker
` (2 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: Luke Shumaker @ 2017-11-08 18:01 UTC (permalink / raw)
To: libc-alpha; +Cc: christian.brauner
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.
v2:
- Rearrange commit order
- Fix code style
- Expand commit message
- Pull xtouch, become-root, and chroot setup into functions
- Better tracking of test failures
- Fix the "with search-path trap" test case
- Add test cases for the readlink target directly
- More readable diagnostic output
- Test with both shared and private procfs
v3:
- Revise commit message
- Fix style of block comments
---
ChangeLog | 4 +
sysdeps/unix/sysv/linux/Makefile | 3 +-
sysdeps/unix/sysv/linux/tst-ttyname.c | 582 ++++++++++++++++++++++++++++++++++
3 files changed, 588 insertions(+), 1 deletion(-)
create mode 100644 sysdeps/unix/sysv/linux/tst-ttyname.c
diff --git a/ChangeLog b/ChangeLog
index 9ea727499c..f677ef4d72 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,9 @@
2017-11-07 Luke Shumaker <lukeshu@parabola.nu>
+ [BZ #22145]
+ * sysdeps/unix/sysv/linux/tst-ttyname.c: New file.
+ * sysdeps/unix/sysv/linux/Makefile: Add tst-ttyname to tests.
+
[BZ #22145]
* sysdeps/unix/sysv/linux/ttyname.c (ttyname):
Defer is_pty check until end of the function.
diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile
index bf76b8773d..c6675b3aa5 100644
--- a/sysdeps/unix/sysv/linux/Makefile
+++ b/sysdeps/unix/sysv/linux/Makefile
@@ -43,7 +43,8 @@ sysdep_headers += sys/mount.h sys/acct.h sys/sysctl.h \
bits/siginfo-arch.h bits/siginfo-consts-arch.h
tests += tst-clone tst-clone2 tst-clone3 tst-fanotify tst-personality \
- tst-quota tst-sync_file_range test-errno-linux tst-sysconf-iov_max
+ tst-quota tst-sync_file_range tst-sysconf-iov_max tst-ttyname \
+ test-errno-linux
# Generate the list of SYS_* macros for the system calls (__NR_*
# macros). The file syscall-names.list contains all possible system
diff --git a/sysdeps/unix/sysv/linux/tst-ttyname.c b/sysdeps/unix/sysv/linux/tst-ttyname.c
new file mode 100644
index 0000000000..93baa6fc08
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/tst-ttyname.c
@@ -0,0 +1,582 @@
+/* Copyright (C) 2017 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public License as
+ published by the Free Software Foundation; either version 2.1 of the
+ License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; see the file COPYING.LIB. If
+ not, see <http://www.gnu.org/licenses/>. */
+
+#include <dirent.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <limits.h>
+#include <sched.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/mount.h>
+#include <sys/prctl.h>
+#include <sys/stat.h>
+#include <sys/wait.h>
+#include <unistd.h>
+
+#include <support/check.h>
+#include <support/namespace.h>
+#include <support/support.h>
+#include <support/temp_file.h>
+#include <support/test-driver.h>
+#include <support/xunistd.h>
+
+/* generic utilities */
+
+#define VERIFY(expr) \
+ do { \
+ if (!(expr)) \
+ { \
+ printf ("error: %s:%d: %s: %m\n", \
+ __FILE__, __LINE__, #expr); \
+ exit (1); \
+ } \
+ } while (0)
+
+static void
+xtouch (const char *path, mode_t mode)
+{
+ xclose (xopen (path, O_WRONLY|O_CREAT|O_NOCTTY, mode));
+}
+
+static size_t
+trim_prefix (char *str, size_t str_len, const char *prefix)
+{
+ size_t prefix_len = strlen (prefix);
+ if (str_len > prefix_len && memcmp (str, prefix, prefix_len) == 0)
+ {
+ memmove (str, str + prefix_len, str_len - prefix_len);
+ return str_len - prefix_len;
+ }
+ return str_len;
+}
+
+/* returns a pointer to static storage */
+static char *
+xreadlink (const char *linkname)
+{
+ static char target[PATH_MAX+1];
+ ssize_t target_len = readlink (linkname, target, PATH_MAX);
+ VERIFY (target_len > 0);
+ target_len = trim_prefix (target, target_len, "(unreachable)");
+ target[target_len] = '\0';
+ return target;
+}
+
+static void
+become_root_in_mount_ns (void) {
+ uid_t orig_uid = getuid ();
+ gid_t orig_gid = getgid ();
+
+ support_become_root ();
+
+ if (unshare (CLONE_NEWNS) < 0)
+ FAIL_UNSUPPORTED ("could not enter new mount namespace");
+
+ /* support_become_root might have put us in a new user namespace;
+ most filesystems (including tmpfs) don't allow file or directory
+ creation from a user namespace unless uid and gid maps are set,
+ even if we have root privileges in the namespace (failing with
+ EOVERFLOW, since the uid overflows the empty (0-length) uid map).
+
+ Also, stat always reports that uid and gid maps are empty, so we
+ have to try actually reading from them to check if they are
+ empty. */
+ int fd;
+
+ if ((fd = open ("/proc/self/uid_map", O_RDWR, 0)) >= 0)
+ {
+ char buf;
+ if (read (fd, &buf, 1) == 0)
+ {
+ char *str = xasprintf ("0 %ld 1\n", (long)orig_uid);
+ if (write (fd, str, strlen (str)) < 0)
+ FAIL_EXIT1 ("write (uid_map, \"%s\"): %m", str);
+ free (str);
+ }
+ xclose (fd);
+ }
+
+ /* Oh, but we can't set the gid map until we turn off setgroups. */
+ if ((fd = open ("/proc/self/setgroups", O_WRONLY, 0)) >= 0)
+ {
+ const char *str = "deny";
+ if (write (fd, str, strlen (str)) < 0)
+ FAIL_EXIT1 ("write (setroups, \"%s\"): %m", str);
+ xclose (fd);
+ }
+
+ if ((fd = open ("/proc/self/gid_map", O_RDWR, 0)) >= 0)
+ {
+ char buf;
+ if (read (fd, &buf, 1) == 0)
+ {
+ char *str = xasprintf ("0 %ld 1\n", (long)orig_gid);
+ if (write (fd, str, strlen (str)) < 0)
+ FAIL_EXIT1 ("write (gid_map, \"%s\"): %m", str);
+ free (str);
+ }
+ xclose (fd);
+ }
+}
+
+/* plain ttyname runner */
+
+struct result {
+ const char *name;
+ int err;
+};
+
+/* strings in result structure are in static storage */
+static struct result
+run_ttyname (int fd)
+{
+ struct result ret;
+ errno = 0;
+ ret.name = ttyname (fd);
+ ret.err = errno;
+ return ret;
+}
+
+static bool
+eq_ttyname (struct result actual, struct result expected)
+{
+ if ((actual.err == expected.err) &&
+ (!actual.name == !expected.name) &&
+ (actual.name ? strcmp (actual.name, expected.name) == 0 : true))
+ {
+ char *name = expected.name
+ ? xasprintf ("\"%s\"", expected.name)
+ : strdup ("NULL");
+ printf ("info: ttyname: PASS {name=%s, errno=%d}\n",
+ name, expected.err);
+ free (name);
+ return true;
+ }
+
+ char *actual_name = actual.name
+ ? xasprintf ("\"%s\"", actual.name)
+ : strdup ("NULL");
+ char *expected_name = expected.name
+ ? xasprintf ("\"%s\"", expected.name)
+ : strdup ("NULL");
+ printf ("error: ttyname: actual {name=%s, errno=%d} != expected {name=%s, errno=%d}\n",
+ actual_name, actual.err,
+ expected_name, expected.err);
+ free (actual_name);
+ free (expected_name);
+ return false;
+}
+
+/* ttyname_r runner */
+
+struct result_r {
+ const char *name;
+ int ret;
+ int err;
+};
+
+/* strings in result structure are in static storage */
+static struct result_r
+run_ttyname_r (int fd)
+{
+ static char buf[TTY_NAME_MAX];
+
+ struct result_r ret;
+ errno = 0;
+ ret.ret = ttyname_r (fd, buf, TTY_NAME_MAX);
+ ret.err = errno;
+ if (ret.ret == 0)
+ ret.name = buf;
+ else
+ ret.name = NULL;
+ return ret;
+}
+
+static bool
+eq_ttyname_r (struct result_r actual, struct result_r expected)
+{
+ if ((actual.err == expected.err) &&
+ (actual.ret == expected.ret) &&
+ (!actual.name == !expected.name) &&
+ (actual.name ? strcmp (actual.name, expected.name) == 0 : true))
+ {
+ char *name = expected.name
+ ? xasprintf ("\"%s\"", expected.name)
+ : strdup ("NULL");
+ printf ("info: ttyname_r: PASS {name=%s, ret=%d, errno=%d}\n",
+ name, expected.ret, expected.err);
+ free (name);
+ return true;
+ }
+
+ char *actual_name = actual.name
+ ? xasprintf ("\"%s\"", actual.name)
+ : strdup ("NULL");
+ char *expected_name = expected.name
+ ? xasprintf ("\"%s\"", expected.name)
+ : strdup ("NULL");
+ printf ("error: ttyname_r: actual {name=%s, ret=%d, errno=%d} != expected {name=%s, ret=%d, errno=%d}\n",
+ actual_name, actual.ret, actual.err,
+ expected_name, expected.ret, expected.err);
+ free (actual_name);
+ free (expected_name);
+ return false;
+}
+
+/* combined runner */
+
+static bool
+doit (int fd, const char *testname, struct result_r expected_r) {
+ struct result expected = {.name=expected_r.name, .err=expected_r.ret};
+ bool ret = true;
+
+ printf ("info: testcase: %s\n", testname);
+
+ ret = eq_ttyname (run_ttyname (fd), expected) && ret;
+ ret = eq_ttyname_r (run_ttyname_r (fd), expected_r) && ret;
+
+ if (!ret)
+ support_record_failure ();
+
+ return ret;
+}
+
+/* chroot setup */
+
+static char *chrootdir;
+
+static void
+prepare (int argc, char **argv)
+{
+ chrootdir = xasprintf ("%s/tst-ttyname-XXXXXX", test_dir);
+ if (mkdtemp (chrootdir) == NULL)
+ FAIL_EXIT1 ("mkdtemp (\"%s\"): %m", chrootdir);
+ add_temp_file (chrootdir);
+}
+#define PREPARE prepare
+
+/* These chroot setup functions put the TTY at at "/console" (where it
+ won't be found by ttyname), and create "/dev/console" as an
+ ordinary file. This way, it's easier to write test-cases that
+ expect ttyname to fail; test-cases that expect it to succeed need
+ to explicitly remount it at "/dev/console". */
+
+static int
+do_in_chroot_1 (int (*cb)(const char *, int)) {
+ printf ("info: entering chroot 1\n");
+
+ /* Open the PTS that we'll be testing on. */
+ int master;
+ char *slavename;
+ VERIFY ((master = posix_openpt (O_RDWR|O_NOCTTY|O_NONBLOCK)) >= 0);
+ VERIFY ((slavename = ptsname (master)));
+ VERIFY (unlockpt (master) == 0);
+ if (strncmp (slavename, "/dev/pts/", 9) != 0)
+ FAIL_UNSUPPORTED ("slave pseudo-terminal is not under /dev/pts/: %s",
+ slavename);
+ int slave = xopen (slavename, O_RDWR, 0);
+ if (!doit (slave, "basic smoketest",
+ (struct result_r){.name=slavename, .ret=0, .err=0}))
+ return 1;
+
+ pid_t pid = xfork ();
+ if (pid == 0)
+ {
+ xclose (master);
+
+ become_root_in_mount_ns ();
+
+ VERIFY (mount ("tmpfs", chrootdir, "tmpfs", 0, "mode=755") == 0);
+ VERIFY (chdir (chrootdir) == 0);
+
+ xmkdir ("proc", 0755);
+ xmkdir ("dev", 0755);
+ xmkdir ("dev/pts", 0755);
+
+ VERIFY (mount ("/proc", "proc", NULL, MS_BIND|MS_REC, NULL) == 0);
+ VERIFY (mount ("devpts", "dev/pts", "devpts",
+ MS_NOSUID|MS_NOEXEC,
+ "newinstance,ptmxmode=0666,mode=620") == 0);
+ VERIFY (symlink ("pts/ptmx", "dev/ptmx") == 0);
+
+ xtouch ("console", 0);
+ xtouch ("dev/console", 0);
+ VERIFY (mount (slavename, "console", NULL, MS_BIND, NULL) == 0);
+
+ xchroot (".");
+
+ char *linkname = xasprintf ("/proc/self/fd/%d", slave);
+ char *target = xreadlink (linkname);
+ VERIFY (strcmp (target, slavename) == 0);
+ free (linkname);
+
+ _exit (cb (slavename, slave));
+ }
+ int status;
+ xwaitpid (pid, &status, 0);
+ VERIFY (WIFEXITED (status));
+ xclose (master);
+ xclose (slave);
+ return WEXITSTATUS (status);
+}
+
+static int
+do_in_chroot_2 (int (*cb)(const char *, int)) {
+ printf ("info: entering chroot 2\n");
+
+ int pid_pipe[2];
+ xpipe (pid_pipe);
+ int exit_pipe[2];
+ xpipe (exit_pipe);
+
+ /* Open the PTS that we'll be testing on. */
+ int master;
+ char *slavename;
+ VERIFY ((master = posix_openpt (O_RDWR|O_NOCTTY|O_NONBLOCK)) >= 0);
+ VERIFY ((slavename = ptsname (master)));
+ VERIFY (unlockpt (master) == 0);
+ if (strncmp (slavename, "/dev/pts/", 9) != 0)
+ FAIL_UNSUPPORTED ("slave pseudo-terminal is not under /dev/pts/: %s",
+ slavename);
+ /* wait until in a new mount ns to open the slave */
+
+ /* enable `wait`ing on grandchildren */
+ VERIFY (prctl (PR_SET_CHILD_SUBREAPER, 1) == 0);
+
+ pid_t pid = xfork (); /* outer child */
+ if (pid == 0)
+ {
+ xclose (master);
+ xclose (pid_pipe[0]);
+ xclose (exit_pipe[1]);
+
+ become_root_in_mount_ns ();
+
+ int slave = xopen (slavename, O_RDWR, 0);
+ if (!doit (slave, "basic smoketest",
+ (struct result_r){.name=slavename, .ret=0, .err=0}))
+ _exit (1);
+
+ VERIFY (mount ("tmpfs", chrootdir, "tmpfs", 0, "mode=755") == 0);
+ VERIFY (chdir (chrootdir) == 0);
+
+ xmkdir ("proc", 0755);
+ xmkdir ("dev", 0755);
+ xmkdir ("dev/pts", 0755);
+
+ VERIFY (mount ("devpts", "dev/pts", "devpts",
+ MS_NOSUID|MS_NOEXEC,
+ "newinstance,ptmxmode=0666,mode=620") == 0);
+ VERIFY (symlink ("pts/ptmx", "dev/ptmx") == 0);
+
+ xtouch ("console", 0);
+ xtouch ("dev/console", 0);
+ VERIFY (mount (slavename, "console", NULL, MS_BIND, NULL) == 0);
+
+ xchroot (".");
+
+ if (unshare (CLONE_NEWNS | CLONE_NEWPID) < 0)
+ FAIL_UNSUPPORTED ("could not enter new PID namespace");
+ pid = xfork (); /* inner child */
+ if (pid == 0)
+ {
+ xclose (pid_pipe[1]);
+
+ /* wait until the outer child has exited */
+ char c;
+ VERIFY (read (exit_pipe[0], &c, 1) == 0);
+ xclose (exit_pipe[0]);
+
+ VERIFY (mount ("proc", "/proc", "proc",
+ MS_NOSUID|MS_NOEXEC|MS_NODEV, NULL) == 0);
+
+ char *linkname = xasprintf ("/proc/self/fd/%d", slave);
+ char *target = xreadlink (linkname);
+ VERIFY (strcmp (target, strrchr (slavename, '/')) == 0);
+ free (linkname);
+
+ _exit (cb (slavename, slave));
+ }
+ xwrite (pid_pipe[1], &pid, sizeof pid);
+ _exit (0);
+ }
+ xclose (pid_pipe[1]);
+ xclose (exit_pipe[0]);
+ xclose (exit_pipe[1]);
+
+ /* wait for the outer child */
+ int status;
+ xwaitpid (pid, &status, 0);
+ VERIFY (WIFEXITED (status));
+ int ret = WEXITSTATUS (status);
+ if (ret != 0)
+ return ret;
+
+ /* set 'pid' to the inner child */
+ VERIFY (read (pid_pipe[0], &pid, sizeof pid) == sizeof pid);
+ xclose (pid_pipe[0]);
+
+ /* wait for the inner child */
+ xwaitpid (pid, &status, 0);
+ VERIFY (WIFEXITED (status));
+ xclose (master);
+ return WEXITSTATUS (status);
+}
+
+/* main test */
+
+static int
+run_chroot_tests (const char *slavename, int slave)
+{
+ struct stat st;
+ bool ok = true;
+
+ /* There are 3 groups of tests here. The first group throws a
+ wrench into the works in a generic way, and verifies that ttyname
+ copes correctly. The remaining groups are increasingly
+ convoluted, as we direct that wrench towards specific parts of
+ the routine, and try to break it in specific ways. */
+
+ /* Basic tests that it doesn't get confused by multiple devpts
+ instances. */
+ {
+ VERIFY (stat (slavename, &st) < 0); /* sanity check */
+ ok = doit (slave, "no conflict, no match",
+ (struct result_r){.name=NULL, .ret=ENODEV, .err=ENODEV}) && ok;
+ VERIFY (mount ("/console", "/dev/console", NULL, MS_BIND, NULL) == 0);
+ ok = doit (slave, "no conflict, console",
+ (struct result_r){.name="/dev/console", .ret=0, .err=0}) && ok;
+ VERIFY (umount ("/dev/console") == 0);
+
+ /* keep creating PTYs until we we get a name collision */
+ while (stat (slavename, &st) < 0)
+ posix_openpt (O_RDWR|O_NOCTTY|O_NONBLOCK);
+ VERIFY (stat (slavename, &st) == 0);
+
+ ok = doit (slave, "conflict, no match",
+ (struct result_r){.name=NULL, .ret=ENODEV, .err=ENODEV}) && ok;
+ VERIFY (mount ("/console", "/dev/console", NULL, MS_BIND, NULL) == 0);
+ ok = doit (slave, "conflict, console",
+ (struct result_r){.name="/dev/console", .ret=0, .err=0}) && ok;
+ VERIFY (umount ("/dev/console") == 0);
+ }
+
+ /* The first tests kinda assumed that they hit certain code-paths
+ based on assuming that the readlink target is 'slavename', but
+ that's not quite always true. They're still a good preliminary
+ sanity check, so keep them, but let's add tests that make sure
+ that those code-paths are hit by doing a readlink ourself. */
+ {
+ char *linkname = xasprintf ("/proc/self/fd/%d", slave);
+ char *target = xreadlink (linkname);
+ free (linkname);
+ /* Depeding on how we set up the chroot, the kernel may or may not
+ trim the leading path to the target (it may give us "/6",
+ instead of "/dev/pts/6"). This test group relies on the target
+ existing, so guarantee that (so create a file at "/6" if
+ necessary). */
+ if (stat (target, &st) < 0)
+ {
+ VERIFY (errno == ENOENT);
+ xtouch (target, 0);
+ }
+
+ VERIFY (mount ("/console", "/dev/console", NULL, MS_BIND, NULL) == 0);
+ VERIFY (mount ("/console", target, NULL, MS_BIND, NULL) == 0);
+ ok = doit (slave, "with readlink target",
+ (struct result_r){.name=target, .ret=0, .err=0}) && ok;
+ VERIFY (umount (target) == 0);
+ VERIFY (umount ("/dev/console") == 0);
+
+ VERIFY (mount ("/console", "/dev/console", NULL, MS_BIND, NULL) == 0);
+ VERIFY (mount (slavename, target, NULL, MS_BIND, NULL) == 0);
+ ok = doit (slave, "with readlink trap; fallback",
+ (struct result_r){.name="/dev/console", .ret=0, .err=0}) && ok;
+ VERIFY (umount (target) == 0);
+ VERIFY (umount ("/dev/console") == 0);
+
+ VERIFY (mount (slavename, target, NULL, MS_BIND, NULL) == 0);
+ ok = doit (slave, "with readlink trap; no fallback",
+ (struct result_r){.name=NULL, .ret=ENODEV, .err=ENODEV}) && ok;
+ VERIFY (umount (target) == 0);
+ }
+
+ /* This test is extra-convoluted because we need to control the
+ order of files within the directory. So, just create 3 files,
+ then use opendir/readdir to see what order they are in, and
+ assign meaning based on that order, not by name. This assumes
+ that (on tmpfs) ordering within the directory is stable in the
+ absence of modification, which seems reasonably safe. */
+ {
+ /* since we're testing the fallback search, disable the readlink
+ happy-path */
+ VERIFY (umount2 ("/proc", MNT_DETACH) == 0);
+
+ xtouch ("/dev/console1", 0);
+ xtouch ("/dev/console2", 0);
+ xtouch ("/dev/console3", 0);
+
+ char *c[3];
+ int ci = 0;
+ DIR *dirstream = opendir ("/dev");
+ VERIFY (dirstream != NULL);
+ struct dirent *d;
+ while ((d = readdir (dirstream)) != NULL && ci < 3)
+ {
+ if (strcmp (d->d_name, "console1") &&
+ strcmp (d->d_name, "console2") &&
+ strcmp (d->d_name, "console3") )
+ continue;
+ c[ci++] = xasprintf ("/dev/%s", d->d_name);
+ }
+ VERIFY (ci == 3);
+ VERIFY (closedir (dirstream) == 0);
+
+ VERIFY (mount (slavename, c[0], NULL, MS_BIND, NULL) == 0);
+ VERIFY (mount ("/console", c[1], NULL, MS_BIND, NULL) == 0);
+ VERIFY (mount (slavename, c[2], NULL, MS_BIND, NULL) == 0);
+ VERIFY (umount2 ("/dev/pts", MNT_DETACH) == 0);
+ ok = doit (slave, "with search-path trap",
+ (struct result_r){.name=c[1], .ret=0, .err=0}) && ok;
+ for (int i = 0; i < 3; i++)
+ {
+ VERIFY (umount (c[i]) == 0);
+ VERIFY (unlink (c[i]) == 0);
+ free (c[i]);
+ }
+ }
+
+ return ok ? 0 : 1;
+}
+
+static int
+do_test (void)
+{
+ int ret1 = do_in_chroot_1 (run_chroot_tests);
+ if (ret1 == EXIT_UNSUPPORTED)
+ return ret1;
+
+ int ret2 = do_in_chroot_2 (run_chroot_tests);
+ if (ret2 == EXIT_UNSUPPORTED)
+ return ret2;
+
+ return ret1 | ret2;
+}
+
+#include <support/test-driver.c>
--
2.15.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3 0/6] Fixup linux ttyname and ttyname_r [BZ #22145]
@ 2017-11-08 18:01 Luke Shumaker
2017-11-08 18:01 ` [PATCH v3 2/6] linux ttyname: Update a reference to kernel docs for kernel 4.10 Luke Shumaker
` (6 more replies)
0 siblings, 7 replies; 11+ messages in thread
From: Luke Shumaker @ 2017-11-08 18:01 UTC (permalink / raw)
To: libc-alpha; +Cc: christian.brauner
The theme of this patchset is to fixup the changes made in 15e9a4f.
Fix a bug [BZ #22145] introduced in the commit (and add tests for both
the bug it fixed and the bug it introduced!), update documentation to
reflect the behavior introduced in the commit.
The FSF should have my copyright assignment paperwork on file (though
I never received a confirmation from them that everything was in
order).
v2:
- See individual commit messages
v3:
- Revise commit messages
- Revise ChangeLog messages
- Split one of the commits in to 2 separate commits
- Fix style of comments (no "*" on continued lines, 2 spaces after
trailing period)
- Fix whitespace style in the definition of is_mytty
Luke Shumaker (6):
manual: Update to mention ENODEV for ttyname and ttyname_r
linux ttyname: Update a reference to kernel docs for kernel 4.10
linux ttyname: Change return type of is_pty from int to bool
linux ttyname and ttyname_r: Make the TTY equivalence checks
consistent
linux ttyname and ttyname_r: Don't bail prematurely [BZ #22145]
linux ttyname and ttyname_r: Add tests
ChangeLog | 26 ++
manual/terminal.texi | 5 +
sysdeps/unix/sysv/linux/Makefile | 3 +-
sysdeps/unix/sysv/linux/tst-ttyname.c | 582 ++++++++++++++++++++++++++++++++++
sysdeps/unix/sysv/linux/ttyname.c | 59 ++--
sysdeps/unix/sysv/linux/ttyname.h | 17 +-
sysdeps/unix/sysv/linux/ttyname_r.c | 61 ++--
7 files changed, 671 insertions(+), 82 deletions(-)
create mode 100644 sysdeps/unix/sysv/linux/tst-ttyname.c
--
2.15.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3 5/6] linux ttyname and ttyname_r: Don't bail prematurely [BZ #22145]
2017-11-08 18:01 [PATCH v3 0/6] Fixup linux ttyname and ttyname_r [BZ #22145] Luke Shumaker
` (4 preceding siblings ...)
2017-11-08 18:09 ` [PATCH v3 4/6] linux ttyname and ttyname_r: Make the TTY equivalence checks consistent Luke Shumaker
@ 2017-11-08 18:09 ` Luke Shumaker
2017-11-09 2:18 ` [PATCH v4 1/1] linux ttyname and ttyname_r: Add tests Luke Shumaker
2017-11-10 16:06 ` [PATCH v3 0/6] Fixup linux ttyname and ttyname_r [BZ #22145] Christian Brauner
6 siblings, 1 reply; 11+ messages in thread
From: Luke Shumaker @ 2017-11-08 18:09 UTC (permalink / raw)
To: libc-alpha; +Cc: christian.brauner
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 (including
systemd-nspawn) 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.
v2:
- Rearrange commit order
- Revise commit message
v3:
- Revise commit message
- Revise ChangeLog message
- Fix style of block comments
---
ChangeLog | 5 +++++
sysdeps/unix/sysv/linux/ttyname.c | 19 ++++++++++++-------
sysdeps/unix/sysv/linux/ttyname_r.c | 20 ++++++++++++--------
3 files changed, 29 insertions(+), 15 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index f28c7c1afc..9ea727499c 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,10 @@
2017-11-07 Luke Shumaker <lukeshu@parabola.nu>
+ [BZ #22145]
+ * sysdeps/unix/sysv/linux/ttyname.c (ttyname):
+ Defer is_pty check until end of the function.
+ * sysdeps/unix/sysv/linux/ttyname_r.c (__ttyname_r): Likewise.
+
[BZ #22145]
* sysdeps/unix/sysv/linux/ttyname.h (is_mytty): New function.
* sysdeps/unix/sysv/linux/ttyname.c (getttyname): Call is_mytty.
diff --git a/sysdeps/unix/sysv/linux/ttyname.c b/sysdeps/unix/sysv/linux/ttyname.c
index 6e97d2d455..f4c955f25b 100644
--- a/sysdeps/unix/sysv/linux/ttyname.c
+++ b/sysdeps/unix/sysv/linux/ttyname.c
@@ -115,6 +115,7 @@ ttyname (int fd)
char procname[30];
struct stat64 st, st1;
int dostat = 0;
+ int doispty = 0;
char *name;
int save = errno;
struct termios term;
@@ -165,13 +166,7 @@ ttyname (int fd)
&& is_mytty (&st, &st1))
return ttyname_buf;
- /* If the link doesn't exist, then it points to a device in another
- namespace. */
- if (is_pty (&st))
- {
- __set_errno (ENODEV);
- return NULL;
- }
+ doispty = 1;
}
if (__xstat64 (_STAT_VER, "/dev/pts", &st1) == 0 && S_ISDIR (st1.st_mode))
@@ -195,5 +190,15 @@ ttyname (int fd)
name = getttyname ("/dev", &st, save, &dostat);
}
+ if (!name && doispty && is_pty (&st))
+ {
+ /* We failed to figure out the TTY's name, but we can at least
+ signal that we did verify that it really is a PTY slave.
+ This happens when we have inherited the file descriptor from
+ a different mount namespace. */
+ __set_errno (ENODEV);
+ return NULL;
+ }
+
return name;
}
diff --git a/sysdeps/unix/sysv/linux/ttyname_r.c b/sysdeps/unix/sysv/linux/ttyname_r.c
index 58eb919c3f..00eefc2c5c 100644
--- a/sysdeps/unix/sysv/linux/ttyname_r.c
+++ b/sysdeps/unix/sysv/linux/ttyname_r.c
@@ -95,6 +95,7 @@ __ttyname_r (int fd, char *buf, size_t buflen)
char procname[30];
struct stat64 st, st1;
int dostat = 0;
+ int doispty = 0;
int save = errno;
/* Test for the absolute minimal size. This makes life easier inside
@@ -149,14 +150,7 @@ __ttyname_r (int fd, char *buf, size_t buflen)
&& is_mytty (&st, &st1))
return 0;
- /* If the link doesn't exist, then it points to a device in another
- * namespace.
- */
- if (is_pty (&st))
- {
- __set_errno (ENODEV);
- return ENODEV;
- }
+ doispty = 1;
}
/* Prepare the result buffer. */
@@ -190,6 +184,16 @@ __ttyname_r (int fd, char *buf, size_t buflen)
save, &dostat);
}
+ if (ret && doispty && is_pty (&st))
+ {
+ /* We failed to figure out the TTY's name, but we can at least
+ signal that we did verify that it really is a PTY slave.
+ This happens when we have inherited the file descriptor from
+ a different mount namespace. */
+ __set_errno (ENODEV);
+ return ENODEV;
+ }
+
return ret;
}
--
2.15.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3 4/6] linux ttyname and ttyname_r: Make the TTY equivalence checks consistent
2017-11-08 18:01 [PATCH v3 0/6] Fixup linux ttyname and ttyname_r [BZ #22145] Luke Shumaker
` (3 preceding siblings ...)
2017-11-08 18:01 ` [PATCH v3 6/6] linux ttyname and ttyname_r: Add tests Luke Shumaker
@ 2017-11-08 18:09 ` Luke Shumaker
2017-11-08 18:09 ` [PATCH v3 5/6] linux ttyname and ttyname_r: Don't bail prematurely [BZ #22145] Luke Shumaker
2017-11-10 16:06 ` [PATCH v3 0/6] Fixup linux ttyname and ttyname_r [BZ #22145] Christian Brauner
6 siblings, 0 replies; 11+ messages in thread
From: Luke Shumaker @ 2017-11-08 18:09 UTC (permalink / raw)
To: libc-alpha; +Cc: christian.brauner
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.
v2:
- Rearrange commit order
- Expand commit message
- Reformat is_mytty
- Have is_pty and is_mytty return bool instead of int
- Specify 'const' on pointer arguments as necessary
v3:
- Revise commit message
- Revise ChangeLog message
- Split is_pty change into a separate commit
- Fix whitespace style
---
ChangeLog | 7 +++++++
sysdeps/unix/sysv/linux/ttyname.c | 40 ++++++++----------------------------
sysdeps/unix/sysv/linux/ttyname.h | 12 +++++++++++
sysdeps/unix/sysv/linux/ttyname_r.c | 41 ++++++++-----------------------------
4 files changed, 36 insertions(+), 64 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 11709a01ac..f28c7c1afc 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,12 @@
2017-11-07 Luke Shumaker <lukeshu@parabola.nu>
+ [BZ #22145]
+ * sysdeps/unix/sysv/linux/ttyname.h (is_mytty): New function.
+ * sysdeps/unix/sysv/linux/ttyname.c (getttyname): Call is_mytty.
+ (ttyname): Likewise.
+ * sysdeps/unix/sysv/linux/ttyname_r.c (getttyname_r): Likewise.
+ (__ttyname_r): Likewise.
+
* sysdeps/unix/sysv/linux/ttyname.h (is_pty): Change return type from
int to bool.
diff --git a/sysdeps/unix/sysv/linux/ttyname.c b/sysdeps/unix/sysv/linux/ttyname.c
index 116cf350e5..6e97d2d455 100644
--- a/sysdeps/unix/sysv/linux/ttyname.c
+++ b/sysdeps/unix/sysv/linux/ttyname.c
@@ -35,14 +35,14 @@
char *__ttyname;
#endif
-static char *getttyname (const char *dev, dev_t mydev,
- ino64_t myino, int save, int *dostat);
+static char *getttyname (const char *dev, const struct stat64 *mytty,
+ int save, int *dostat);
libc_freeres_ptr (static char *getttyname_name);
static char *
attribute_compat_text_section
-getttyname (const char *dev, dev_t mydev, ino64_t myino, int save, int *dostat)
+getttyname (const char *dev, const struct stat64 *mytty, int save, int *dostat)
{
static size_t namelen;
struct stat64 st;
@@ -63,7 +63,7 @@ getttyname (const char *dev, dev_t mydev, ino64_t myino, int save, int *dostat)
*((char *) __mempcpy (getttyname_name, dev, devlen - 1)) = '/';
while ((d = __readdir64 (dirstream)) != NULL)
- if ((d->d_fileno == myino || *dostat)
+ if ((d->d_fileno == mytty->st_ino || *dostat)
&& strcmp (d->d_name, "stdin")
&& strcmp (d->d_name, "stdout")
&& strcmp (d->d_name, "stderr"))
@@ -85,12 +85,7 @@ getttyname (const char *dev, dev_t mydev, ino64_t myino, int save, int *dostat)
}
memcpy (&getttyname_name[devlen], d->d_name, dlen);
if (__xstat64 (_STAT_VER, getttyname_name, &st) == 0
-#ifdef _STATBUF_ST_RDEV
- && S_ISCHR (st.st_mode) && st.st_rdev == mydev
-#else
- && d->d_fileno == myino && st.st_dev == mydev
-#endif
- )
+ && is_mytty (mytty, &st))
{
(void) __closedir (dirstream);
#if 0
@@ -167,12 +162,7 @@ ttyname (int fd)
/* Verify readlink result, fall back on iterating through devices. */
if (ttyname_buf[0] == '/'
&& __xstat64 (_STAT_VER, ttyname_buf, &st1) == 0
-#ifdef _STATBUF_ST_RDEV
- && S_ISCHR (st1.st_mode)
- && st1.st_rdev == st.st_rdev
-#endif
- && st1.st_ino == st.st_ino
- && st1.st_dev == st.st_dev)
+ && is_mytty (&st, &st1))
return ttyname_buf;
/* If the link doesn't exist, then it points to a device in another
@@ -186,11 +176,7 @@ ttyname (int fd)
if (__xstat64 (_STAT_VER, "/dev/pts", &st1) == 0 && S_ISDIR (st1.st_mode))
{
-#ifdef _STATBUF_ST_RDEV
- name = getttyname ("/dev/pts", st.st_rdev, st.st_ino, save, &dostat);
-#else
- name = getttyname ("/dev/pts", st.st_dev, st.st_ino, save, &dostat);
-#endif
+ name = getttyname ("/dev/pts", &st, save, &dostat);
}
else
{
@@ -200,21 +186,13 @@ ttyname (int fd)
if (!name && dostat != -1)
{
-#ifdef _STATBUF_ST_RDEV
- name = getttyname ("/dev", st.st_rdev, st.st_ino, save, &dostat);
-#else
- name = getttyname ("/dev", st.st_dev, st.st_ino, save, &dostat);
-#endif
+ name = getttyname ("/dev", &st, save, &dostat);
}
if (!name && dostat != -1)
{
dostat = 1;
-#ifdef _STATBUF_ST_RDEV
- name = getttyname ("/dev", st.st_rdev, st.st_ino, save, &dostat);
-#else
- name = getttyname ("/dev", st.st_dev, st.st_ino, save, &dostat);
-#endif
+ name = getttyname ("/dev", &st, save, &dostat);
}
return name;
diff --git a/sysdeps/unix/sysv/linux/ttyname.h b/sysdeps/unix/sysv/linux/ttyname.h
index ce4845a587..c290bfab36 100644
--- a/sysdeps/unix/sysv/linux/ttyname.h
+++ b/sysdeps/unix/sysv/linux/ttyname.h
@@ -33,3 +33,15 @@ is_pty (struct stat64 *sb)
return false;
#endif
}
+
+static inline bool
+is_mytty (const struct stat64 *mytty, const struct stat64 *maybe)
+{
+ return (maybe->st_ino == mytty->st_ino
+ && maybe->st_dev == mytty->st_dev
+#ifdef _STATBUF_ST_RDEV
+ && S_ISCHR (maybe->st_mode)
+ && maybe->st_rdev == mytty->st_rdev
+#endif
+ );
+}
diff --git a/sysdeps/unix/sysv/linux/ttyname_r.c b/sysdeps/unix/sysv/linux/ttyname_r.c
index 18f35ef2b7..58eb919c3f 100644
--- a/sysdeps/unix/sysv/linux/ttyname_r.c
+++ b/sysdeps/unix/sysv/linux/ttyname_r.c
@@ -31,12 +31,12 @@
#include "ttyname.h"
static int getttyname_r (char *buf, size_t buflen,
- dev_t mydev, ino64_t myino, int save,
+ const struct stat64 *mytty, int save,
int *dostat);
static int
attribute_compat_text_section
-getttyname_r (char *buf, size_t buflen, dev_t mydev, ino64_t myino,
+getttyname_r (char *buf, size_t buflen, const struct stat64 *mytty,
int save, int *dostat)
{
struct stat64 st;
@@ -52,7 +52,7 @@ getttyname_r (char *buf, size_t buflen, dev_t mydev, ino64_t myino,
}
while ((d = __readdir64 (dirstream)) != NULL)
- if ((d->d_fileno == myino || *dostat)
+ if ((d->d_fileno == mytty->st_ino || *dostat)
&& strcmp (d->d_name, "stdin")
&& strcmp (d->d_name, "stdout")
&& strcmp (d->d_name, "stderr"))
@@ -72,12 +72,7 @@ getttyname_r (char *buf, size_t buflen, dev_t mydev, ino64_t myino,
cp[0] = '\0';
if (__xstat64 (_STAT_VER, buf, &st) == 0
-#ifdef _STATBUF_ST_RDEV
- && S_ISCHR (st.st_mode) && st.st_rdev == mydev
-#else
- && d->d_fileno == myino && st.st_dev == mydev
-#endif
- )
+ && is_mytty (mytty, &st))
{
(void) __closedir (dirstream);
__set_errno (save);
@@ -151,12 +146,7 @@ __ttyname_r (int fd, char *buf, size_t buflen)
/* Verify readlink result, fall back on iterating through devices. */
if (buf[0] == '/'
&& __xstat64 (_STAT_VER, buf, &st1) == 0
-#ifdef _STATBUF_ST_RDEV
- && S_ISCHR (st1.st_mode)
- && st1.st_rdev == st.st_rdev
-#endif
- && st1.st_ino == st.st_ino
- && st1.st_dev == st.st_dev)
+ && is_mytty (&st, &st1))
return 0;
/* If the link doesn't exist, then it points to a device in another
@@ -175,13 +165,8 @@ __ttyname_r (int fd, char *buf, size_t buflen)
if (__xstat64 (_STAT_VER, buf, &st1) == 0 && S_ISDIR (st1.st_mode))
{
-#ifdef _STATBUF_ST_RDEV
- ret = getttyname_r (buf, buflen, st.st_rdev, st.st_ino, save,
+ ret = getttyname_r (buf, buflen, &st, save,
&dostat);
-#else
- ret = getttyname_r (buf, buflen, st.st_dev, st.st_ino, save,
- &dostat);
-#endif
}
else
{
@@ -193,26 +178,16 @@ __ttyname_r (int fd, char *buf, size_t buflen)
{
buf[sizeof ("/dev/") - 1] = '\0';
buflen += sizeof ("pts/") - 1;
-#ifdef _STATBUF_ST_RDEV
- ret = getttyname_r (buf, buflen, st.st_rdev, st.st_ino, save,
- &dostat);
-#else
- ret = getttyname_r (buf, buflen, st.st_dev, st.st_ino, save,
+ ret = getttyname_r (buf, buflen, &st, save,
&dostat);
-#endif
}
if (ret && dostat != -1)
{
buf[sizeof ("/dev/") - 1] = '\0';
dostat = 1;
-#ifdef _STATBUF_ST_RDEV
- ret = getttyname_r (buf, buflen, st.st_rdev, st.st_ino,
- save, &dostat);
-#else
- ret = getttyname_r (buf, buflen, st.st_dev, st.st_ino,
+ ret = getttyname_r (buf, buflen, &st,
save, &dostat);
-#endif
}
return ret;
--
2.15.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v4 1/1] linux ttyname and ttyname_r: Add tests
2017-11-08 18:09 ` [PATCH v3 5/6] linux ttyname and ttyname_r: Don't bail prematurely [BZ #22145] Luke Shumaker
@ 2017-11-09 2:18 ` Luke Shumaker
2017-11-09 16:24 ` [PATCH v5 " Luke Shumaker
0 siblings, 1 reply; 11+ messages in thread
From: Luke Shumaker @ 2017-11-09 2:18 UTC (permalink / raw)
To: libc-alpha; +Cc: christian.brauner
From: Luke Shumaker <lukeshu@parabola.nu>
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.
v2:
- Rearrange commit order
- Fix code style
- Expand commit message
- Pull xtouch, become-root, and chroot setup into functions
- Better tracking of test failures
- Fix the "with search-path trap" test case
- Add test cases for the readlink target directly
- More readable diagnostic output
- Test with both shared and private procfs
v3:
- Revise commit message
- Fix style of block comments
v4:
- Fix brace style
- Clarify comments
- Use if/else blocks instead of ternary operators
- Say "if (!...) ok = false;" rather than "ok = ... && ok;"
- Use xstrdup instead of strdup
---
ChangeLog | 4 +
sysdeps/unix/sysv/linux/Makefile | 3 +-
sysdeps/unix/sysv/linux/tst-ttyname.c | 625 ++++++++++++++++++++++++++++++++++
3 files changed, 631 insertions(+), 1 deletion(-)
create mode 100644 sysdeps/unix/sysv/linux/tst-ttyname.c
diff --git a/ChangeLog b/ChangeLog
index 9ea727499c..f677ef4d72 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,9 @@
2017-11-07 Luke Shumaker <lukeshu@parabola.nu>
+ [BZ #22145]
+ * sysdeps/unix/sysv/linux/tst-ttyname.c: New file.
+ * sysdeps/unix/sysv/linux/Makefile: Add tst-ttyname to tests.
+
[BZ #22145]
* sysdeps/unix/sysv/linux/ttyname.c (ttyname):
Defer is_pty check until end of the function.
diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile
index bf76b8773d..c6675b3aa5 100644
--- a/sysdeps/unix/sysv/linux/Makefile
+++ b/sysdeps/unix/sysv/linux/Makefile
@@ -43,7 +43,8 @@ sysdep_headers += sys/mount.h sys/acct.h sys/sysctl.h \
bits/siginfo-arch.h bits/siginfo-consts-arch.h
tests += tst-clone tst-clone2 tst-clone3 tst-fanotify tst-personality \
- tst-quota tst-sync_file_range test-errno-linux tst-sysconf-iov_max
+ tst-quota tst-sync_file_range tst-sysconf-iov_max tst-ttyname \
+ test-errno-linux
# Generate the list of SYS_* macros for the system calls (__NR_*
# macros). The file syscall-names.list contains all possible system
diff --git a/sysdeps/unix/sysv/linux/tst-ttyname.c b/sysdeps/unix/sysv/linux/tst-ttyname.c
new file mode 100644
index 0000000000..f47d81ecc3
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/tst-ttyname.c
@@ -0,0 +1,625 @@
+/* Copyright (C) 2017 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public License as
+ published by the Free Software Foundation; either version 2.1 of the
+ License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; see the file COPYING.LIB. If
+ not, see <http://www.gnu.org/licenses/>. */
+
+#include <dirent.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <limits.h>
+#include <sched.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/mount.h>
+#include <sys/prctl.h>
+#include <sys/stat.h>
+#include <sys/wait.h>
+#include <unistd.h>
+
+#include <support/check.h>
+#include <support/namespace.h>
+#include <support/support.h>
+#include <support/temp_file.h>
+#include <support/test-driver.h>
+#include <support/xunistd.h>
+
+/* generic utilities */
+
+#define VERIFY(expr) \
+ do { \
+ if (!(expr)) \
+ { \
+ printf ("error: %s:%d: %s: %m\n", \
+ __FILE__, __LINE__, #expr); \
+ exit (1); \
+ } \
+ } while (0)
+
+static void
+xtouch (const char *path, mode_t mode)
+{
+ xclose (xopen (path, O_WRONLY|O_CREAT|O_NOCTTY, mode));
+}
+
+static size_t
+trim_prefix (char *str, size_t str_len, const char *prefix)
+{
+ size_t prefix_len = strlen (prefix);
+ if (str_len > prefix_len && memcmp (str, prefix, prefix_len) == 0)
+ {
+ memmove (str, str + prefix_len, str_len - prefix_len);
+ return str_len - prefix_len;
+ }
+ return str_len;
+}
+
+/* returns a pointer to static storage */
+static char *
+xreadlink (const char *linkname)
+{
+ static char target[PATH_MAX+1];
+ ssize_t target_len = readlink (linkname, target, PATH_MAX);
+ VERIFY (target_len > 0);
+ target_len = trim_prefix (target, target_len, "(unreachable)");
+ target[target_len] = '\0';
+ return target;
+}
+
+static void
+become_root_in_mount_ns (void)
+{
+ uid_t orig_uid = getuid ();
+ gid_t orig_gid = getgid ();
+
+ support_become_root ();
+
+ if (unshare (CLONE_NEWNS) < 0)
+ FAIL_UNSUPPORTED ("could not enter new mount namespace");
+
+ /* support_become_root might have put us in a new user namespace;
+ most filesystems (including tmpfs) don't allow file or directory
+ creation from a user namespace unless uid and gid maps are set,
+ even if we have root privileges in the namespace (failing with
+ EOVERFLOW, since the uid overflows the empty (0-length) uid map).
+
+ Also, stat always reports that uid and gid maps are empty, so we
+ have to try actually reading from them to check if they are
+ empty. */
+ int fd;
+
+ if ((fd = open ("/proc/self/uid_map", O_RDWR, 0)) >= 0)
+ {
+ char buf;
+ if (read (fd, &buf, 1) == 0)
+ {
+ char *str = xasprintf ("0 %ld 1\n", (long)orig_uid);
+ if (write (fd, str, strlen (str)) < 0)
+ FAIL_EXIT1 ("write (uid_map, \"%s\"): %m", str);
+ free (str);
+ }
+ xclose (fd);
+ }
+
+ /* Setting the gid map has the additional complexity that we have to
+ first turn off setgroups. */
+ if ((fd = open ("/proc/self/setgroups", O_WRONLY, 0)) >= 0)
+ {
+ const char *str = "deny";
+ if (write (fd, str, strlen (str)) < 0)
+ FAIL_EXIT1 ("write (setroups, \"%s\"): %m", str);
+ xclose (fd);
+ }
+
+ if ((fd = open ("/proc/self/gid_map", O_RDWR, 0)) >= 0)
+ {
+ char buf;
+ if (read (fd, &buf, 1) == 0)
+ {
+ char *str = xasprintf ("0 %ld 1\n", (long)orig_gid);
+ if (write (fd, str, strlen (str)) < 0)
+ FAIL_EXIT1 ("write (gid_map, \"%s\"): %m", str);
+ free (str);
+ }
+ xclose (fd);
+ }
+}
+
+/* plain ttyname runner */
+
+struct result
+{
+ const char *name;
+ int err;
+};
+
+/* strings in result structure are in static storage */
+static struct result
+run_ttyname (int fd)
+{
+ struct result ret;
+ errno = 0;
+ ret.name = ttyname (fd);
+ ret.err = errno;
+ return ret;
+}
+
+static bool
+eq_ttyname (struct result actual, struct result expected)
+{
+ char *actual_name, *expected_name;
+
+ if ((actual.err == expected.err) &&
+ (!actual.name == !expected.name) &&
+ (actual.name ? strcmp (actual.name, expected.name) == 0 : true))
+ {
+ if (expected.name)
+ expected_name = xasprintf ("\"%s\"", expected.name);
+ else
+ expected_name = xstrdup ("NULL");
+
+ printf ("info: ttyname: PASS {name=%s, errno=%d}\n",
+ expected_name, expected.err);
+
+ free (expected_name);
+ return true;
+ }
+
+ if (actual.name)
+ actual_name = xasprintf ("\"%s\"", actual.name);
+ else
+ actual_name = xstrdup ("NULL");
+
+ if (expected.name)
+ expected_name = xasprintf ("\"%s\"", expected.name);
+ else
+ expected_name = xstrdup ("NULL");
+
+ printf ("error: ttyname: actual {name=%s, errno=%d} != expected {name=%s, errno=%d}\n",
+ actual_name, actual.err,
+ expected_name, expected.err);
+
+ free (actual_name);
+ free (expected_name);
+ return false;
+}
+
+/* ttyname_r runner */
+
+struct result_r
+{
+ const char *name;
+ int ret;
+ int err;
+};
+
+/* strings in result structure are in static storage */
+static struct result_r
+run_ttyname_r (int fd)
+{
+ static char buf[TTY_NAME_MAX];
+
+ struct result_r ret;
+ errno = 0;
+ ret.ret = ttyname_r (fd, buf, TTY_NAME_MAX);
+ ret.err = errno;
+ if (ret.ret == 0)
+ ret.name = buf;
+ else
+ ret.name = NULL;
+ return ret;
+}
+
+static bool
+eq_ttyname_r (struct result_r actual, struct result_r expected)
+{
+ char *actual_name, *expected_name;
+
+ if ((actual.err == expected.err) &&
+ (actual.ret == expected.ret) &&
+ (!actual.name == !expected.name) &&
+ (actual.name ? strcmp (actual.name, expected.name) == 0 : true))
+ {
+ if (expected.name)
+ expected_name = xasprintf ("\"%s\"", expected.name);
+ else
+ expected_name = xstrdup ("NULL");
+
+ printf ("info: ttyname_r: PASS {name=%s, ret=%d, errno=%d}\n",
+ expected_name, expected.ret, expected.err);
+
+ free (expected_name);
+ return true;
+ }
+
+ if (actual.name)
+ actual_name = xasprintf ("\"%s\"", actual.name);
+ else
+ actual_name = xstrdup ("NULL");
+
+ if (expected.name)
+ expected_name = xasprintf ("\"%s\"", expected.name);
+ else
+ expected_name = xstrdup ("NULL");
+
+ printf ("error: ttyname_r: actual {name=%s, ret=%d, errno=%d} != expected {name=%s, ret=%d, errno=%d}\n",
+ actual_name, actual.ret, actual.err,
+ expected_name, expected.ret, expected.err);
+
+ free (actual_name);
+ free (expected_name);
+ return false;
+}
+
+/* combined runner */
+
+static bool
+doit (int fd, const char *testname, struct result_r expected_r)
+{
+ struct result expected = {.name=expected_r.name, .err=expected_r.ret};
+ bool ret = true;
+
+ printf ("info: testcase: %s\n", testname);
+
+ if (!eq_ttyname (run_ttyname (fd), expected))
+ ret = false;
+ if (!eq_ttyname_r (run_ttyname_r (fd), expected_r))
+ ret = false;
+
+ if (!ret)
+ support_record_failure ();
+
+ return ret;
+}
+
+/* chroot setup */
+
+static char *chrootdir;
+
+static void
+prepare (int argc, char **argv)
+{
+ chrootdir = xasprintf ("%s/tst-ttyname-XXXXXX", test_dir);
+ if (mkdtemp (chrootdir) == NULL)
+ FAIL_EXIT1 ("mkdtemp (\"%s\"): %m", chrootdir);
+ add_temp_file (chrootdir);
+}
+#define PREPARE prepare
+
+/* These chroot setup functions put the TTY at at "/console" (where it
+ won't be found by ttyname), and create "/dev/console" as an
+ ordinary file. This way, it's easier to write test-cases that
+ expect ttyname to fail; test-cases that expect it to succeed need
+ to explicitly remount it at "/dev/console". */
+
+static int
+do_in_chroot_1 (int (*cb)(const char *, int))
+{
+ printf ("info: entering chroot 1\n");
+
+ /* Open the PTS that we'll be testing on. */
+ int master;
+ char *slavename;
+ VERIFY ((master = posix_openpt (O_RDWR|O_NOCTTY|O_NONBLOCK)) >= 0);
+ VERIFY ((slavename = ptsname (master)));
+ VERIFY (unlockpt (master) == 0);
+ if (strncmp (slavename, "/dev/pts/", 9) != 0)
+ FAIL_UNSUPPORTED ("slave pseudo-terminal is not under /dev/pts/: %s",
+ slavename);
+ int slave = xopen (slavename, O_RDWR, 0);
+ if (!doit (slave, "basic smoketest",
+ (struct result_r){.name=slavename, .ret=0, .err=0}))
+ return 1;
+
+ pid_t pid = xfork ();
+ if (pid == 0)
+ {
+ xclose (master);
+
+ become_root_in_mount_ns ();
+
+ VERIFY (mount ("tmpfs", chrootdir, "tmpfs", 0, "mode=755") == 0);
+ VERIFY (chdir (chrootdir) == 0);
+
+ xmkdir ("proc", 0755);
+ xmkdir ("dev", 0755);
+ xmkdir ("dev/pts", 0755);
+
+ VERIFY (mount ("/proc", "proc", NULL, MS_BIND|MS_REC, NULL) == 0);
+ VERIFY (mount ("devpts", "dev/pts", "devpts",
+ MS_NOSUID|MS_NOEXEC,
+ "newinstance,ptmxmode=0666,mode=620") == 0);
+ VERIFY (symlink ("pts/ptmx", "dev/ptmx") == 0);
+
+ xtouch ("console", 0);
+ xtouch ("dev/console", 0);
+ VERIFY (mount (slavename, "console", NULL, MS_BIND, NULL) == 0);
+
+ xchroot (".");
+
+ char *linkname = xasprintf ("/proc/self/fd/%d", slave);
+ char *target = xreadlink (linkname);
+ VERIFY (strcmp (target, slavename) == 0);
+ free (linkname);
+
+ _exit (cb (slavename, slave));
+ }
+ int status;
+ xwaitpid (pid, &status, 0);
+ VERIFY (WIFEXITED (status));
+ xclose (master);
+ xclose (slave);
+ return WEXITSTATUS (status);
+}
+
+static int
+do_in_chroot_2 (int (*cb)(const char *, int))
+{
+ printf ("info: entering chroot 2\n");
+
+ int pid_pipe[2];
+ xpipe (pid_pipe);
+ int exit_pipe[2];
+ xpipe (exit_pipe);
+
+ /* Open the PTS that we'll be testing on. */
+ int master;
+ char *slavename;
+ VERIFY ((master = posix_openpt (O_RDWR|O_NOCTTY|O_NONBLOCK)) >= 0);
+ VERIFY ((slavename = ptsname (master)));
+ VERIFY (unlockpt (master) == 0);
+ if (strncmp (slavename, "/dev/pts/", 9) != 0)
+ FAIL_UNSUPPORTED ("slave pseudo-terminal is not under /dev/pts/: %s",
+ slavename);
+ /* wait until in a new mount ns to open the slave */
+
+ /* enable `wait`ing on grandchildren */
+ VERIFY (prctl (PR_SET_CHILD_SUBREAPER, 1) == 0);
+
+ pid_t pid = xfork (); /* outer child */
+ if (pid == 0)
+ {
+ xclose (master);
+ xclose (pid_pipe[0]);
+ xclose (exit_pipe[1]);
+
+ become_root_in_mount_ns ();
+
+ int slave = xopen (slavename, O_RDWR, 0);
+ if (!doit (slave, "basic smoketest",
+ (struct result_r){.name=slavename, .ret=0, .err=0}))
+ _exit (1);
+
+ VERIFY (mount ("tmpfs", chrootdir, "tmpfs", 0, "mode=755") == 0);
+ VERIFY (chdir (chrootdir) == 0);
+
+ xmkdir ("proc", 0755);
+ xmkdir ("dev", 0755);
+ xmkdir ("dev/pts", 0755);
+
+ VERIFY (mount ("devpts", "dev/pts", "devpts",
+ MS_NOSUID|MS_NOEXEC,
+ "newinstance,ptmxmode=0666,mode=620") == 0);
+ VERIFY (symlink ("pts/ptmx", "dev/ptmx") == 0);
+
+ xtouch ("console", 0);
+ xtouch ("dev/console", 0);
+ VERIFY (mount (slavename, "console", NULL, MS_BIND, NULL) == 0);
+
+ xchroot (".");
+
+ if (unshare (CLONE_NEWNS | CLONE_NEWPID) < 0)
+ FAIL_UNSUPPORTED ("could not enter new PID namespace");
+ pid = xfork (); /* inner child */
+ if (pid == 0)
+ {
+ xclose (pid_pipe[1]);
+
+ /* wait until the outer child has exited */
+ char c;
+ VERIFY (read (exit_pipe[0], &c, 1) == 0);
+ xclose (exit_pipe[0]);
+
+ VERIFY (mount ("proc", "/proc", "proc",
+ MS_NOSUID|MS_NOEXEC|MS_NODEV, NULL) == 0);
+
+ char *linkname = xasprintf ("/proc/self/fd/%d", slave);
+ char *target = xreadlink (linkname);
+ VERIFY (strcmp (target, strrchr (slavename, '/')) == 0);
+ free (linkname);
+
+ _exit (cb (slavename, slave));
+ }
+ xwrite (pid_pipe[1], &pid, sizeof pid);
+ _exit (0);
+ }
+ xclose (pid_pipe[1]);
+ xclose (exit_pipe[0]);
+ xclose (exit_pipe[1]);
+
+ /* wait for the outer child */
+ int status;
+ xwaitpid (pid, &status, 0);
+ VERIFY (WIFEXITED (status));
+ int ret = WEXITSTATUS (status);
+ if (ret != 0)
+ return ret;
+
+ /* set 'pid' to the inner child */
+ VERIFY (read (pid_pipe[0], &pid, sizeof pid) == sizeof pid);
+ xclose (pid_pipe[0]);
+
+ /* wait for the inner child */
+ xwaitpid (pid, &status, 0);
+ VERIFY (WIFEXITED (status));
+ xclose (master);
+ return WEXITSTATUS (status);
+}
+
+/* main test */
+
+static int
+run_chroot_tests (const char *slavename, int slave)
+{
+ struct stat st;
+ bool ok = true;
+
+ /* There are 3 groups of tests here. The first group throws a
+ wrench into the works in a generic way, and verifies that ttyname
+ copes correctly. The remaining groups are increasingly
+ convoluted, as we direct that wrench towards specific parts of
+ the routine, and try to break it in specific ways. */
+
+ /* Basic tests that it doesn't get confused by multiple devpts
+ instances. */
+ {
+ VERIFY (stat (slavename, &st) < 0); /* sanity check */
+ if (!doit (slave, "no conflict, no match",
+ (struct result_r){.name=NULL, .ret=ENODEV, .err=ENODEV}))
+ ok = false;
+ VERIFY (mount ("/console", "/dev/console", NULL, MS_BIND, NULL) == 0);
+ if (!doit (slave, "no conflict, console",
+ (struct result_r){.name="/dev/console", .ret=0, .err=0}))
+ ok = false;
+ VERIFY (umount ("/dev/console") == 0);
+
+ /* keep creating PTYs until we we get a name collision */
+ while (stat (slavename, &st) < 0)
+ posix_openpt (O_RDWR|O_NOCTTY|O_NONBLOCK);
+ VERIFY (stat (slavename, &st) == 0);
+
+ if (!doit (slave, "conflict, no match",
+ (struct result_r){.name=NULL, .ret=ENODEV, .err=ENODEV}))
+ ok = false;
+ VERIFY (mount ("/console", "/dev/console", NULL, MS_BIND, NULL) == 0);
+ if (!doit (slave, "conflict, console",
+ (struct result_r){.name="/dev/console", .ret=0, .err=0}))
+ ok = false;
+ VERIFY (umount ("/dev/console") == 0);
+ }
+
+ /* The first tests kinda assumed that they hit certain code-paths
+ based on assuming that the readlink target is 'slavename', but
+ that's not quite always true. They're still a good preliminary
+ sanity check, so keep them, but let's add tests that make sure
+ that those code-paths are hit by doing a readlink ourself. */
+ {
+ char *linkname = xasprintf ("/proc/self/fd/%d", slave);
+ char *target = xreadlink (linkname);
+ free (linkname);
+ /* Depeding on how we set up the chroot, the kernel may or may not
+ trim the leading path to the target (it may give us "/6",
+ instead of "/dev/pts/6"). We test it both ways (do_in_chroot_1
+ and do_in_chroot_2). This test group relies on the target
+ existing, so guarantee that it does exist by creating it if
+ necessary. */
+ if (stat (target, &st) < 0)
+ {
+ VERIFY (errno == ENOENT);
+ xtouch (target, 0);
+ }
+
+ VERIFY (mount ("/console", "/dev/console", NULL, MS_BIND, NULL) == 0);
+ VERIFY (mount ("/console", target, NULL, MS_BIND, NULL) == 0);
+ if (!doit (slave, "with readlink target",
+ (struct result_r){.name=target, .ret=0, .err=0}))
+ ok = false;
+ VERIFY (umount (target) == 0);
+ VERIFY (umount ("/dev/console") == 0);
+
+ VERIFY (mount ("/console", "/dev/console", NULL, MS_BIND, NULL) == 0);
+ VERIFY (mount (slavename, target, NULL, MS_BIND, NULL) == 0);
+ if (!doit (slave, "with readlink trap; fallback",
+ (struct result_r){.name="/dev/console", .ret=0, .err=0}))
+ ok = false;
+ VERIFY (umount (target) == 0);
+ VERIFY (umount ("/dev/console") == 0);
+
+ VERIFY (mount (slavename, target, NULL, MS_BIND, NULL) == 0);
+ if (!doit (slave, "with readlink trap; no fallback",
+ (struct result_r){.name=NULL, .ret=ENODEV, .err=ENODEV}))
+ ok = false;
+ VERIFY (umount (target) == 0);
+ }
+
+ /* This test makes sure that everything still works OK if readdir
+ finds a pseudo-match before and/or after the actual match. Now,
+ to do that, we need to control that readdir finds the
+ pseudo-matches before and after the actual match; and there's no
+ good way to control that order in absence of whitebox testing.
+ So, just create 3 files, then use opendir/readdir to see what
+ order they are in, and assign meaning based on that order, not by
+ name; assigning the first to be a pseudo-match, the second to be
+ the actual match, and the third to be a pseudo-match. This
+ assumes that (on tmpfs) ordering within the directory is stable
+ in the absence of modification, which seems reasonably safe. */
+ {
+ /* since we're testing the fallback search, disable the readlink
+ happy-path */
+ VERIFY (umount2 ("/proc", MNT_DETACH) == 0);
+
+ xtouch ("/dev/console1", 0);
+ xtouch ("/dev/console2", 0);
+ xtouch ("/dev/console3", 0);
+
+ char *c[3];
+ int ci = 0;
+ DIR *dirstream = opendir ("/dev");
+ VERIFY (dirstream != NULL);
+ struct dirent *d;
+ while ((d = readdir (dirstream)) != NULL && ci < 3)
+ {
+ if (strcmp (d->d_name, "console1") &&
+ strcmp (d->d_name, "console2") &&
+ strcmp (d->d_name, "console3") )
+ continue;
+ c[ci++] = xasprintf ("/dev/%s", d->d_name);
+ }
+ VERIFY (ci == 3);
+ VERIFY (closedir (dirstream) == 0);
+
+ VERIFY (mount (slavename, c[0], NULL, MS_BIND, NULL) == 0);
+ VERIFY (mount ("/console", c[1], NULL, MS_BIND, NULL) == 0);
+ VERIFY (mount (slavename, c[2], NULL, MS_BIND, NULL) == 0);
+ VERIFY (umount2 ("/dev/pts", MNT_DETACH) == 0);
+ if (!doit (slave, "with search-path trap",
+ (struct result_r){.name=c[1], .ret=0, .err=0}))
+ ok = false;
+ for (int i = 0; i < 3; i++)
+ {
+ VERIFY (umount (c[i]) == 0);
+ VERIFY (unlink (c[i]) == 0);
+ free (c[i]);
+ }
+ }
+
+ return ok ? 0 : 1;
+}
+
+static int
+do_test (void)
+{
+ int ret1 = do_in_chroot_1 (run_chroot_tests);
+ if (ret1 == EXIT_UNSUPPORTED)
+ return ret1;
+
+ int ret2 = do_in_chroot_2 (run_chroot_tests);
+ if (ret2 == EXIT_UNSUPPORTED)
+ return ret2;
+
+ return ret1 | ret2;
+}
+
+#include <support/test-driver.c>
--
2.15.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v5 1/1] linux ttyname and ttyname_r: Add tests
2017-11-09 2:18 ` [PATCH v4 1/1] linux ttyname and ttyname_r: Add tests Luke Shumaker
@ 2017-11-09 16:24 ` Luke Shumaker
0 siblings, 0 replies; 11+ messages in thread
From: Luke Shumaker @ 2017-11-09 16:24 UTC (permalink / raw)
To: libc-alpha; +Cc: christian.brauner
From: Luke Shumaker <lukeshu@parabola.nu>
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.
v2:
- Rearrange commit order
- Fix code style
- Expand commit message
- Pull xtouch, become-root, and chroot setup into functions
- Better tracking of test failures
- Fix the "with search-path trap" test case
- Add test cases for the readlink target directly
- More readable diagnostic output
- Test with both shared and private procfs
v3:
- Revise commit message
- Fix style of block comments
v4:
- Fix brace style
- Clarify comments
- Use if/else blocks instead of ternary operators
- Say "if (!...) ok = false;" rather than "ok = ... && ok;"
- Use xstrdup instead of strdup
v5:
- Avoid English idiom in a comment
---
ChangeLog | 4 +
sysdeps/unix/sysv/linux/Makefile | 3 +-
sysdeps/unix/sysv/linux/tst-ttyname.c | 625 ++++++++++++++++++++++++++++++++++
3 files changed, 631 insertions(+), 1 deletion(-)
create mode 100644 sysdeps/unix/sysv/linux/tst-ttyname.c
diff --git a/ChangeLog b/ChangeLog
index 9ea727499c..f677ef4d72 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,9 @@
2017-11-07 Luke Shumaker <lukeshu@parabola.nu>
+ [BZ #22145]
+ * sysdeps/unix/sysv/linux/tst-ttyname.c: New file.
+ * sysdeps/unix/sysv/linux/Makefile: Add tst-ttyname to tests.
+
[BZ #22145]
* sysdeps/unix/sysv/linux/ttyname.c (ttyname):
Defer is_pty check until end of the function.
diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile
index bf76b8773d..c6675b3aa5 100644
--- a/sysdeps/unix/sysv/linux/Makefile
+++ b/sysdeps/unix/sysv/linux/Makefile
@@ -43,7 +43,8 @@ sysdep_headers += sys/mount.h sys/acct.h sys/sysctl.h \
bits/siginfo-arch.h bits/siginfo-consts-arch.h
tests += tst-clone tst-clone2 tst-clone3 tst-fanotify tst-personality \
- tst-quota tst-sync_file_range test-errno-linux tst-sysconf-iov_max
+ tst-quota tst-sync_file_range tst-sysconf-iov_max tst-ttyname \
+ test-errno-linux
# Generate the list of SYS_* macros for the system calls (__NR_*
# macros). The file syscall-names.list contains all possible system
diff --git a/sysdeps/unix/sysv/linux/tst-ttyname.c b/sysdeps/unix/sysv/linux/tst-ttyname.c
new file mode 100644
index 0000000000..a5a4bb9689
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/tst-ttyname.c
@@ -0,0 +1,625 @@
+/* Copyright (C) 2017 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public License as
+ published by the Free Software Foundation; either version 2.1 of the
+ License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; see the file COPYING.LIB. If
+ not, see <http://www.gnu.org/licenses/>. */
+
+#include <dirent.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <limits.h>
+#include <sched.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/mount.h>
+#include <sys/prctl.h>
+#include <sys/stat.h>
+#include <sys/wait.h>
+#include <unistd.h>
+
+#include <support/check.h>
+#include <support/namespace.h>
+#include <support/support.h>
+#include <support/temp_file.h>
+#include <support/test-driver.h>
+#include <support/xunistd.h>
+
+/* generic utilities */
+
+#define VERIFY(expr) \
+ do { \
+ if (!(expr)) \
+ { \
+ printf ("error: %s:%d: %s: %m\n", \
+ __FILE__, __LINE__, #expr); \
+ exit (1); \
+ } \
+ } while (0)
+
+static void
+xtouch (const char *path, mode_t mode)
+{
+ xclose (xopen (path, O_WRONLY|O_CREAT|O_NOCTTY, mode));
+}
+
+static size_t
+trim_prefix (char *str, size_t str_len, const char *prefix)
+{
+ size_t prefix_len = strlen (prefix);
+ if (str_len > prefix_len && memcmp (str, prefix, prefix_len) == 0)
+ {
+ memmove (str, str + prefix_len, str_len - prefix_len);
+ return str_len - prefix_len;
+ }
+ return str_len;
+}
+
+/* returns a pointer to static storage */
+static char *
+xreadlink (const char *linkname)
+{
+ static char target[PATH_MAX+1];
+ ssize_t target_len = readlink (linkname, target, PATH_MAX);
+ VERIFY (target_len > 0);
+ target_len = trim_prefix (target, target_len, "(unreachable)");
+ target[target_len] = '\0';
+ return target;
+}
+
+static void
+become_root_in_mount_ns (void)
+{
+ uid_t orig_uid = getuid ();
+ gid_t orig_gid = getgid ();
+
+ support_become_root ();
+
+ if (unshare (CLONE_NEWNS) < 0)
+ FAIL_UNSUPPORTED ("could not enter new mount namespace");
+
+ /* support_become_root might have put us in a new user namespace;
+ most filesystems (including tmpfs) don't allow file or directory
+ creation from a user namespace unless uid and gid maps are set,
+ even if we have root privileges in the namespace (failing with
+ EOVERFLOW, since the uid overflows the empty (0-length) uid map).
+
+ Also, stat always reports that uid and gid maps are empty, so we
+ have to try actually reading from them to check if they are
+ empty. */
+ int fd;
+
+ if ((fd = open ("/proc/self/uid_map", O_RDWR, 0)) >= 0)
+ {
+ char buf;
+ if (read (fd, &buf, 1) == 0)
+ {
+ char *str = xasprintf ("0 %ld 1\n", (long)orig_uid);
+ if (write (fd, str, strlen (str)) < 0)
+ FAIL_EXIT1 ("write (uid_map, \"%s\"): %m", str);
+ free (str);
+ }
+ xclose (fd);
+ }
+
+ /* Setting the gid map has the additional complexity that we have to
+ first turn off setgroups. */
+ if ((fd = open ("/proc/self/setgroups", O_WRONLY, 0)) >= 0)
+ {
+ const char *str = "deny";
+ if (write (fd, str, strlen (str)) < 0)
+ FAIL_EXIT1 ("write (setroups, \"%s\"): %m", str);
+ xclose (fd);
+ }
+
+ if ((fd = open ("/proc/self/gid_map", O_RDWR, 0)) >= 0)
+ {
+ char buf;
+ if (read (fd, &buf, 1) == 0)
+ {
+ char *str = xasprintf ("0 %ld 1\n", (long)orig_gid);
+ if (write (fd, str, strlen (str)) < 0)
+ FAIL_EXIT1 ("write (gid_map, \"%s\"): %m", str);
+ free (str);
+ }
+ xclose (fd);
+ }
+}
+
+/* plain ttyname runner */
+
+struct result
+{
+ const char *name;
+ int err;
+};
+
+/* strings in result structure are in static storage */
+static struct result
+run_ttyname (int fd)
+{
+ struct result ret;
+ errno = 0;
+ ret.name = ttyname (fd);
+ ret.err = errno;
+ return ret;
+}
+
+static bool
+eq_ttyname (struct result actual, struct result expected)
+{
+ char *actual_name, *expected_name;
+
+ if ((actual.err == expected.err) &&
+ (!actual.name == !expected.name) &&
+ (actual.name ? strcmp (actual.name, expected.name) == 0 : true))
+ {
+ if (expected.name)
+ expected_name = xasprintf ("\"%s\"", expected.name);
+ else
+ expected_name = xstrdup ("NULL");
+
+ printf ("info: ttyname: PASS {name=%s, errno=%d}\n",
+ expected_name, expected.err);
+
+ free (expected_name);
+ return true;
+ }
+
+ if (actual.name)
+ actual_name = xasprintf ("\"%s\"", actual.name);
+ else
+ actual_name = xstrdup ("NULL");
+
+ if (expected.name)
+ expected_name = xasprintf ("\"%s\"", expected.name);
+ else
+ expected_name = xstrdup ("NULL");
+
+ printf ("error: ttyname: actual {name=%s, errno=%d} != expected {name=%s, errno=%d}\n",
+ actual_name, actual.err,
+ expected_name, expected.err);
+
+ free (actual_name);
+ free (expected_name);
+ return false;
+}
+
+/* ttyname_r runner */
+
+struct result_r
+{
+ const char *name;
+ int ret;
+ int err;
+};
+
+/* strings in result structure are in static storage */
+static struct result_r
+run_ttyname_r (int fd)
+{
+ static char buf[TTY_NAME_MAX];
+
+ struct result_r ret;
+ errno = 0;
+ ret.ret = ttyname_r (fd, buf, TTY_NAME_MAX);
+ ret.err = errno;
+ if (ret.ret == 0)
+ ret.name = buf;
+ else
+ ret.name = NULL;
+ return ret;
+}
+
+static bool
+eq_ttyname_r (struct result_r actual, struct result_r expected)
+{
+ char *actual_name, *expected_name;
+
+ if ((actual.err == expected.err) &&
+ (actual.ret == expected.ret) &&
+ (!actual.name == !expected.name) &&
+ (actual.name ? strcmp (actual.name, expected.name) == 0 : true))
+ {
+ if (expected.name)
+ expected_name = xasprintf ("\"%s\"", expected.name);
+ else
+ expected_name = xstrdup ("NULL");
+
+ printf ("info: ttyname_r: PASS {name=%s, ret=%d, errno=%d}\n",
+ expected_name, expected.ret, expected.err);
+
+ free (expected_name);
+ return true;
+ }
+
+ if (actual.name)
+ actual_name = xasprintf ("\"%s\"", actual.name);
+ else
+ actual_name = xstrdup ("NULL");
+
+ if (expected.name)
+ expected_name = xasprintf ("\"%s\"", expected.name);
+ else
+ expected_name = xstrdup ("NULL");
+
+ printf ("error: ttyname_r: actual {name=%s, ret=%d, errno=%d} != expected {name=%s, ret=%d, errno=%d}\n",
+ actual_name, actual.ret, actual.err,
+ expected_name, expected.ret, expected.err);
+
+ free (actual_name);
+ free (expected_name);
+ return false;
+}
+
+/* combined runner */
+
+static bool
+doit (int fd, const char *testname, struct result_r expected_r)
+{
+ struct result expected = {.name=expected_r.name, .err=expected_r.ret};
+ bool ret = true;
+
+ printf ("info: testcase: %s\n", testname);
+
+ if (!eq_ttyname (run_ttyname (fd), expected))
+ ret = false;
+ if (!eq_ttyname_r (run_ttyname_r (fd), expected_r))
+ ret = false;
+
+ if (!ret)
+ support_record_failure ();
+
+ return ret;
+}
+
+/* chroot setup */
+
+static char *chrootdir;
+
+static void
+prepare (int argc, char **argv)
+{
+ chrootdir = xasprintf ("%s/tst-ttyname-XXXXXX", test_dir);
+ if (mkdtemp (chrootdir) == NULL)
+ FAIL_EXIT1 ("mkdtemp (\"%s\"): %m", chrootdir);
+ add_temp_file (chrootdir);
+}
+#define PREPARE prepare
+
+/* These chroot setup functions put the TTY at at "/console" (where it
+ won't be found by ttyname), and create "/dev/console" as an
+ ordinary file. This way, it's easier to write test-cases that
+ expect ttyname to fail; test-cases that expect it to succeed need
+ to explicitly remount it at "/dev/console". */
+
+static int
+do_in_chroot_1 (int (*cb)(const char *, int))
+{
+ printf ("info: entering chroot 1\n");
+
+ /* Open the PTS that we'll be testing on. */
+ int master;
+ char *slavename;
+ VERIFY ((master = posix_openpt (O_RDWR|O_NOCTTY|O_NONBLOCK)) >= 0);
+ VERIFY ((slavename = ptsname (master)));
+ VERIFY (unlockpt (master) == 0);
+ if (strncmp (slavename, "/dev/pts/", 9) != 0)
+ FAIL_UNSUPPORTED ("slave pseudo-terminal is not under /dev/pts/: %s",
+ slavename);
+ int slave = xopen (slavename, O_RDWR, 0);
+ if (!doit (slave, "basic smoketest",
+ (struct result_r){.name=slavename, .ret=0, .err=0}))
+ return 1;
+
+ pid_t pid = xfork ();
+ if (pid == 0)
+ {
+ xclose (master);
+
+ become_root_in_mount_ns ();
+
+ VERIFY (mount ("tmpfs", chrootdir, "tmpfs", 0, "mode=755") == 0);
+ VERIFY (chdir (chrootdir) == 0);
+
+ xmkdir ("proc", 0755);
+ xmkdir ("dev", 0755);
+ xmkdir ("dev/pts", 0755);
+
+ VERIFY (mount ("/proc", "proc", NULL, MS_BIND|MS_REC, NULL) == 0);
+ VERIFY (mount ("devpts", "dev/pts", "devpts",
+ MS_NOSUID|MS_NOEXEC,
+ "newinstance,ptmxmode=0666,mode=620") == 0);
+ VERIFY (symlink ("pts/ptmx", "dev/ptmx") == 0);
+
+ xtouch ("console", 0);
+ xtouch ("dev/console", 0);
+ VERIFY (mount (slavename, "console", NULL, MS_BIND, NULL) == 0);
+
+ xchroot (".");
+
+ char *linkname = xasprintf ("/proc/self/fd/%d", slave);
+ char *target = xreadlink (linkname);
+ VERIFY (strcmp (target, slavename) == 0);
+ free (linkname);
+
+ _exit (cb (slavename, slave));
+ }
+ int status;
+ xwaitpid (pid, &status, 0);
+ VERIFY (WIFEXITED (status));
+ xclose (master);
+ xclose (slave);
+ return WEXITSTATUS (status);
+}
+
+static int
+do_in_chroot_2 (int (*cb)(const char *, int))
+{
+ printf ("info: entering chroot 2\n");
+
+ int pid_pipe[2];
+ xpipe (pid_pipe);
+ int exit_pipe[2];
+ xpipe (exit_pipe);
+
+ /* Open the PTS that we'll be testing on. */
+ int master;
+ char *slavename;
+ VERIFY ((master = posix_openpt (O_RDWR|O_NOCTTY|O_NONBLOCK)) >= 0);
+ VERIFY ((slavename = ptsname (master)));
+ VERIFY (unlockpt (master) == 0);
+ if (strncmp (slavename, "/dev/pts/", 9) != 0)
+ FAIL_UNSUPPORTED ("slave pseudo-terminal is not under /dev/pts/: %s",
+ slavename);
+ /* wait until in a new mount ns to open the slave */
+
+ /* enable `wait`ing on grandchildren */
+ VERIFY (prctl (PR_SET_CHILD_SUBREAPER, 1) == 0);
+
+ pid_t pid = xfork (); /* outer child */
+ if (pid == 0)
+ {
+ xclose (master);
+ xclose (pid_pipe[0]);
+ xclose (exit_pipe[1]);
+
+ become_root_in_mount_ns ();
+
+ int slave = xopen (slavename, O_RDWR, 0);
+ if (!doit (slave, "basic smoketest",
+ (struct result_r){.name=slavename, .ret=0, .err=0}))
+ _exit (1);
+
+ VERIFY (mount ("tmpfs", chrootdir, "tmpfs", 0, "mode=755") == 0);
+ VERIFY (chdir (chrootdir) == 0);
+
+ xmkdir ("proc", 0755);
+ xmkdir ("dev", 0755);
+ xmkdir ("dev/pts", 0755);
+
+ VERIFY (mount ("devpts", "dev/pts", "devpts",
+ MS_NOSUID|MS_NOEXEC,
+ "newinstance,ptmxmode=0666,mode=620") == 0);
+ VERIFY (symlink ("pts/ptmx", "dev/ptmx") == 0);
+
+ xtouch ("console", 0);
+ xtouch ("dev/console", 0);
+ VERIFY (mount (slavename, "console", NULL, MS_BIND, NULL) == 0);
+
+ xchroot (".");
+
+ if (unshare (CLONE_NEWNS | CLONE_NEWPID) < 0)
+ FAIL_UNSUPPORTED ("could not enter new PID namespace");
+ pid = xfork (); /* inner child */
+ if (pid == 0)
+ {
+ xclose (pid_pipe[1]);
+
+ /* wait until the outer child has exited */
+ char c;
+ VERIFY (read (exit_pipe[0], &c, 1) == 0);
+ xclose (exit_pipe[0]);
+
+ VERIFY (mount ("proc", "/proc", "proc",
+ MS_NOSUID|MS_NOEXEC|MS_NODEV, NULL) == 0);
+
+ char *linkname = xasprintf ("/proc/self/fd/%d", slave);
+ char *target = xreadlink (linkname);
+ VERIFY (strcmp (target, strrchr (slavename, '/')) == 0);
+ free (linkname);
+
+ _exit (cb (slavename, slave));
+ }
+ xwrite (pid_pipe[1], &pid, sizeof pid);
+ _exit (0);
+ }
+ xclose (pid_pipe[1]);
+ xclose (exit_pipe[0]);
+ xclose (exit_pipe[1]);
+
+ /* wait for the outer child */
+ int status;
+ xwaitpid (pid, &status, 0);
+ VERIFY (WIFEXITED (status));
+ int ret = WEXITSTATUS (status);
+ if (ret != 0)
+ return ret;
+
+ /* set 'pid' to the inner child */
+ VERIFY (read (pid_pipe[0], &pid, sizeof pid) == sizeof pid);
+ xclose (pid_pipe[0]);
+
+ /* wait for the inner child */
+ xwaitpid (pid, &status, 0);
+ VERIFY (WIFEXITED (status));
+ xclose (master);
+ return WEXITSTATUS (status);
+}
+
+/* main test */
+
+static int
+run_chroot_tests (const char *slavename, int slave)
+{
+ struct stat st;
+ bool ok = true;
+
+ /* There are 3 groups of tests here. The first group fairly
+ generically does things known to mess up ttyname, and verifies
+ that ttyname copes correctly. The remaining groups are
+ increasingly convoluted, as we target specific parts of ttyname
+ to try to confuse. */
+
+ /* Basic tests that it doesn't get confused by multiple devpts
+ instances. */
+ {
+ VERIFY (stat (slavename, &st) < 0); /* sanity check */
+ if (!doit (slave, "no conflict, no match",
+ (struct result_r){.name=NULL, .ret=ENODEV, .err=ENODEV}))
+ ok = false;
+ VERIFY (mount ("/console", "/dev/console", NULL, MS_BIND, NULL) == 0);
+ if (!doit (slave, "no conflict, console",
+ (struct result_r){.name="/dev/console", .ret=0, .err=0}))
+ ok = false;
+ VERIFY (umount ("/dev/console") == 0);
+
+ /* keep creating PTYs until we we get a name collision */
+ while (stat (slavename, &st) < 0)
+ posix_openpt (O_RDWR|O_NOCTTY|O_NONBLOCK);
+ VERIFY (stat (slavename, &st) == 0);
+
+ if (!doit (slave, "conflict, no match",
+ (struct result_r){.name=NULL, .ret=ENODEV, .err=ENODEV}))
+ ok = false;
+ VERIFY (mount ("/console", "/dev/console", NULL, MS_BIND, NULL) == 0);
+ if (!doit (slave, "conflict, console",
+ (struct result_r){.name="/dev/console", .ret=0, .err=0}))
+ ok = false;
+ VERIFY (umount ("/dev/console") == 0);
+ }
+
+ /* The first tests kinda assumed that they hit certain code-paths
+ based on assuming that the readlink target is 'slavename', but
+ that's not quite always true. They're still a good preliminary
+ sanity check, so keep them, but let's add tests that make sure
+ that those code-paths are hit by doing a readlink ourself. */
+ {
+ char *linkname = xasprintf ("/proc/self/fd/%d", slave);
+ char *target = xreadlink (linkname);
+ free (linkname);
+ /* Depeding on how we set up the chroot, the kernel may or may not
+ trim the leading path to the target (it may give us "/6",
+ instead of "/dev/pts/6"). We test it both ways (do_in_chroot_1
+ and do_in_chroot_2). This test group relies on the target
+ existing, so guarantee that it does exist by creating it if
+ necessary. */
+ if (stat (target, &st) < 0)
+ {
+ VERIFY (errno == ENOENT);
+ xtouch (target, 0);
+ }
+
+ VERIFY (mount ("/console", "/dev/console", NULL, MS_BIND, NULL) == 0);
+ VERIFY (mount ("/console", target, NULL, MS_BIND, NULL) == 0);
+ if (!doit (slave, "with readlink target",
+ (struct result_r){.name=target, .ret=0, .err=0}))
+ ok = false;
+ VERIFY (umount (target) == 0);
+ VERIFY (umount ("/dev/console") == 0);
+
+ VERIFY (mount ("/console", "/dev/console", NULL, MS_BIND, NULL) == 0);
+ VERIFY (mount (slavename, target, NULL, MS_BIND, NULL) == 0);
+ if (!doit (slave, "with readlink trap; fallback",
+ (struct result_r){.name="/dev/console", .ret=0, .err=0}))
+ ok = false;
+ VERIFY (umount (target) == 0);
+ VERIFY (umount ("/dev/console") == 0);
+
+ VERIFY (mount (slavename, target, NULL, MS_BIND, NULL) == 0);
+ if (!doit (slave, "with readlink trap; no fallback",
+ (struct result_r){.name=NULL, .ret=ENODEV, .err=ENODEV}))
+ ok = false;
+ VERIFY (umount (target) == 0);
+ }
+
+ /* This test makes sure that everything still works OK if readdir
+ finds a pseudo-match before and/or after the actual match. Now,
+ to do that, we need to control that readdir finds the
+ pseudo-matches before and after the actual match; and there's no
+ good way to control that order in absence of whitebox testing.
+ So, just create 3 files, then use opendir/readdir to see what
+ order they are in, and assign meaning based on that order, not by
+ name; assigning the first to be a pseudo-match, the second to be
+ the actual match, and the third to be a pseudo-match. This
+ assumes that (on tmpfs) ordering within the directory is stable
+ in the absence of modification, which seems reasonably safe. */
+ {
+ /* since we're testing the fallback search, disable the readlink
+ happy-path */
+ VERIFY (umount2 ("/proc", MNT_DETACH) == 0);
+
+ xtouch ("/dev/console1", 0);
+ xtouch ("/dev/console2", 0);
+ xtouch ("/dev/console3", 0);
+
+ char *c[3];
+ int ci = 0;
+ DIR *dirstream = opendir ("/dev");
+ VERIFY (dirstream != NULL);
+ struct dirent *d;
+ while ((d = readdir (dirstream)) != NULL && ci < 3)
+ {
+ if (strcmp (d->d_name, "console1") &&
+ strcmp (d->d_name, "console2") &&
+ strcmp (d->d_name, "console3") )
+ continue;
+ c[ci++] = xasprintf ("/dev/%s", d->d_name);
+ }
+ VERIFY (ci == 3);
+ VERIFY (closedir (dirstream) == 0);
+
+ VERIFY (mount (slavename, c[0], NULL, MS_BIND, NULL) == 0);
+ VERIFY (mount ("/console", c[1], NULL, MS_BIND, NULL) == 0);
+ VERIFY (mount (slavename, c[2], NULL, MS_BIND, NULL) == 0);
+ VERIFY (umount2 ("/dev/pts", MNT_DETACH) == 0);
+ if (!doit (slave, "with search-path trap",
+ (struct result_r){.name=c[1], .ret=0, .err=0}))
+ ok = false;
+ for (int i = 0; i < 3; i++)
+ {
+ VERIFY (umount (c[i]) == 0);
+ VERIFY (unlink (c[i]) == 0);
+ free (c[i]);
+ }
+ }
+
+ return ok ? 0 : 1;
+}
+
+static int
+do_test (void)
+{
+ int ret1 = do_in_chroot_1 (run_chroot_tests);
+ if (ret1 == EXIT_UNSUPPORTED)
+ return ret1;
+
+ int ret2 = do_in_chroot_2 (run_chroot_tests);
+ if (ret2 == EXIT_UNSUPPORTED)
+ return ret2;
+
+ return ret1 | ret2;
+}
+
+#include <support/test-driver.c>
--
2.15.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 0/6] Fixup linux ttyname and ttyname_r [BZ #22145]
2017-11-08 18:01 [PATCH v3 0/6] Fixup linux ttyname and ttyname_r [BZ #22145] Luke Shumaker
` (5 preceding siblings ...)
2017-11-08 18:09 ` [PATCH v3 5/6] linux ttyname and ttyname_r: Don't bail prematurely [BZ #22145] Luke Shumaker
@ 2017-11-10 16:06 ` Christian Brauner
2017-11-10 20:01 ` Luke Shumaker
6 siblings, 1 reply; 11+ messages in thread
From: Christian Brauner @ 2017-11-10 16:06 UTC (permalink / raw)
To: Luke Shumaker; +Cc: libc-alpha
Hi Luke,
Would you please resend the whole patch series bumping
<idx> in [PATCH v<idx> m/n] for all patches? As it is right now it's a little
confusing which v<idx> is the latest for each individual patch.
Thanks!
Christian
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 0/6] Fixup linux ttyname and ttyname_r [BZ #22145]
2017-11-10 16:06 ` [PATCH v3 0/6] Fixup linux ttyname and ttyname_r [BZ #22145] Christian Brauner
@ 2017-11-10 20:01 ` Luke Shumaker
0 siblings, 0 replies; 11+ messages in thread
From: Luke Shumaker @ 2017-11-10 20:01 UTC (permalink / raw)
To: Christian Brauner; +Cc: Luke Shumaker, libc-alpha
On Fri, 10 Nov 2017 11:06:00 -0500,
Christian Brauner wrote:
> Hi Luke,
>
> Would you please resend the whole patch series bumping
> <idx> in [PATCH v<idx> m/n] for all patches? As it is right now it's a little
> confusing which v<idx> is the latest for each individual patch.
>
> Thanks!
> Christian
Except for tst-ttyname (which is v5), everything else is v3.
But I'll resubmit it all as v6.
--
Happy hacking,
~ Luke Shumaker
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2017-11-10 20:01 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-08 18:01 [PATCH v3 0/6] Fixup linux ttyname and ttyname_r [BZ #22145] Luke Shumaker
2017-11-08 18:01 ` [PATCH v3 2/6] linux ttyname: Update a reference to kernel docs for kernel 4.10 Luke Shumaker
2017-11-08 18:01 ` [PATCH v3 1/6] manual: Update to mention ENODEV for ttyname and ttyname_r Luke Shumaker
2017-11-08 18:01 ` [PATCH v3 3/6] linux ttyname: Change return type of is_pty from int to bool Luke Shumaker
2017-11-08 18:01 ` [PATCH v3 6/6] linux ttyname and ttyname_r: Add tests Luke Shumaker
2017-11-08 18:09 ` [PATCH v3 4/6] linux ttyname and ttyname_r: Make the TTY equivalence checks consistent Luke Shumaker
2017-11-08 18:09 ` [PATCH v3 5/6] linux ttyname and ttyname_r: Don't bail prematurely [BZ #22145] Luke Shumaker
2017-11-09 2:18 ` [PATCH v4 1/1] linux ttyname and ttyname_r: Add tests Luke Shumaker
2017-11-09 16:24 ` [PATCH v5 " Luke Shumaker
2017-11-10 16:06 ` [PATCH v3 0/6] Fixup linux ttyname and ttyname_r [BZ #22145] Christian Brauner
2017-11-10 20:01 ` Luke Shumaker
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).