diff options
| -rw-r--r-- | ChangeLog | 22 | ||||
| -rw-r--r-- | src/c99-to-c89.diff | 6 | ||||
| -rw-r--r-- | src/mv.c | 10 | ||||
| -rw-r--r-- | src/remove.c | 3 | ||||
| -rw-r--r-- | tests/Makefile.am | 4 | ||||
| -rw-r--r-- | tests/mv/Makefile.am | 1 | ||||
| -rwxr-xr-x | tests/mv/sticky-to-xpart | 70 |
7 files changed, 110 insertions, 6 deletions
@@ -1,5 +1,27 @@ 2007-10-05 Jim Meyering <meyering@redhat.com> + Make a failing cross-partition mv give a sensible diagnostic. + A cross-partition move of a file in a sticky tmpdir and owned by + another user would evoke an invalid diagnostic after copying it: + mv: cannot remove `x': Operation not permitted + Either of the following (mv.c, remove.c) changes would fix the bug by + itself. I think it's slightly better to use both; the added cost is + minimal: mv: an extra lstat-per-mv-cmdline-arg-that-goes-cross-partition, + rm: an extra lstat-per-unlink-that-fails-w/EPERM. + * src/remove.c (remove_entry): Also lstat the file upon EPERM. + * src/mv.c (rm_option_init): Initialize root_dev_ino just as is done + in rm, so that a cross-partition invoked remove.c:rm call works the + same way as one invoked from the command-line use of "rm". That + setting of root_dev_ino makes rm() do the equivalent of an additional + lstat for each argument, which in turn gives rm enough information to + issue the right diagnostic. + * tests/mv/sticky-to-xpart (version): New file. Test for the above. + * tests/mv/Makefile.am (TESTS): Add sticky-to-xpart. + Arrange for "make check-root" to run the new root-only test. + * tests/Makefile.am (tb): New target, to run the new root-only test. + (all_t): Add tb. + * src/c99-to-c89.diff: Adjust offsets. + Add PACKAGE_VERSION to TESTS_ENVIRONMENT via check.mk. * tests/check.mk (TESTS_ENVIRONMENT): Add PACKAGE_VERSION here, rather than in every Makefile.am that needs it. diff --git a/src/c99-to-c89.diff b/src/c99-to-c89.diff index 01e213beb..8a3737247 100644 --- a/src/c99-to-c89.diff +++ b/src/c99-to-c89.diff @@ -42,7 +42,7 @@ diff -upr src/remove.c src/remove.c if (!yesno ()) return RM_USER_DECLINED; -@@ -1533,6 +1537,7 @@ rm_1 (Dirstack_state *ds, char const *fi +@@ -1534,6 +1538,7 @@ rm_1 (Dirstack_state *ds, char const *fi return RM_ERROR; } @@ -50,7 +50,7 @@ diff -upr src/remove.c src/remove.c struct stat st; cache_stat_init (&st); cycle_check_init (&ds->cycle_check_state); -@@ -1555,6 +1560,7 @@ rm_1 (Dirstack_state *ds, char const *fi +@@ -1556,6 +1561,7 @@ rm_1 (Dirstack_state *ds, char const *fi AD_push_initial (ds); AD_INIT_OTHER_MEMBERS (); @@ -58,7 +58,7 @@ diff -upr src/remove.c src/remove.c enum RM_status status = remove_entry (AT_FDCWD, ds, filename, DT_UNKNOWN, &st, x); if (status == RM_NONEMPTY_DIR) -@@ -1573,6 +1579,8 @@ rm_1 (Dirstack_state *ds, char const *fi +@@ -1574,6 +1580,8 @@ rm_1 (Dirstack_state *ds, char const *fi ds_clear (ds); return status; } @@ -32,6 +32,7 @@ #include "filenamecat.h" #include "quote.h" #include "remove.h" +#include "root-dev-ino.h" /* The official name of this program (e.g., no `g' prefix). */ #define PROGRAM_NAME "mv" @@ -92,7 +93,6 @@ static void rm_option_init (struct rm_options *x) { x->ignore_missing_files = false; - x->root_dev_ino = NULL; x->recursive = true; x->one_file_system = false; @@ -108,6 +108,14 @@ rm_option_init (struct rm_options *x) the initial working directory, in case one of those is a `.'-relative name. */ x->require_restore_cwd = true; + + { + static struct dev_ino dev_ino_buf; + x->root_dev_ino = get_root_dev_ino (&dev_ino_buf); + if (x->root_dev_ino == NULL) + error (EXIT_FAILURE, errno, _("failed to get attributes of %s"), + quote ("/")); + } } static void diff --git a/src/remove.c b/src/remove.c index ac1010953..3cbfe6683 100644 --- a/src/remove.c +++ b/src/remove.c @@ -1082,7 +1082,8 @@ remove_entry (int fd_cwd, Dirstack_state const *ds, char const *filename, if (! x->recursive || (cache_stat_ok (st) && !S_ISDIR (st->st_mode)) - || (errno == EACCES && is_nondir_lstat (fd_cwd, filename, st)) + || ((errno == EACCES || errno == EPERM) + && is_nondir_lstat (fd_cwd, filename, st)) ) { if (ignorable_missing (x, errno)) diff --git a/tests/Makefile.am b/tests/Makefile.am index 254efc470..af9328de2 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -50,7 +50,7 @@ SUBDIRS = \ uniq wc ## N O T E :: Please do not add new directories. -all_t = t1 t2 t3 t4 t5 t6 t7 t8 t9 ta +all_t = t1 t2 t3 t4 t5 t6 t7 t8 t9 ta tb .PHONY: check-root $(all_t) check-root: $(all_t) @@ -74,6 +74,8 @@ t9: cd cp && $(MAKE) check TESTS=cp-a-selinux ta: cd mkdir && $(MAKE) check TESTS=writable-under-readonly +tb: + cd mv && $(MAKE) check TESTS=sticky-to-xpart check-recursive: root-hint diff --git a/tests/mv/Makefile.am b/tests/mv/Makefile.am index 4fa09fbd2..ba5d41d98 100644 --- a/tests/mv/Makefile.am +++ b/tests/mv/Makefile.am @@ -17,6 +17,7 @@ # along with this program. If not, see <http://www.gnu.org/licenses/>. TESTS = \ + sticky-to-xpart \ hard-verbose \ backup-dir \ dir2dir \ diff --git a/tests/mv/sticky-to-xpart b/tests/mv/sticky-to-xpart new file mode 100755 index 000000000..04690d75b --- /dev/null +++ b/tests/mv/sticky-to-xpart @@ -0,0 +1,70 @@ +#!/bin/sh +# A cross-partition move of a file in a sticky tmpdir and owned by +# someone else would evoke an invalid diagnostic: +# mv: cannot remove `x': Operation not permitted +# Affects coreutils-6.0-6.9. + +# Copyright (C) 2007 Free Software Foundation, Inc. + +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. + +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. + +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. + +if test "$VERBOSE" = yes; then + set -x + mv --version +fi + +. $srcdir/../envvar-check +. $srcdir/../lang-default +PRIV_CHECK_ARG=require-root . $srcdir/../priv-check +. $srcdir/../test-lib.sh + +cleanup_() { rm -rf "$other_partition_tmpdir"; } +. "$abs_top_srcdir/tests/other-fs-tmpdir" + +# Set up to run a test where non-root user tries to move a root-owned +# file from a sticky tmpdir to a directory owned by that user on +# a different partition. + +mkdir t || framework_failure +chmod a=rwx,o+t t || framework_failure +echo > t/root-owned || framework_failure +chmod a+r t/root-owned || framework_failure +chown "$NON_ROOT_USERNAME" "$other_partition_tmpdir" || framework_failure + +# We have to allow $NON_ROOT_USERNAME access to ".". +chmod go+x . || framework_failure + +# Ensure that $NON_ROOT_USERNAME can access the required version of mv. +version=`setuidgid $NON_ROOT_USERNAME env PATH="$PATH" mv --version|sed -n '1s/.* //p'` +case $version in + $PACKAGE_VERSION) ;; + *) echo "$0: cannot access just-built mv as user $NON_ROOT_USERNAME" 1>&2 + fail=1 ;; +esac + +setuidgid $NON_ROOT_USERNAME env PATH="$PATH" \ + mv t/root-owned $other_partition_tmpdir 2> out-t && fail=1 + +# On some systems, we get `Not owner'. Convert it. +# On other systems (HPUX), we get `Permission denied'. Convert it, too. +onp='Operation not permitted' +sed "s/Not owner/$onp/;s/Permission denied/$onp/" out-t > out + +cat <<\EOF > exp +mv: cannot remove `t/root-owned': Operation not permitted +EOF + +compare out exp || fail=1 + +(exit $fail); exit $fail |