From unknown Fri Aug 15 21:22:11 2025 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-Mailer: MIME-tools 5.509 (Entity 5.509) Content-Type: text/plain; charset=utf-8 From: bug#18736 <18736@debbugs.gnu.org> To: bug#18736 <18736@debbugs.gnu.org> Subject: Status: chroot regression - chroot avoids the chroot() call too eagerly. Reply-To: bug#18736 <18736@debbugs.gnu.org> Date: Sat, 16 Aug 2025 04:22:11 +0000 retitle 18736 chroot regression - chroot avoids the chroot() call too eager= ly. reassign 18736 coreutils submitter 18736 Rogier severity 18736 normal thanks From debbugs-submit-bounces@debbugs.gnu.org Wed Oct 15 11:43:46 2014 Received: (at submit) by debbugs.gnu.org; 15 Oct 2014 15:43:46 +0000 Received: from localhost ([127.0.0.1]:44609 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1XeQjp-00071R-7Y for submit@debbugs.gnu.org; Wed, 15 Oct 2014 11:43:45 -0400 Received: from eggs.gnu.org ([208.118.235.92]:36937) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1XeL5f-0004UP-Ep for submit@debbugs.gnu.org; Wed, 15 Oct 2014 05:41:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XeL5V-0004E7-0q for submit@debbugs.gnu.org; Wed, 15 Oct 2014 05:41:55 -0400 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on eggs.gnu.org X-Spam-Level: * X-Spam-Status: No, score=1.1 required=5.0 tests=BAYES_50, FREEMAIL_ENVFROM_END_DIGIT,FREEMAIL_FROM,T_DKIM_INVALID autolearn=disabled version=3.3.2 Received: from lists.gnu.org ([2001:4830:134:3::11]:43030) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XeL5U-0004E1-TY for submit@debbugs.gnu.org; Wed, 15 Oct 2014 05:41:44 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38203) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XeL55-0003I9-0d for bug-coreutils@gnu.org; Wed, 15 Oct 2014 05:41:44 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XeL4B-0003rx-Bf for bug-coreutils@gnu.org; Wed, 15 Oct 2014 05:41:18 -0400 Received: from mail-wi0-x22c.google.com ([2a00:1450:400c:c05::22c]:63833) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XeL4B-0003rZ-4o for bug-coreutils@gnu.org; Wed, 15 Oct 2014 05:40:23 -0400 Received: by mail-wi0-f172.google.com with SMTP id n3so12362987wiv.17 for ; Wed, 15 Oct 2014 02:40:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:to:subject:date:message-id:user-agent:mime-version :content-transfer-encoding:content-type; bh=amEcfxbyxGRaKAdVFdKluuceG++vdGTHl4rvnyhr8kU=; b=h5ySbokJd4wyY/GErIAWc8arv5aS+SUB5KWa9w/o4EkBRnWE4qz7cW4EMtWu1SgFsc OdHLFtQL2gtkN102Khef4HmQmGv+T4EEPKCJS3qxaR5/wRohJK9cu5WyPx70jH9BICAs S9b+5tCwJTS4Mzu0wP1Mt43vqUZUXdMpZcCxPl22+aEUEejG8VbBejuv44CLFpWP9vSr SihluZKrWlDm/FwhLJfnj448YFZUqTdIit/fsKfmo5hvGyfNbff2Ca40k4eIY1xE+hfg oFYpS2YVi0hrurW1pNGtFnNQ1j2J1g+NiMEB/4UU6+OR9bB40BCKobu9DDys+3dFF6Gm FUcw== X-Received: by 10.194.62.238 with SMTP id b14mr10748796wjs.46.1413366021507; Wed, 15 Oct 2014 02:40:21 -0700 (PDT) Received: from wiske.localnet ([86.93.246.90]) by mx.google.com with ESMTPSA id ic4sm18690551wid.19.2014.10.15.02.40.20 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 15 Oct 2014 02:40:20 -0700 (PDT) From: Rogier To: bug-coreutils@gnu.org Subject: chroot regression - chroot avoids the chroot() call too eagerly. Date: Wed, 15 Oct 2014 11:40:16 +0200 Message-ID: <3341391.OsvZrqhJ5a@wiske> User-Agent: KMail/4.14.1 (Linux/3.16-2-amd64; KDE/4.14.1; x86_64; ; ) MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-detected-operating-system: by eggs.gnu.org: Error: Malformed IPv6 address (bad octet value). X-detected-operating-system: by eggs.gnu.org: Error: Malformed IPv6 address (bad octet value). X-Received-From: 2001:4830:134:3::11 X-Spam-Score: -3.8 (---) X-Debbugs-Envelope-To: submit X-Mailman-Approved-At: Wed, 15 Oct 2014 11:43:43 -0400 X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.8 (---) Hi, Since a few months, it seems that chroot has started avoiding the chroot call if it can be determined to be idempotent. It looks like the new check is based on inode comparison - if the inode is the same, the chroot() call is considered idempotent, and not performed. However, while two directories being in the same file system and having the same inode number implies that they are the same directory, it does not necessarily imply that they have same file system path (i.e. use the same mountpoint), so there is no guarantee that the entire directory trees rooted at the two directories are also identical even though the directories are.This means that chroot will fail to chroot() in cases when the call would in fact not be idempotent. On my system (debian testing), I have / bind-mounted elsewhere, with a slightly different directory tree mounted beneath it, as is mounted beneath /. In my case, I need this so that I can make partial backups of the system - including some file systems but leaving out others. Undoubtly, there are other use-cases as well - I wouldn't be surprised if users of libpam-chroot can be affected by this, depending on exactly how they configure their chroot environments. The new chroot behavior no longer allows chrooting into such alternate trees. In my case, that means that my backup fails. Kind regards, Rogier. From debbugs-submit-bounces@debbugs.gnu.org Wed Oct 15 13:17:26 2014 Received: (at 18736) by debbugs.gnu.org; 15 Oct 2014 17:17:26 +0000 Received: from localhost ([127.0.0.1]:44701 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1XeSCS-0003hz-PJ for submit@debbugs.gnu.org; Wed, 15 Oct 2014 13:17:25 -0400 Received: from mx1.redhat.com ([209.132.183.28]:6178) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1XeSCP-0003hn-3e for 18736@debbugs.gnu.org; Wed, 15 Oct 2014 13:17:22 -0400 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s9FHHJl5022395 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Wed, 15 Oct 2014 13:17:20 -0400 Received: from [10.36.116.19] (ovpn-116-19.ams2.redhat.com [10.36.116.19]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s9FHHDl6016425 (version=TLSv1/SSLv3 cipher=DHE-RSA-CAMELLIA256-SHA bits=256 verify=NO); Wed, 15 Oct 2014 13:17:18 -0400 Message-ID: <543EAC18.9040206@draigBrady.com> Date: Wed, 15 Oct 2014 18:17:12 +0100 From: =?ISO-8859-1?Q?P=E1draig_Brady?= User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130110 Thunderbird/17.0.2 MIME-Version: 1.0 To: Rogier Subject: Re: bug#18736: chroot regression - chroot avoids the chroot() call too eagerly. References: <3341391.OsvZrqhJ5a@wiske> In-Reply-To: <3341391.OsvZrqhJ5a@wiske> X-Enigmail-Version: 1.6 Content-Type: multipart/mixed; boundary="------------060208050305040905030306" X-Scanned-By: MIMEDefang 2.68 on 10.5.11.22 X-Spam-Score: -5.0 (-----) X-Debbugs-Envelope-To: 18736 Cc: 18736@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -5.0 (-----) This is a multi-part message in MIME format. --------------060208050305040905030306 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 8bit On 10/15/2014 10:40 AM, Rogier wrote: > Hi, > > Since a few months, it seems that chroot has started avoiding the > chroot call if it can be determined to be idempotent. > It looks like the new check is based on inode comparison - if the > inode is the same, the chroot() call is considered idempotent, and not > performed. > > However, while two directories being in the same file system and having > the same inode number implies that they are the same directory, it > does not necessarily imply that they have same file system path (i.e. > use the same mountpoint), so there is no guarantee that the entire > directory trees rooted at the two directories are also identical even > though the directories are.This means that chroot will fail to > chroot() in cases when the call would in fact not be idempotent. > > On my system (debian testing), I have / bind-mounted elsewhere, with a > slightly different directory tree mounted beneath it, as is mounted > beneath /. In my case, I need this so that I can make partial backups > of the system - including some file systems but leaving out others. > Undoubtly, there are other use-cases as well - I wouldn't be surprised > if users of libpam-chroot can be affected by this, depending on exactly > how they configure their chroot environments. > > The new chroot behavior no longer allows chrooting into such alternate > trees. In my case, that means that my backup fails. I didn't consider this unusual case when avoiding the chroot() call, for efficiency (especially in the --userspec case), and to add consistency between platforms in the handling of `chroot / true` for non root users. I agree with your analysis and that we should revert to the previous behavior here, which is done in the attached patch. thanks! Pádraig. --------------060208050305040905030306 Content-Type: text/x-patch; name="chroot-always.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="chroot-always.patch" >From a51ed4e892b74aa400633292f1413138ccb4da62 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A1draig=20Brady?= Date: Wed, 15 Oct 2014 18:08:42 +0100 Subject: [PATCH] chroot: call chroot() unconditionally to handle bind mounted "/" * src/chroot.c (main): To handle a separate tree to "/" which is constructed by first bind mounting "/", we need to unconditionally call chroot(). This will cause inconsistency with some platforms not allowing `chroot / true` for non root users, but that is a lesser issue than not calling chroot() on the potentially different tree. * tests/misc/chroot-fail.sh: Adjust appropriately. * NEWS: Mention the bug fixes. Fixes http://bugs.gnu.org/18736 --- NEWS | 11 +++---- src/chroot.c | 67 +++++++++++++++++++++----------------------- tests/misc/chroot-fail.sh | 46 +++++++++++++++++------------- 3 files changed, 63 insertions(+), 61 deletions(-) diff --git a/NEWS b/NEWS index 52332bd..5fbdc6a 100644 --- a/NEWS +++ b/NEWS @@ -7,6 +7,11 @@ GNU coreutils NEWS -*- outline -*- dd supports more robust SIGINFO/SIGUSR1 handling for outputting statistics. Previously those signals may have inadvertently terminated the process. + chroot again calls chroot(DIR) and chdir("/"), even if DIR is "/". + This handles separate bind mounted "/" trees, and environments + depending on the implicit chdir("/"). + [bugs introduced in coreutils-8.23] + cp no longer issues an incorrect warning about directory hardlinks when a source directory is specified multiple times. Now, consistent with other file types, a warning is issued for source directories with duplicate names, @@ -25,12 +30,6 @@ GNU coreutils NEWS -*- outline -*- dd accepts a new status=progress level to print data transfer statistics on stderr approximately every second. -** Changes in behavior - - chroot changes the current directory to "/" in again - unless the above new - --skip-chdir option is specified. - [bug introduced in coreutils-8.23] - ** Improvements cp,install,mv will convert smaller runs of NULs in the input to holes, diff --git a/src/chroot.c b/src/chroot.c index 171ced9..bea2e9c 100644 --- a/src/chroot.c +++ b/src/chroot.c @@ -291,48 +291,45 @@ main (int argc, char **argv) usage (EXIT_CANCELED); } - /* Only do chroot specific actions if actually changing root. - The main difference here is that we don't change working dir. */ - if (! is_oldroot) + /* We have to look up users and groups twice. + - First, outside the chroot to load potentially necessary passwd/group + parsing plugins (e.g. NSS); + - Second, inside chroot to redo parsing in case IDs are different. + Within chroot lookup is the main justification for having + the --user option supported by the chroot command itself. + We do this even if is_oldroot, as we could potentially + still have a different tree in this case with a bind mounted '/'. */ + if (userspec) + ignore_value (parse_user_spec (userspec, &uid, &gid, NULL, NULL)); + + /* If no gid is supplied or looked up, do so now. + Also lookup the username for use with getgroups. */ + if (uid_set (uid) && (! groups || gid_unset (gid))) { - /* We have to look up users and groups twice. - - First, outside the chroot to load potentially necessary passwd/group - parsing plugins (e.g. NSS); - - Second, inside chroot to redo parsing in case IDs are different. - Within chroot lookup is the main justification for having - the --user option supported by the chroot command itself. */ - if (userspec) - ignore_value (parse_user_spec (userspec, &uid, &gid, NULL, NULL)); - - /* If no gid is supplied or looked up, do so now. - Also lookup the username for use with getgroups. */ - if (uid_set (uid) && (! groups || gid_unset (gid))) + const struct passwd *pwd; + if ((pwd = getpwuid (uid))) { - const struct passwd *pwd; - if ((pwd = getpwuid (uid))) - { - if (gid_unset (gid)) - gid = pwd->pw_gid; - username = pwd->pw_name; - } + if (gid_unset (gid)) + gid = pwd->pw_gid; + username = pwd->pw_name; } + } - if (groups && *groups) - ignore_value (parse_additional_groups (groups, &out_gids, &n_gids, - false)); + if (groups && *groups) + ignore_value (parse_additional_groups (groups, &out_gids, &n_gids, + false)); #if HAVE_SETGROUPS - else if (! groups && gid_set (gid) && username) - { - int ngroups = xgetgroups (username, gid, &out_gids); - if (0 < ngroups) - n_gids = ngroups; - } + else if (! groups && gid_set (gid) && username) + { + int ngroups = xgetgroups (username, gid, &out_gids); + if (0 < ngroups) + n_gids = ngroups; + } #endif - if (chroot (newroot) != 0) - error (EXIT_CANCELED, errno, _("cannot change root directory to %s"), - newroot); - } + if (chroot (newroot) != 0) + error (EXIT_CANCELED, errno, _("cannot change root directory to %s"), + newroot); if (! skip_chdir && chdir ("/")) error (EXIT_CANCELED, errno, _("cannot chdir to root directory")); diff --git a/tests/misc/chroot-fail.sh b/tests/misc/chroot-fail.sh index 82ae23c..75f724a 100755 --- a/tests/misc/chroot-fail.sh +++ b/tests/misc/chroot-fail.sh @@ -29,14 +29,18 @@ test $? = 125 || fail=1 chroot --- / true # unknown option test $? = 125 || fail=1 -# Note chroot("/") succeeds for non-root users on some systems, but not all, -# however we avoid the chroot() with "/" to have common behavior. -chroot / sh -c 'exit 2' # exit status propagation -test $? = 2 || fail=1 -chroot / . # invalid command -test $? = 126 || fail=1 -chroot / no_such # no such command -test $? = 127 || fail=1 +# chroot("/") succeeds for non-root users on some systems, but not all. +if chroot / true ; then + can_chroot_root=1 + chroot / sh -c 'exit 2' # exit status propagation + test $? = 2 || fail=1 + chroot / . # invalid command + test $? = 126 || fail=1 + chroot / no_such # no such command + test $? = 127 || fail=1 +else + test $? = 125 || fail=1 +fi # Ensure that --skip-chdir fails with a non-"/" argument. cat <<\EOF > exp || framework_failure_ @@ -47,17 +51,19 @@ chroot --skip-chdir . env pwd >out 2>err && fail=1 compare /dev/null out || fail=1 compare exp err || fail=1 -# Ensure we don't chroot("/") when NEWROOT is old "/". -ln -s / isroot || framework_failure_ -for dir in '/' '/.' '/../' isroot; do - # Verify that chroot(1) succeeds and performs chdir("/") - # (chroot(1) of coreutils-8.23 failed to run the latter). - curdir=$(chroot "$dir" env pwd) || fail=1 - test "$curdir" = '/' || fail=1 - - # Test the "--skip-chdir" option. - curdir=$(chroot --skip-chdir "$dir" env pwd) || fail=1 - test "$curdir" = '/' && fail=1 -done +# Ensure we chdir("/") appropriately when NEWROOT is old "/". +if test "$can_chroot_root"; then + ln -s / isroot || framework_failure_ + for dir in '/' '/.' '/../' isroot; do + # Verify that chroot(1) succeeds and performs chdir("/") + # (chroot(1) of coreutils-8.23 failed to run the latter). + curdir=$(chroot "$dir" env pwd) || fail=1 + test "$curdir" = '/' || fail=1 + + # Test the "--skip-chdir" option. + curdir=$(chroot --skip-chdir "$dir" env pwd) || fail=1 + test "$curdir" = '/' && fail=1 + done +fi Exit $fail -- 1.7.7.6 --------------060208050305040905030306-- From debbugs-submit-bounces@debbugs.gnu.org Wed Oct 15 17:55:18 2014 Received: (at 18736) by debbugs.gnu.org; 15 Oct 2014 21:55:18 +0000 Received: from localhost ([127.0.0.1]:44944 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1XeWXN-0003p9-RA for submit@debbugs.gnu.org; Wed, 15 Oct 2014 17:55:18 -0400 Received: from mout.kundenserver.de ([212.227.126.131]:60067) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1XeWXK-0003oz-QZ for 18736@debbugs.gnu.org; Wed, 15 Oct 2014 17:55:15 -0400 Received: from [192.168.1.10] (pD9565569.dip0.t-ipconnect.de [217.86.85.105]) by mrelayeu.kundenserver.de (node=mreue007) with ESMTP (Nemesis) id 0MBv9P-1XUJNw40sp-00AomC; Wed, 15 Oct 2014 23:55:12 +0200 Message-ID: <543EED3F.7000405@bernhard-voelker.de> Date: Wed, 15 Oct 2014 23:55:11 +0200 From: Bernhard Voelker User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.1.0 MIME-Version: 1.0 To: =?windows-1252?Q?P=E1draig_Brady?= , Rogier Subject: Re: bug#18736: chroot regression - chroot avoids the chroot() call too eagerly. References: <3341391.OsvZrqhJ5a@wiske> <543EAC18.9040206@draigBrady.com> In-Reply-To: <543EAC18.9040206@draigBrady.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit X-Provags-ID: V02:K0:wcuNTYiMIRNBACjMcCKUvLIt36Un1jb2GvM2iSKSrhC 7ZFfeuTT8oavRD6+4d/xncwByF6sP6XGqNTye2epCTuyIOVMwm GP0JOA44UkxEonr5vLpFfTvL/J0AGyh+lrVxpbzxqAKiPldAdS jb2pdlTbtTWlGq/5MaIqJmLA91FZwD6tNPgJjmcloa7XiCFzNI 8jptFi2E9tSFBjV6ZLPSqSnpSXhCpvNKpiSk7YCX3MLwdGDN73 8kAP6G2miwyWjQdCHtkOYbkGmfnrdydHe697f7PQPOxONZdgk5 8bCyT5hL3+kSfAFeWPDFeRO1TCnZDBAxIANvCi2nP3PDLuSwzq yMfrI25xniwORD0KTHihSOaJsWeeslQaqLRt1SCke X-UI-Out-Filterresults: notjunk:1; X-Spam-Score: 0.0 (/) X-Debbugs-Envelope-To: 18736 Cc: 18736@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: 0.0 (/) On 10/15/2014 07:17 PM, Pádraig Brady wrote: > I agree with your analysis and that we should revert > to the previous behavior here, which is done in > the attached patch. Hi Padraig, I also agree that chroot(1) should chroot(2) in such a case, but wouldn't be the obvious fix to STREQ() the canonicalized DIR against "/" rather than reverting the whole change - something like the following? Have a nice day, Berny diff --git a/src/chroot.c b/src/chroot.c index 171ced9..7f60106 100644 --- a/src/chroot.c +++ b/src/chroot.c @@ -175,7 +175,13 @@ is_root (const char* dir) error (EXIT_CANCELED, errno, _("failed to get attributes of %s"), quote (dir)); - return SAME_INODE (root_ino, arg_st); + if (! SAME_INODE (root_ino, arg_st)) + return false; + + char *resolved = canonicalize_file_name (dir); + bool is_res_root = resolved && STREQ ("/", resolved); + free (resolved); + return is_res_root; } void -- 1.8.4.5 From debbugs-submit-bounces@debbugs.gnu.org Wed Oct 15 19:14:46 2014 Received: (at 18736) by debbugs.gnu.org; 15 Oct 2014 23:14:46 +0000 Received: from localhost ([127.0.0.1]:44978 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1XeXmH-0005tU-Fv for submit@debbugs.gnu.org; Wed, 15 Oct 2014 19:14:45 -0400 Received: from mx1.redhat.com ([209.132.183.28]:43373) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1XeXmF-0005tK-7T for 18736@debbugs.gnu.org; Wed, 15 Oct 2014 19:14:44 -0400 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s9FNEc9Q010501 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Wed, 15 Oct 2014 19:14:39 -0400 Received: from [10.36.116.33] (ovpn-116-33.ams2.redhat.com [10.36.116.33]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s9FNEZXc026805 (version=TLSv1/SSLv3 cipher=DHE-RSA-CAMELLIA256-SHA bits=256 verify=NO); Wed, 15 Oct 2014 19:14:37 -0400 Message-ID: <543EFFDB.10504@draigBrady.com> Date: Thu, 16 Oct 2014 00:14:35 +0100 From: =?windows-1252?Q?P=E1draig_Brady?= User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130110 Thunderbird/17.0.2 MIME-Version: 1.0 To: Bernhard Voelker Subject: Re: bug#18736: chroot regression - chroot avoids the chroot() call too eagerly. References: <3341391.OsvZrqhJ5a@wiske> <543EAC18.9040206@draigBrady.com> <543EED3F.7000405@bernhard-voelker.de> In-Reply-To: <543EED3F.7000405@bernhard-voelker.de> X-Enigmail-Version: 1.6 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit X-Scanned-By: MIMEDefang 2.68 on 10.5.11.26 X-Spam-Score: -5.0 (-----) X-Debbugs-Envelope-To: 18736 Cc: 18736@debbugs.gnu.org, Rogier X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -5.0 (-----) On 10/15/2014 10:55 PM, Bernhard Voelker wrote: > On 10/15/2014 07:17 PM, Pádraig Brady wrote: >> I agree with your analysis and that we should revert >> to the previous behavior here, which is done in >> the attached patch. > > Hi Padraig, > > I also agree that chroot(1) should chroot(2) in such a case, but wouldn't > be the obvious fix to STREQ() the canonicalized DIR against "/" rather > than reverting the whole change - something like the following? > > Have a nice day, > Berny > > diff --git a/src/chroot.c b/src/chroot.c > index 171ced9..7f60106 100644 > --- a/src/chroot.c > +++ b/src/chroot.c > @@ -175,7 +175,13 @@ is_root (const char* dir) > error (EXIT_CANCELED, errno, _("failed to get attributes of %s"), > quote (dir)); > > - return SAME_INODE (root_ino, arg_st); > + if (! SAME_INODE (root_ino, arg_st)) > + return false; > + > + char *resolved = canonicalize_file_name (dir); > + bool is_res_root = resolved && STREQ ("/", resolved); > + free (resolved); > + return is_res_root; > } Yes I considered that and it should work for this case. BTW the inode check would then be moot right? Doing that would give better performance for --userspec=... --skip-chdir and provide consistency for non root `chroot /` across platforms. I'm worried though there are other edge cases we've not considered, and the benefits above are not worth the risk? Pádraig. From debbugs-submit-bounces@debbugs.gnu.org Wed Oct 15 20:05:30 2014 Received: (at 18736) by debbugs.gnu.org; 16 Oct 2014 00:05:30 +0000 Received: from localhost ([127.0.0.1]:45004 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1XeYZN-0007AU-8y for submit@debbugs.gnu.org; Wed, 15 Oct 2014 20:05:30 -0400 Received: from mail3.vodafone.ie ([213.233.128.45]:48905) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1XeYZI-0007AK-FD for 18736@debbugs.gnu.org; Wed, 15 Oct 2014 20:05:26 -0400 X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: AiMCAAELP1RtTy5R/2dsb2JhbAANToNhUwWIYblBiWSHUQKBJgGEfwEBAQR5EAsNAQMDAQIBCRYPCQMCAQIBPQgGCgMBBQIBARaIKQOycpYFAQEBAQEBAQEBAQEBAQEBAQEBAQEBF40MAYMwEQcJhEIBBIYtiTaEKIFQaIhCPIYlhzeCVYYMgWprgkoBAQE Received: from unknown (HELO [192.168.1.43]) ([109.79.46.81]) by mail3.vodafone.ie with ESMTP; 16 Oct 2014 01:05:22 +0100 Message-ID: <543F0BC1.5010609@draigBrady.com> Date: Thu, 16 Oct 2014 01:05:21 +0100 From: =?windows-1252?Q?P=E1draig_Brady?= User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130110 Thunderbird/17.0.2 MIME-Version: 1.0 To: Bernhard Voelker Subject: Re: bug#18736: chroot regression - chroot avoids the chroot() call too eagerly. References: <3341391.OsvZrqhJ5a@wiske> <543EAC18.9040206@draigBrady.com> <543EED3F.7000405@bernhard-voelker.de> <543EFFDB.10504@draigBrady.com> In-Reply-To: <543EFFDB.10504@draigBrady.com> X-Enigmail-Version: 1.6 Content-Type: multipart/mixed; boundary="------------080609070607020208070700" X-Spam-Score: 0.0 (/) X-Debbugs-Envelope-To: 18736 Cc: 18736@debbugs.gnu.org, Rogier X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: 0.0 (/) This is a multi-part message in MIME format. --------------080609070607020208070700 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit On 10/16/2014 12:14 AM, Pádraig Brady wrote: > On 10/15/2014 10:55 PM, Bernhard Voelker wrote: >> On 10/15/2014 07:17 PM, Pádraig Brady wrote: >>> I agree with your analysis and that we should revert >>> to the previous behavior here, which is done in >>> the attached patch. >> >> Hi Padraig, >> >> I also agree that chroot(1) should chroot(2) in such a case, but wouldn't >> be the obvious fix to STREQ() the canonicalized DIR against "/" rather >> than reverting the whole change - something like the following? >> >> Have a nice day, >> Berny >> >> diff --git a/src/chroot.c b/src/chroot.c >> index 171ced9..7f60106 100644 >> --- a/src/chroot.c >> +++ b/src/chroot.c >> @@ -175,7 +175,13 @@ is_root (const char* dir) >> error (EXIT_CANCELED, errno, _("failed to get attributes of %s"), >> quote (dir)); >> >> - return SAME_INODE (root_ino, arg_st); >> + if (! SAME_INODE (root_ino, arg_st)) >> + return false; >> + >> + char *resolved = canonicalize_file_name (dir); >> + bool is_res_root = resolved && STREQ ("/", resolved); >> + free (resolved); >> + return is_res_root; >> } > > Yes I considered that and it should work for this case. > BTW the inode check would then be moot right? > > Doing that would give better performance for --userspec=... --skip-chdir > and provide consistency for non root `chroot /` across platforms. > I'm worried though there are other edge cases we've not considered, > and the benefits above are not worth the risk? I suppose a compromise is to keep most of the better performance for: chroot --userspec=... --skip-chdir / but do the chroot(2) unconditionally. Updated patch is attached. thanks, Pádraig. --------------080609070607020208070700 Content-Type: text/x-patch; name="chroot-always.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="chroot-always.patch" >From d520929586ee2063d48359aaaef8f28807604cae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A1draig=20Brady?= Date: Wed, 15 Oct 2014 18:08:42 +0100 Subject: [PATCH] chroot: call chroot() unconditionally to handle bind mounted "/" * src/chroot.c (is_root): Adjust to compare canonicalized paths rather than inodes, to handle (return false in) the case where we have a tree that is constructed by first bind mounting "/" (thus having the same inode). (main): Unconditionally call chroot() because it's safer and of minimal performance benefit to avoid in this case. This will cause inconsistency with some platforms not allowing `chroot / true` for non root users. * tests/misc/chroot-fail.sh: Adjust appropriately. * NEWS: Mention the bug fixes. Fixes http://bugs.gnu.org/18736 --- NEWS | 11 ++++----- src/chroot.c | 29 +++++++++++---------------- tests/misc/chroot-fail.sh | 46 +++++++++++++++++++++++++------------------- 3 files changed, 43 insertions(+), 43 deletions(-) diff --git a/NEWS b/NEWS index 52332bd..5fbdc6a 100644 --- a/NEWS +++ b/NEWS @@ -7,6 +7,11 @@ GNU coreutils NEWS -*- outline -*- dd supports more robust SIGINFO/SIGUSR1 handling for outputting statistics. Previously those signals may have inadvertently terminated the process. + chroot again calls chroot(DIR) and chdir("/"), even if DIR is "/". + This handles separate bind mounted "/" trees, and environments + depending on the implicit chdir("/"). + [bugs introduced in coreutils-8.23] + cp no longer issues an incorrect warning about directory hardlinks when a source directory is specified multiple times. Now, consistent with other file types, a warning is issued for source directories with duplicate names, @@ -25,12 +30,6 @@ GNU coreutils NEWS -*- outline -*- dd accepts a new status=progress level to print data transfer statistics on stderr approximately every second. -** Changes in behavior - - chroot changes the current directory to "/" in again - unless the above new - --skip-chdir option is specified. - [bug introduced in coreutils-8.23] - ** Improvements cp,install,mv will convert smaller runs of NULs in the input to holes, diff --git a/src/chroot.c b/src/chroot.c index 171ced9..3aacafa 100644 --- a/src/chroot.c +++ b/src/chroot.c @@ -162,20 +162,17 @@ parse_additional_groups (char const *groups, GETGROUPS_T **pgids, return ret; } +/* Return whether the passed path is equivalent to "/". + Note we don't compare against get_root_dev_ino() as "/" + could be bind mounted to a separate location. */ + static bool is_root (const char* dir) { - struct dev_ino root_ino; - if (! get_root_dev_ino (&root_ino)) - error (EXIT_CANCELED, errno, _("failed to get attributes of %s"), - quote ("/")); - - struct stat arg_st; - if (stat (dir, &arg_st) == -1) - error (EXIT_CANCELED, errno, _("failed to get attributes of %s"), - quote (dir)); - - return SAME_INODE (root_ino, arg_st); + char *resolved = canonicalize_file_name (dir); + bool is_res_root = resolved && STREQ ("/", resolved); + free (resolved); + return is_res_root; } void @@ -291,8 +288,6 @@ main (int argc, char **argv) usage (EXIT_CANCELED); } - /* Only do chroot specific actions if actually changing root. - The main difference here is that we don't change working dir. */ if (! is_oldroot) { /* We have to look up users and groups twice. @@ -328,12 +323,12 @@ main (int argc, char **argv) n_gids = ngroups; } #endif - - if (chroot (newroot) != 0) - error (EXIT_CANCELED, errno, _("cannot change root directory to %s"), - newroot); } + if (chroot (newroot) != 0) + error (EXIT_CANCELED, errno, _("cannot change root directory to %s"), + newroot); + if (! skip_chdir && chdir ("/")) error (EXIT_CANCELED, errno, _("cannot chdir to root directory")); diff --git a/tests/misc/chroot-fail.sh b/tests/misc/chroot-fail.sh index 82ae23c..75f724a 100755 --- a/tests/misc/chroot-fail.sh +++ b/tests/misc/chroot-fail.sh @@ -29,14 +29,18 @@ test $? = 125 || fail=1 chroot --- / true # unknown option test $? = 125 || fail=1 -# Note chroot("/") succeeds for non-root users on some systems, but not all, -# however we avoid the chroot() with "/" to have common behavior. -chroot / sh -c 'exit 2' # exit status propagation -test $? = 2 || fail=1 -chroot / . # invalid command -test $? = 126 || fail=1 -chroot / no_such # no such command -test $? = 127 || fail=1 +# chroot("/") succeeds for non-root users on some systems, but not all. +if chroot / true ; then + can_chroot_root=1 + chroot / sh -c 'exit 2' # exit status propagation + test $? = 2 || fail=1 + chroot / . # invalid command + test $? = 126 || fail=1 + chroot / no_such # no such command + test $? = 127 || fail=1 +else + test $? = 125 || fail=1 +fi # Ensure that --skip-chdir fails with a non-"/" argument. cat <<\EOF > exp || framework_failure_ @@ -47,17 +51,19 @@ chroot --skip-chdir . env pwd >out 2>err && fail=1 compare /dev/null out || fail=1 compare exp err || fail=1 -# Ensure we don't chroot("/") when NEWROOT is old "/". -ln -s / isroot || framework_failure_ -for dir in '/' '/.' '/../' isroot; do - # Verify that chroot(1) succeeds and performs chdir("/") - # (chroot(1) of coreutils-8.23 failed to run the latter). - curdir=$(chroot "$dir" env pwd) || fail=1 - test "$curdir" = '/' || fail=1 - - # Test the "--skip-chdir" option. - curdir=$(chroot --skip-chdir "$dir" env pwd) || fail=1 - test "$curdir" = '/' && fail=1 -done +# Ensure we chdir("/") appropriately when NEWROOT is old "/". +if test "$can_chroot_root"; then + ln -s / isroot || framework_failure_ + for dir in '/' '/.' '/../' isroot; do + # Verify that chroot(1) succeeds and performs chdir("/") + # (chroot(1) of coreutils-8.23 failed to run the latter). + curdir=$(chroot "$dir" env pwd) || fail=1 + test "$curdir" = '/' || fail=1 + + # Test the "--skip-chdir" option. + curdir=$(chroot --skip-chdir "$dir" env pwd) || fail=1 + test "$curdir" = '/' && fail=1 + done +fi Exit $fail -- 1.7.7.6 --------------080609070607020208070700-- From debbugs-submit-bounces@debbugs.gnu.org Thu Oct 16 03:38:14 2014 Received: (at 18736) by debbugs.gnu.org; 16 Oct 2014 07:38:14 +0000 Received: from localhost ([127.0.0.1]:45220 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1XefdV-0003Qx-Nv for submit@debbugs.gnu.org; Thu, 16 Oct 2014 03:38:14 -0400 Received: from mout.kundenserver.de ([212.227.126.131]:60314) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1XefdS-0003Qn-Je for 18736@debbugs.gnu.org; Thu, 16 Oct 2014 03:38:11 -0400 Received: from [10.0.4.15] (gw.camline.com [62.153.148.194]) by mrelayeu.kundenserver.de (node=mreue005) with ESMTP (Nemesis) id 0M4Vnk-1YNp4m0OlQ-00ygFJ; Thu, 16 Oct 2014 09:38:08 +0200 Message-ID: <543F75DF.6000600@bernhard-voelker.de> Date: Thu, 16 Oct 2014 09:38:07 +0200 From: Bernhard Voelker User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.1.0 MIME-Version: 1.0 To: =?windows-1252?Q?P=E1draig_Brady?= Subject: Re: bug#18736: chroot regression - chroot avoids the chroot() call too eagerly. References: <3341391.OsvZrqhJ5a@wiske> <543EAC18.9040206@draigBrady.com> <543EED3F.7000405@bernhard-voelker.de> <543EFFDB.10504@draigBrady.com> <543F0BC1.5010609@draigBrady.com> In-Reply-To: <543F0BC1.5010609@draigBrady.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 8bit X-Provags-ID: V02:K0:0r3e1+lOqUsEEBHzGXjbQcSiV+kdGI6ctk9UcedGSOJ uJhtGgYmqxp80w8CTsQYHACcXneqKl0p3b7yQPgc5PnlhO4wEE mQrHoXVC9J1t/MbUux89ZQAColJbJCMWPuJHWo2AMO6x4pErYO tHZGMlPGuFu8MiflNxBcevpl3nVr95ttyXnI+JiqyAUlXUxpLp 2ds/eX5MJbMkVAnTJlT5+oXJfRAYKet9hnoceGduPjZ6iPe616 7cQgbbVlHl+1KQYYWvx8BmrBlsIaEiJ84m1CGE2IseN/qzSj4U yy+nC/9Kd2h6g74YG6Vc1uasUEkjbJ42Yt1w5mTjRMohjA9uJ5 QxJgpXVu73s0oq/LjYj/Ols6PcWtXhQ7DYGWq2F3n X-UI-Out-Filterresults: notjunk:1; X-Spam-Score: 0.0 (/) X-Debbugs-Envelope-To: 18736 Cc: 18736@debbugs.gnu.org, Rogier X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: 0.0 (/) Hi Padraig, On 10/16/2014 02:05 AM, Pádraig Brady wrote: > From d520929586ee2063d48359aaaef8f28807604cae Mon Sep 17 00:00:00 2001 > From: =?UTF-8?q?P=C3=A1draig=20Brady?= > Date: Wed, 15 Oct 2014 18:08:42 +0100 > Subject: [PATCH] chroot: call chroot() unconditionally to handle bind mounted > "/" > > * src/chroot.c (is_root): Adjust to compare canonicalized paths > rather than inodes, to handle (return false in) the case where > we have a tree that is constructed by first bind mounting "/" > (thus having the same inode). > (main): Unconditionally call chroot() because it's safer > and of minimal performance benefit to avoid in this case. > This will cause inconsistency with some platforms > not allowing `chroot / true` for non root users. I'm not sure that introducing the inconsistency again is the right way to go, although I don't have a strong preference (40:60): I'd probably go with the smaller change in is_root() and wait for other edge cases to come (which I think is improbable). > diff --git a/tests/misc/chroot-fail.sh b/tests/misc/chroot-fail.sh > index 82ae23c..75f724a 100755 > --- a/tests/misc/chroot-fail.sh > +++ b/tests/misc/chroot-fail.sh > @@ -29,14 +29,18 @@ test $? = 125 || fail=1 > chroot --- / true # unknown option > test $? = 125 || fail=1 > > -# Note chroot("/") succeeds for non-root users on some systems, but not all, > -# however we avoid the chroot() with "/" to have common behavior. > -chroot / sh -c 'exit 2' # exit status propagation > -test $? = 2 || fail=1 > -chroot / . # invalid command > -test $? = 126 || fail=1 > -chroot / no_such # no such command > -test $? = 127 || fail=1 > +# chroot("/") succeeds for non-root users on some systems, but not all. > +if chroot / true ; then > + can_chroot_root=1 > + chroot / sh -c 'exit 2' # exit status propagation > + test $? = 2 || fail=1 > + chroot / . # invalid command > + test $? = 126 || fail=1 > + chroot / no_such # no such command > + test $? = 127 || fail=1 > +else > + test $? = 125 || fail=1 > +fi > > # Ensure that --skip-chdir fails with a non-"/" argument. > cat <<\EOF > exp || framework_failure_ > @@ -47,17 +51,19 @@ chroot --skip-chdir . env pwd >out 2>err && fail=1 > compare /dev/null out || fail=1 > compare exp err || fail=1 > > -# Ensure we don't chroot("/") when NEWROOT is old "/". > -ln -s / isroot || framework_failure_ > -for dir in '/' '/.' '/../' isroot; do > - # Verify that chroot(1) succeeds and performs chdir("/") > - # (chroot(1) of coreutils-8.23 failed to run the latter). > - curdir=$(chroot "$dir" env pwd) || fail=1 > - test "$curdir" = '/' || fail=1 > - > - # Test the "--skip-chdir" option. > - curdir=$(chroot --skip-chdir "$dir" env pwd) || fail=1 > - test "$curdir" = '/' && fail=1 > -done > +# Ensure we chdir("/") appropriately when NEWROOT is old "/". > +if test "$can_chroot_root"; then > + ln -s / isroot || framework_failure_ > + for dir in '/' '/.' '/../' isroot; do > + # Verify that chroot(1) succeeds and performs chdir("/") > + # (chroot(1) of coreutils-8.23 failed to run the latter). > + curdir=$(chroot "$dir" env pwd) || fail=1 > + test "$curdir" = '/' || fail=1 > + > + # Test the "--skip-chdir" option. > + curdir=$(chroot --skip-chdir "$dir" env pwd) || fail=1 > + test "$curdir" = '/' && fail=1 > + done > +fi > > Exit $fail > -- 1.7.7.6 Minor nit: it's better to initialize 'can_chroot_root' here. Thanks & have a nice day, Berny From debbugs-submit-bounces@debbugs.gnu.org Thu Oct 16 06:02:19 2014 Received: (at 18736) by debbugs.gnu.org; 16 Oct 2014 10:02:19 +0000 Received: from localhost ([127.0.0.1]:45270 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1Xehsw-00077l-BI for submit@debbugs.gnu.org; Thu, 16 Oct 2014 06:02:19 -0400 Received: from mx1.redhat.com ([209.132.183.28]:19245) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1Xehst-00077b-HX for 18736@debbugs.gnu.org; Thu, 16 Oct 2014 06:02:16 -0400 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s9GA2AZj016782 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Thu, 16 Oct 2014 06:02:10 -0400 Received: from [10.36.116.111] (ovpn-116-111.ams2.redhat.com [10.36.116.111]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s9GA28or028552 (version=TLSv1/SSLv3 cipher=DHE-RSA-CAMELLIA256-SHA bits=256 verify=NO); Thu, 16 Oct 2014 06:02:09 -0400 Message-ID: <543F979F.2030206@draigBrady.com> Date: Thu, 16 Oct 2014 11:02:07 +0100 From: =?windows-1252?Q?P=E1draig_Brady?= User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130110 Thunderbird/17.0.2 MIME-Version: 1.0 To: Bernhard Voelker Subject: Re: bug#18736: chroot regression - chroot avoids the chroot() call too eagerly. References: <3341391.OsvZrqhJ5a@wiske> <543EAC18.9040206@draigBrady.com> <543EED3F.7000405@bernhard-voelker.de> <543EFFDB.10504@draigBrady.com> <543F0BC1.5010609@draigBrady.com> <543F75DF.6000600@bernhard-voelker.de> In-Reply-To: <543F75DF.6000600@bernhard-voelker.de> X-Enigmail-Version: 1.6 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit X-Scanned-By: MIMEDefang 2.68 on 10.5.11.26 X-Spam-Score: -5.0 (-----) X-Debbugs-Envelope-To: 18736 Cc: 18736@debbugs.gnu.org, Rogier X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -5.0 (-----) On 10/16/2014 08:38 AM, Bernhard Voelker wrote: > Hi Padraig, > > On 10/16/2014 02:05 AM, Pádraig Brady wrote: >> From d520929586ee2063d48359aaaef8f28807604cae Mon Sep 17 00:00:00 2001 >> From: =?UTF-8?q?P=C3=A1draig=20Brady?= >> Date: Wed, 15 Oct 2014 18:08:42 +0100 >> Subject: [PATCH] chroot: call chroot() unconditionally to handle bind mounted >> "/" >> >> * src/chroot.c (is_root): Adjust to compare canonicalized paths >> rather than inodes, to handle (return false in) the case where >> we have a tree that is constructed by first bind mounting "/" >> (thus having the same inode). >> (main): Unconditionally call chroot() because it's safer >> and of minimal performance benefit to avoid in this case. >> This will cause inconsistency with some platforms >> not allowing `chroot / true` for non root users. > > I'm not sure that introducing the inconsistency again is the right > way to go, although I don't have a strong preference (40:60): > I'd probably go with the smaller change in is_root() and wait for > other edge cases to come (which I think is improbable). I don't have a strong preference either but would be (60:40) the other way. In my mind they comes under the "it's better to ask forgiveness than permission" idea. I.E. it's better to invoke lower layer logic where possible, rather than adding higher level logic. Given it's more risky to avoid the chroot() I'd be inclined to wait until there were complaints about the inconsistent behavior rather than the other way around. >> diff --git a/tests/misc/chroot-fail.sh b/tests/misc/chroot-fail.sh > Minor nit: it's better to initialize 'can_chroot_root' here. Good point. Done. thanks, Pádraig. From debbugs-submit-bounces@debbugs.gnu.org Thu Oct 16 06:30:56 2014 Received: (at 18736) by debbugs.gnu.org; 16 Oct 2014 10:30:57 +0000 Received: from localhost ([127.0.0.1]:45284 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1XeiKe-0007ri-H0 for submit@debbugs.gnu.org; Thu, 16 Oct 2014 06:30:56 -0400 Received: from mout.kundenserver.de ([212.227.126.130]:53344) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1XeiKb-0007rZ-TX for 18736@debbugs.gnu.org; Thu, 16 Oct 2014 06:30:54 -0400 Received: from [10.0.4.15] (gw.camline.com [62.153.148.194]) by mrelayeu.kundenserver.de (node=mreue006) with ESMTP (Nemesis) id 0M2TKr-1YU41x1MlJ-00sPhJ; Thu, 16 Oct 2014 12:30:52 +0200 Message-ID: <543F9E5B.9010904@bernhard-voelker.de> Date: Thu, 16 Oct 2014 12:30:51 +0200 From: Bernhard Voelker User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.1.0 MIME-Version: 1.0 To: =?windows-1252?Q?P=E1draig_Brady?= Subject: Re: bug#18736: chroot regression - chroot avoids the chroot() call too eagerly. References: <3341391.OsvZrqhJ5a@wiske> <543EAC18.9040206@draigBrady.com> <543EED3F.7000405@bernhard-voelker.de> <543EFFDB.10504@draigBrady.com> <543F0BC1.5010609@draigBrady.com> <543F75DF.6000600@bernhard-voelker.de> <543F979F.2030206@draigBrady.com> In-Reply-To: <543F979F.2030206@draigBrady.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 8bit X-Provags-ID: V02:K0:OlQmabRV+sJLJEpjOZidSrT7yfLLLOixrH/9Uk9j6h2 XCw9kbfgdaUxEy20io96g88Bh+sFNmcEGrkroSBeJgViEtADkk ZievOc7sF1SqmM+6xkvXtU/v3XfAd6LpdoYIKX9ah1UFGmMgKV rIgd2PdSSyEcsRxKNcO/0d47ITyScdTaoJ3ehYrLfwByrRzWna z8jLOlLsYe8B5ZTC3dnQ1cHV8hLjzkZjB/oY6a5L/fGB4RR3o7 ZUnF0mz6SnLgFxspW5dwS2QybHSUpzcWVGl7Xv8hTDCrNqzD/R 6EiNMBj/c2begnK7KdP7G26UhkePRflaFXKiUPM0BLrcK7Axha 8zDJ2CX7wiInpGNdRXsZvRhG+6QLaFN0mvMzpB3qk X-UI-Out-Filterresults: notjunk:1; X-Spam-Score: 0.0 (/) X-Debbugs-Envelope-To: 18736 Cc: 18736@debbugs.gnu.org, Rogier X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: 0.0 (/) On 10/16/2014 12:02 PM, Pádraig Brady wrote: > I don't have a strong preference either but would be (60:40) the other way. > In my mind they comes under the "it's better to ask forgiveness than permission" idea. > I.E. it's better to invoke lower layer logic where possible, rather than > adding higher level logic. Given it's more risky to avoid the chroot() > I'd be inclined to wait until there were complaints about the inconsistent > behavior rather than the other way around. Another possibility would be to suppress issuing an error diagnostic for a chroot() failure when DIR is "/" ... Have a nice day, Berny From debbugs-submit-bounces@debbugs.gnu.org Thu Oct 16 20:40:36 2014 Received: (at control) by debbugs.gnu.org; 17 Oct 2014 00:40:36 +0000 Received: from localhost ([127.0.0.1]:46471 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1Xevau-0000cf-9u for submit@debbugs.gnu.org; Thu, 16 Oct 2014 20:40:36 -0400 Received: from mx1.redhat.com ([209.132.183.28]:7096) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1Xevaq-0000cV-Lw for control@debbugs.gnu.org; Thu, 16 Oct 2014 20:40:33 -0400 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s9H0eVdf011110 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Thu, 16 Oct 2014 20:40:31 -0400 Received: from [10.36.116.19] (ovpn-116-19.ams2.redhat.com [10.36.116.19]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s9H0eTte003145 (version=TLSv1/SSLv3 cipher=DHE-RSA-CAMELLIA256-SHA bits=256 verify=NO) for ; Thu, 16 Oct 2014 20:40:30 -0400 Message-ID: <5440657C.1010005@draigBrady.com> Date: Fri, 17 Oct 2014 01:40:28 +0100 From: =?UTF-8?B?UMOhZHJhaWcgQnJhZHk=?= User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130110 Thunderbird/17.0.2 MIME-Version: 1.0 To: GNU bug tracker automated control server X-Enigmail-Version: 1.6 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Scanned-By: MIMEDefang 2.68 on 10.5.11.26 X-Spam-Score: -3.0 (---) X-Debbugs-Envelope-To: control X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.0 (---) close 18736 stop From unknown Fri Aug 15 21:22:11 2025 Received: (at fakecontrol) by fakecontrolmessage; To: internal_control@debbugs.gnu.org From: Debbugs Internal Request Subject: Internal Control Message-Id: bug archived. Date: Fri, 14 Nov 2014 12:24:04 +0000 User-Agent: Fakemail v42.6.9 # This is a fake control message. # # The action: # bug archived. thanks # This fakemail brought to you by your local debbugs # administrator