From 009089ee8a94ad5066b83c5a4cdecfd762a400aa Mon Sep 17 00:00:00 2001 From: Johannes Schauer Marin Rodrigues Date: Tue, 14 Jun 2022 08:26:48 +0200 Subject: [PATCH] Mount a new instance of /dev/pts in the chroot Before, we bind-mounted /dev/ptmx and /dev/pts from the host into the chroot. This will make posix_openpt() fail with 'No such file or directory'. The ability to create pseudo terminals is important for apt (which will throw a warning otherwise) or running script(1) or source package testsuites like for src:util-linux. This functionality is restored by mounting a new devpts instance to /dev/pts and making /dev/ptmx a symlink to /dev/pts/ptmx. Mounting with ptmxmode=666 is required such that also non-root users in unshare mode are able to create pseudo terminals. See also: https://www.kernel.org/doc/Documentation/filesystems/devpts.txt https://salsa.debian.org/debian/schroot/-/merge_requests/2 https://bugs.debian.org/856877 https://bugs.debian.org/817236 --- coverage.txt | 4 ++ mmdebstrap | 47 ++++++++++++++-- tests/dev-ptmx | 145 +++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 191 insertions(+), 5 deletions(-) create mode 100644 tests/dev-ptmx diff --git a/coverage.txt b/coverage.txt index c496580..e28d525 100644 --- a/coverage.txt +++ b/coverage.txt @@ -318,3 +318,7 @@ Skip-If: Test: no-sbin-in-path Modes: fakechroot + +Test: dev-ptmx +Modes: root unshare +Needs-QEMU: true diff --git a/mmdebstrap b/mmdebstrap index 8007183..829cc66 100755 --- a/mmdebstrap +++ b/mmdebstrap @@ -1091,6 +1091,20 @@ sub run_chroot { ); next; } + if ($fname eq "ptmx") { + # We must not bind-mount ptmx from the outside or + # otherwise posix_openpt() will fail. Instead + # /dev/ptmx must refer to /dev/pts/ptmx either by + # symlink or by bind-mounting. We choose a symlink. + symlink '/dev/pts/ptmx', + "$options->{root}/dev/ptmx" + or error "cannot create /dev/pts/ptmx symlink"; + push @cleanup_tasks, sub { + unlink "$options->{root}/dev/ptmx" + or error "unlink /dev/ptmx"; + }; + next; + } if (!-e "/dev/$fname") { warning("skipping creation of ./dev/$fname because" . " /dev/$fname does not exist" @@ -1133,13 +1147,13 @@ sub run_chroot { . " /dev directory is missing in the target"); next; } - if (!-e "/dev/$fname") { + if (!-e "/dev/$fname" && $fname ne "pts/") { warning("skipping creation of ./dev/$fname because" . " /dev/$fname does not exist" . " on the outside"); next; } - if (!-d "/dev/$fname") { + if (!-d "/dev/$fname" && $fname ne "pts/") { warning("skipping creation of ./dev/$fname because" . " /dev/$fname on the outside is not a" . " directory"); @@ -1187,9 +1201,32 @@ sub run_chroot { "$options->{root}/dev/$fname") or warning("umount ./dev/$fname failed: $?"); }; - 0 == system('mount', '-o', 'bind', "/dev/$fname", - "$options->{root}/dev/$fname") - or error "mount ./dev/$fname failed: $?"; + if ($fname eq "pts/") { + # We cannot just bind-mount /dev/pts from the host as + # doing so will make posix_openpt() fail. Instead, we + # need to mount a new devpts. + # We need ptmxmode=666 because /dev/ptmx is a symlink + # to /dev/pts/ptmx and without it posix_openpt() will + # fail if we are not the root user. + # See also: + # kernel.org/doc/Documentation/filesystems/devpts.txt + # salsa.debian.org/debian/schroot/-/merge_requests/2 + # https://bugs.debian.org/856877 + # https://bugs.debian.org/817236 + 0 == system( + 'mount', + '-t', + 'devpts', + 'none', + "$options->{root}/dev/pts", + '-o', + 'noexec,nosuid,uid=5,mode=620,ptmxmode=666' + ) or error "mount /dev/pts failed"; + } else { + 0 == system('mount', '-o', 'bind', "/dev/$fname", + "$options->{root}/dev/$fname") + or error "mount ./dev/$fname failed: $?"; + } } else { error "unsupported type: $type"; } diff --git a/tests/dev-ptmx b/tests/dev-ptmx new file mode 100644 index 0000000..c22820e --- /dev/null +++ b/tests/dev-ptmx @@ -0,0 +1,145 @@ +#!/bin/sh +set -eu +export LC_ALL=C.UTF-8 + +if [ {{ MODE }} != unshare ] && [ {{ MODE }} != root ]; then + echo "test requires root or unshare mode" >&2 + exit 1 +fi + +if [ ! -e /mmdebstrap-testenv ]; then + echo "this test modifies the system and should only be run inside a container" >&2 + exit 1 +fi +if [ "$(id -u)" -eq 0 ] && ! id -u user > /dev/null 2>&1; then + adduser --gecos user --disabled-password user +fi +prefix= +[ "$(id -u)" -eq 0 ] && [ "{{ MODE }}" != "root" ] && prefix="runuser -u user --" + +# this mimics what apt does in apt-pkg/deb/dpkgpm.cc/pkgDPkgPM::StartPtyMagic() +cat > /tmp/test.c << 'END' +#define _GNU_SOURCE + +#include +#include +#include +#include +#include +#include +#include + +int main() { + int ret; + int fd = posix_openpt(O_RDWR | O_NOCTTY); + if (fd < 0) { + perror("posix_openpt"); + return 1; + } + char buf[64]; // 64 is used by apt + ret = ptsname_r(fd, buf, sizeof(buf)); + if (ret != 0) { + perror("ptsname_r"); + return 1; + } + ret = grantpt(fd); + if (ret == -1) { + perror("grantpt"); + return 1; + } + struct termios origtt; + ret = tcgetattr(STDIN_FILENO, &origtt); + if (ret != 0) { + perror("tcgetattr1"); + return 1; + } + struct termios tt; + ret = tcgetattr(STDOUT_FILENO, &tt); + if (ret != 0) { + perror("tcgetattr2"); + return 1; + } + struct winsize win; + ret = ioctl(STDOUT_FILENO, TIOCGWINSZ, &win); + if (ret < 0) { + perror("ioctl stdout TIOCGWINSZ"); + return 1; + } + ret = ioctl(fd, TIOCSWINSZ, &win); + if (ret < 0) { + perror("ioctl fd TIOCGWINSZ"); + return 1; + } + ret = tcsetattr(fd, TCSANOW, &tt); + if (ret != 0) { + perror("tcsetattr1"); + return 1; + } + cfmakeraw(&tt); + tt.c_lflag &= ~ECHO; + tt.c_lflag |= ISIG; + sigset_t sigmask; + sigset_t sigmask_old; + ret = sigemptyset(&sigmask); + if (ret != 0) { + perror("sigemptyset"); + return 1; + } + ret = sigaddset(&sigmask, SIGTTOU); + if (ret != 0) { + perror("sigaddset"); + return 1; + } + ret = sigprocmask(SIG_BLOCK,&sigmask, &sigmask_old); + if (ret != 0) { + perror("sigprocmask1"); + return 1; + } + ret = tcsetattr(STDIN_FILENO, TCSAFLUSH, &tt); + if (ret != 0) { + perror("tcsetattr2"); + return 1; + } + ret = sigprocmask(SIG_BLOCK,&sigmask_old, NULL); + if (ret != 0) { + perror("sigprocmask2"); + return 1; + } + ret = tcsetattr(STDIN_FILENO, TCSAFLUSH, &origtt); + if (ret != 0) { + perror("tcsetattr3"); + return 1; + } + return 0; +} +END + +# use script to create a fake tty +# run all tests as root and as a normal user (the latter requires ptmxmode=666) +script -qfc "$prefix {{ CMD }} --mode={{ MODE }} --variant=apt \ + --include=gcc,libc6-dev,python3 \ + --customize-hook='chroot \"\$1\" adduser --gecos user --disabled-password user' \ + --customize-hook='chroot \"\$1\" python3 -c \"import pty; print(pty.openpty())\"' \ + --customize-hook='chroot \"\$1\" runuser -u user -- python3 -c \"import pty; print(pty.openpty())\"' \ + --customize-hook='chroot \"\$1\" script -c \"echo foobar\"' \ + --customize-hook='chroot \"\$1\" runuser -u user -- env --chdir=/home/user script -c \"echo foobar\"' \ + --customize-hook='chroot \"\$1\" apt-get install --yes doc-debian 2>&1 | tee /tmp/log' \ + --customize-hook=\"copy-in /tmp/test.c /tmp\" \ + --customize-hook='chroot \"\$1\" gcc /tmp/test.c -o /tmp/test' \ + --customize-hook='chroot \"\$1\" /tmp/test' \ + --customize-hook='chroot \"\$1\" runuser -u user -- /tmp/test' \ + --customize-hook='rm \"\$1\"/tmp/test \"\$1\"/tmp/test.c' \ + {{ DIST }} /dev/null {{ MIRROR }}" /dev/null + +fail=0 +grep '^E:' /tmp/log && fail=1 +grep 'Can not write log' /tmp/log && fail=1 +grep 'posix_openpt' /tmp/log && fail=1 +grep 'No such file or directory' /tmp/log && fail=1 +if [ $fail -eq 1 ]; then + echo "apt failed to write log:" >&2 + cat /tmp/log >&2 + exit 1 +fi + +rm /tmp/test.c /tmp/log