From d1fb62d16f4c1de66b6abc0e6f6561198a4ee04b Mon Sep 17 00:00:00 2001
From: =?latin1?q?P=E1draig=20Brady?= <P@draigBrady.com>
Date: Wed, 15 Oct 2014 18:08:42 +0100
Subject: 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
(cherry picked from commit d520929586ee2063d48359aaaef8f28807604cae)

Conflicts:
	NEWS
---
 src/chroot.c              | 29 ++++++++++++-----------------
 tests/misc/chroot-fail.sh | 46 ++++++++++++++++++++++++++--------------------
 2 files changed, 38 insertions(+), 37 deletions(-)

diff --git a/src/chroot.c b/src/chroot.c
index 3a3a247..f9472ac 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.12.1