[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#817236: schroot: no access to pseudo-terminals in new chroots



I have filed #856877 against schroot suggesting mounting a new instance
of /dev/pts, effectively making schroot behave less like chroot and more
like a container manager in this particular respect.

However, that causes a nasty regression for interactive use (ttyname()
won't report that the interactive tty outside the container is a tty,
because it has no name inside the container), so I don't think it can
be applied unless schroot also changes to give the invoked shell a
pty inside the container as its stdin/stdout/stderr
(like systemd-nspawn does).

So I think it is best if this is solved in debootstrap, to the extent
that it can be.

I attach extended test coverage in debootstrap's autopkgtests for what
I've proposed schroot should consider doing, and for what I'm going to
propose pbuilder does. Up to you whether these are applied or not,
given the down side for interactive use, but I've successfully tested
them in a sid lxc container (lxc from jessie-backports, under the jessie
kernel), in a sid qemu VM (with sid kernel), and in a sid schroot (in
a jessie VM with the jessie kernel).

    S
>From 6890fea3e31910b03996812072da2900d1735e95 Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@debian.org>
Date: Sun, 5 Mar 2017 17:41:32 +0000
Subject: [PATCH 1/2] autopkgtest: Exercise possible future pbuilder behaviour

---
 debian/tests/debian-testing         | 13 +++++++----
 debian/tests/fake/pbuilder-proposed | 43 +++++++++++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+), 4 deletions(-)
 create mode 100755 debian/tests/fake/pbuilder-proposed

diff --git a/debian/tests/debian-testing b/debian/tests/debian-testing
index fb7da66..928e4e1 100755
--- a/debian/tests/debian-testing
+++ b/debian/tests/debian-testing
@@ -35,10 +35,13 @@ sub capture {
 sub check_fake_schroot {
     my %params = @_;
     my $reference = $params{reference};
+    my $version = $params{version} || '1.6.10-3';
     my $extra_argv = $params{extra_argv} || [];
 
+    # Use unshare -m to make sure the /dev mount gets cleaned up on exit, even
+    # on failures
     my $response = capture([qw(unshare -m),
-        "$srcdir/debian/tests/fake/schroot-1.6.10-3", @{$extra_argv},
+        "$srcdir/debian/tests/fake/schroot-$version", @{$extra_argv},
             $params{chroot},
         qw(runuser -u nobody --),
         qw(script -q -c), 'cat /etc/debian_version', '/dev/null']);
@@ -49,9 +52,10 @@ sub check_fake_schroot {
 sub check_fake_pbuilder {
     my %params = @_;
     my $reference = $params{reference};
+    my $version = $params{version} || '0.228.4-1';
 
     my $response = capture([qw(unshare -m),
-        "$srcdir/debian/tests/fake/pbuilder-0.228.4-1", $params{chroot},
+        "$srcdir/debian/tests/fake/pbuilder-$version", $params{chroot},
         qw(runuser -u nobody --),
         qw(script -q -c), 'cat /etc/debian_version', '/dev/null']);
     $response =~ s/\r//g;
@@ -108,8 +112,6 @@ sub check_chroot {
                 cat /etc/debian_version)]),
         $reference);
 
-    # Use unshare -m to make sure the /dev mount gets cleaned up on exit, even
-    # on failures
     check_fake_schroot(%params, reference => $reference);
 
     # https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=817236
@@ -135,6 +137,9 @@ sub check_chroot {
             extra_argv => ['--sbuild']);
     }
 
+    check_fake_pbuilder(%params, reference => $reference,
+        version => 'proposed');
+
     # https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=817236
     if (! $params{can_mknod_ptmx}) {
         TODO: {
diff --git a/debian/tests/fake/pbuilder-proposed b/debian/tests/fake/pbuilder-proposed
new file mode 100755
index 0000000..cea60b7
--- /dev/null
+++ b/debian/tests/fake/pbuilder-proposed
@@ -0,0 +1,43 @@
+#!/bin/sh
+# fake/pbuilder-proposed -- emulate how pbuilder is proposed to chroot in
+# future.
+#
+# Copyright © 2017 Simon McVittie
+# SPDX-License-Identifier: MIT
+# (see debian/copyright)
+
+set -e
+
+BUILDPLACE="$1"
+shift
+if test -z "$BUILDPLACE" || test -z "$1"; then
+	echo "Usage: $0 CHROOT COMMAND...">&2
+	exit 2
+fi
+
+devpts_options="noexec,nosuid,gid=5,mode=620"
+
+mkdir -p "$BUILDPLACE/dev/pts"
+if ! mount -t devpts none "$BUILDPLACE/dev/pts" -o "$devpts_options,newinstance,ptmxmode=666"; then
+    mount -t devpts none "$BUILDPLACE/dev/pts" -o "$devpts_options"
+fi
+
+mounted_ptmx=no
+
+if [ -e "$BUILDPLACE/dev/pts/ptmx" ] && \
+        [ -e "$BUILDPLACE/dev/ptmx" ] && \
+        ! [ "$BUILDPLACE/dev/pts/ptmx" -ef "$BUILDPLACE/dev/ptmx" ]; then
+    mounted_ptmx=yes
+    chmod 666 "$BUILDPLACE/dev/pts/ptmx"
+    mount --bind "$BUILDPLACE/dev/pts/ptmx" "$BUILDPLACE/dev/ptmx"
+fi
+
+ls -l "$BUILDPLACE/dev/ptmx" | sed -e 's/^/# fake-pbuilder: /' >&2
+ls -l "$BUILDPLACE/dev/pts/ptmx" | sed -e 's/^/# fake-pbuilder: /' >&2
+
+e=0
+chroot "$BUILDPLACE" "$@" || e=$?
+
+[ "$mounted_ptmx" = no ] || umount "$BUILDPLACE/dev/ptmx"
+umount "$BUILDPLACE/dev/pts"
+exit "$e"
-- 
2.11.0

>From 322e9ee8c4d7e713df83257913ee6c6ba4a9daea Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@debian.org>
Date: Sun, 5 Mar 2017 18:27:41 +0000
Subject: [PATCH 2/2] autopkgtest: Exercise possible future sbuild behaviour
 (#856877)

---
 debian/tests/debian-testing        |  3 ++
 debian/tests/fake/schroot-proposed | 57 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 60 insertions(+)
 create mode 100755 debian/tests/fake/schroot-proposed

diff --git a/debian/tests/debian-testing b/debian/tests/debian-testing
index 928e4e1..65ee196 100755
--- a/debian/tests/debian-testing
+++ b/debian/tests/debian-testing
@@ -113,6 +113,9 @@ sub check_chroot {
         $reference);
 
     check_fake_schroot(%params, reference => $reference);
+    check_fake_schroot(%params, reference => $reference, version => 'proposed');
+    check_fake_schroot(%params, reference => $reference, version => 'proposed',
+        extra_argv => ['--sbuild']);
 
     # https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=817236
     if (Dpkg::Version->new($params{kernel}) < Dpkg::Version->new('4.7') &&
diff --git a/debian/tests/fake/schroot-proposed b/debian/tests/fake/schroot-proposed
new file mode 100755
index 0000000..96d4309
--- /dev/null
+++ b/debian/tests/fake/schroot-proposed
@@ -0,0 +1,57 @@
+#!/bin/sh
+# fake/schroot-proposed -- emulate proposed mount behaviour for schroot
+#
+# Copyright © 2017 Simon McVittie
+# SPDX-License-Identifier: MIT
+# (see debian/copyright)
+
+set -e
+
+# /etc/schroot/default/fstab
+bind_dev=yes
+
+while true; do
+	case "$1" in
+		(--sbuild)
+			shift
+			# /etc/schroot/sbuild/fstab
+			bind_dev=no
+			;;
+		(*)
+			break
+	esac
+done
+
+chroot="$1"
+shift
+if test -z "$chroot" || test -z "$1"; then
+	echo "Usage: $0 CHROOT COMMAND...">&2
+	exit 2
+fi
+
+[ "$bind_dev" = no ] || mount --bind /dev "$chroot/dev"
+mount -t devpts -o rw,newinstance,ptmxmode=666,mode=620,gid=5 /dev/pts "$chroot/dev/pts"
+
+ls -l "$chroot/dev/ptmx" | sed -e 's/^/# fake-schroot: /' >&2
+ls -l "$chroot/dev/pts/ptmx" | sed -e 's/^/# fake-schroot: /' >&2
+
+mounted_ptmx=no
+
+if [ -e "$chroot/dev/pts/ptmx" ] && \
+        [ -e "$chroot/dev/ptmx" ] && \
+        ! [ "$chroot/dev/pts/ptmx" -ef "$chroot/dev/ptmx" ]; then
+    mount --bind "$chroot/dev/pts/ptmx" "$chroot/dev/ptmx"
+    mounted_ptmx=yes
+fi
+
+ls -l "$chroot/dev/ptmx" | sed -e 's/^/# fake-schroot: /' >&2
+ls -l "$chroot/dev/pts/ptmx" | sed -e 's/^/# fake-schroot: /' >&2
+
+e=0
+chroot "$chroot" "$@" || e=$?
+
+[ "$mounted_ptmx" = no ] || umount "$chroot/dev/ptmx"
+umount "$chroot/dev/pts"
+[ "$bind_dev" = no ] || umount "$chroot/dev"
+
+exit "$e"
-- 
2.11.0


Reply to: