diff options
| -rw-r--r-- | NEWS | 8 | ||||
| -rw-r--r-- | THANKS.in | 3 | ||||
| -rw-r--r-- | src/copy.c | 19 | ||||
| -rw-r--r-- | tests/Makefile.am | 1 | ||||
| -rwxr-xr-x | tests/cp/nfs-removal-race | 70 |
5 files changed, 100 insertions, 1 deletions
@@ -11,6 +11,14 @@ GNU coreutils NEWS -*- outline -*- changed, the new group ID would be listed, even though it is not yet effective. [bug introduced in coreutils-8.1] + cp S D is no longer subject to a race: if an existing D were removed + between the initial stat and subsequent open-without-O_CREATE, cp would + fail with a confusing diagnostic saying that the destination, D, was not + found. Now, in this unusual case, it retries the open (but with O_CREATE), + and hence usually succeeds. With NFS attribute caching, the condition + was particularly easy to trigger, since there, the removal of D could + precede the initial stat. [This bug was present in "the beginning".] + ** New features fmt now accepts the --goal=WIDTH (-g) option. @@ -439,7 +439,7 @@ Minh Tran-Le tranle@intellicorp.com Morten Welinder terra@diku.dk Nao Nishijima nao.nishijima.xt@hitachi.com Neal H Walfield neal@cs.uml.edu -Neil Brown neilb@cse.unsw.edu.au +Neil F. Brown neilb@suse.de Nelson H. F. Beebe beebe@math.utah.edu Nick Estes debian@nickstoys.com Nick Graham nick.d.graham@gmail.com @@ -489,6 +489,7 @@ Phil Richards phil.richards@vf.vodafone.co.uk Philippe De Muyter phdm@macqel.be Philippe Schnoebelen Philippe.Schnoebelen@imag.fr Phillip Jones mouse@datastacks.com +Philipp Thomas pth@suse.de Piergiorgio Sartor sartor@sony.de Pieter Bowman bowman@math.utah.edu Piotr Gackiewicz gacek@intertele.pl diff --git a/src/copy.c b/src/copy.c index 844ebcd78..2558fea14 100644 --- a/src/copy.c +++ b/src/copy.c @@ -892,6 +892,8 @@ copy_reg (char const *src_name, char const *dst_name, if (*new_dst) { + open_with_O_CREAT:; + int open_flags = O_WRONLY | O_CREAT | O_BINARY; dest_desc = open (dst_name, open_flags | O_EXCL, dst_mode & ~omitted_permissions); @@ -942,6 +944,23 @@ copy_reg (char const *src_name, char const *dst_name, if (dest_desc < 0) { + /* If we've just failed due to ENOENT for an ostensibly preexisting + destination (*new_dst was 0), that's a bit of a contradiction/race: + the prior stat/lstat said the file existed (*new_dst was 0), yet + the subsequent open-existing-file failed with ENOENT. With NFS, + the race window is wider still, since its meta-data caching tends + to make the stat succeed for a just-removed remote file, while the + more-definitive initial open call will fail with ENOENT. When this + situation arises, we attempt to open again, but this time with + O_CREAT. Do this only when not in move-mode, since when handling + a cross-device move, we must never open an existing destination. */ + if (dest_errno == ENOENT && ! *new_dst && ! x->move_mode) + { + *new_dst = 1; + goto open_with_O_CREAT; + } + + /* Otherwise, it's an error. */ error (0, dest_errno, _("cannot create regular file %s"), quote (dst_name)); return_val = false; diff --git a/tests/Makefile.am b/tests/Makefile.am index 72717e318..ca190517b 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -349,6 +349,7 @@ TESTS = \ cp/link-no-deref \ cp/link-preserve \ cp/link-symlink \ + cp/nfs-removal-race \ cp/no-deref-link1 \ cp/no-deref-link2 \ cp/no-deref-link3 \ diff --git a/tests/cp/nfs-removal-race b/tests/cp/nfs-removal-race new file mode 100755 index 000000000..6a435b0c5 --- /dev/null +++ b/tests/cp/nfs-removal-race @@ -0,0 +1,70 @@ +#!/bin/sh +# Running cp S D on an NFS client while another client has just removed D +# would lead (w/coreutils-8.16 and earlier) to cp's initial stat call +# seeing (via stale NFS cache) that D exists, so that cp would then call +# open without the O_CREAT flag. Yet, the open must actually consult +# the server, which confesses that D has been deleted, thus causing the +# open call to fail with ENOENT. +# +# This test simulates that situation by intercepting stat for a nonexistent +# destination, D, and making the stat fill in the result struct for another +# file and return 0. +# +# This test is skipped on systems that lack LD_PRELOAD support; that's fine. +# Similarly, on a system that lacks <dlfcn.h> or __xstat, skipping it is fine. + +# Copyright (C) 2012 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/>. + +. "${srcdir=.}/init.sh"; path_prepend_ ../src +print_ver_ cp + +# Replace each stat call with a call to this wrapper. +cat > k.c <<'EOF' || framework_failure_ +#define _GNU_SOURCE +#include <sys/types.h> +#include <dlfcn.h> + +#define __xstat __xstat_orig + +#include <sys/stat.h> +#include <stddef.h> + +#undef __xstat + +int +__xstat (int ver, const char *path, struct stat *st) +{ + static int (*real_stat)(int ver, const char *path, struct stat *st) = NULL; + if (!real_stat) + real_stat = dlsym (RTLD_NEXT, "__xstat"); + /* When asked to stat nonexistent "d", + return results suggesting it exists. */ + return real_stat (ver, *path == 'd' && path[1] == 0 ? "d2" : path, st); +} +EOF + +# Then compile/link it: +$CC -shared -fPIC -O2 k.c -o k.so \ + || framework_failure_ 'failed to compile with -shared -fPIC' + +touch d2 || framework_failure_ +echo xyz > src || framework_failure_ + +# Finally, run the test: +LD_PRELOAD=./k.so cp src d || fail=1 + +compare src d || fail=1 +Exit $fail |