chroot: don't chdir() if not changing root

This allows chroot to be used as a light weight tool
to change user identification for a command,
while not changing the current working directory.
It also makes `chroot / true` consistently succeed on
all platforms for non root users.

* src/chroot.c (main): If the same root is specified. i.e. '/'
then don't change the current working directory, and avoid the
overhead of the other redundant calls.
* tests/misc/chroot-fail.sh: Remove failure guard previously
needed on some systems.  Also add an explicit case to ensure
we don't change directory.
* NEWS: Mention the change in behavior.

diff --git a/NEWS b/NEWS
index c204b680dca9781ccd846dcf997e2ef06c03265e..93e3a09806cfccfe453dad75cfb46c64887618f8 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -82,6 +82,9 @@ GNU coreutils NEWS                                    -*- outline -*-
 
 ** Changes in behavior
 
+  chroot with an argument of "/" no longer implicitly changes the current
+  directory to "/", allowing changing only user credentials for a command.
+
   ls with none of LS_COLORS or COLORTERM environment variables set,
   will now honor an empty or unknown TERM environment variable,
   and not output colors even with --colors=always.

diff --git a/src/chroot.c b/src/chroot.c
index 8b08b84ea196adaf77ecb3cd7cfce28b85e30132..a2debac1c57a898bffba7d80b86aa20753bfba12 100644 (file)
--- a/src/chroot.c
+++ b/src/chroot.c
@@ -225,21 +225,27 @@ main (int argc, char **argv)
       usage (EXIT_CANCELED);
     }
 
-  /* 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 the parsing in case IDs are different.  */
-  if (userspec)
-    ignore_value (parse_user_spec (userspec, &uid, &gid, NULL, NULL));
-  if (groups && *groups)
-    ignore_value (parse_additional_groups (groups, &out_gids, &n_gids, false));
-
-  if (chroot (argv[optind]) != 0)
-    error (EXIT_CANCELED, errno, _("cannot change root directory to %s"),
-           argv[optind]);
-
-  if (chdir ("/"))
-    error (EXIT_CANCELED, errno, _("cannot chdir to root directory"));
+  /* Only do chroot specific actions if actually changing root.
+     The main difference here is that we don't change working dir.  */
+  if (! STREQ (argv[optind], "/"))
+    {
+      /* 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.  */
+      if (userspec)
+        ignore_value (parse_user_spec (userspec, &uid, &gid, NULL, NULL));
+      if (groups && *groups)
+        ignore_value (parse_additional_groups (groups, &out_gids, &n_gids,
+                                               false));
+
+      if (chroot (argv[optind]) != 0)
+        error (EXIT_CANCELED, errno, _("cannot change root directory to %s"),
+               argv[optind]);
+
+      if (chdir ("/"))
+        error (EXIT_CANCELED, errno, _("cannot chdir to root directory"));
+    }
 
   if (argc == optind + 1)
     {

diff --git a/tests/misc/chroot-fail.sh b/tests/misc/chroot-fail.sh
index 11f03458c8b9b2db354e554c8fe48047236a26a9..56be8e23164d45338fd0fb454a47606590e78792 100755 (executable)
--- a/tests/misc/chroot-fail.sh
+++ b/tests/misc/chroot-fail.sh
@@ -28,16 +28,18 @@ test $? = 125 || fail=1
 chroot --- / true # unknown option
 test $? = 125 || fail=1
 
-# chroot("/") succeeds for non-root users on some systems, but not all.
-if chroot / true ; then
-  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
+# Note chroot("/") succeeds for non-root users on some systems, but not all,
+# however we avoid the chroot() with "/" to have common behvavior.
+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
+
+# Ensure we don't chdir("/") when not changing root
+# to allow only changing user ids for a command.
+curdir=$(chroot / env pwd) || fail=1
+test "$curdir" = '/' && fail=1
 
 Exit $fail