40d9f66558
Fix bnc#760926 and add a second upstream patch for id OBS-URL: https://build.opensuse.org/request/show/116644 OBS-URL: https://build.opensuse.org/package/show/openSUSE:Factory/coreutils?expand=0&rev=77
179 lines
7.2 KiB
Diff
179 lines
7.2 KiB
Diff
commit ee9e43460f366406edff96b5abfb3ff33587e062
|
|
Author: Jim Meyering <meyering@redhat.com>
|
|
Date: Fri May 4 16:42:31 2012 +0200
|
|
|
|
cp: handle a race condition more sensibly
|
|
|
|
* src/copy.c (copy_reg): In a narrow race (stat sees dest, yet
|
|
open-without-O_CREAT fails with ENOENT), retry the open with O_CREAT.
|
|
* tests/cp/nfs-removal-race: New file.
|
|
* tests/Makefile.am (TESTS): Add it.
|
|
* NEWS (Bug fixes): Mention it.
|
|
Reported by Philipp Thomas and Neil F. Brown in
|
|
http://bugs.gnu.org/11100
|
|
|
|
Index: NEWS
|
|
===================================================================
|
|
--- NEWS.orig 2012-05-07 14:20:52.079810454 +0200
|
|
+++ NEWS 2012-05-07 14:21:35.566738140 +0200
|
|
@@ -9,6 +9,14 @@ GNU coreutils NEWS
|
|
changed, the new group ID would be listed, even though it is not
|
|
yet effective.
|
|
|
|
+ '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_CREAT, 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_CREAT),
|
|
+ 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".]
|
|
+
|
|
|
|
* Noteworthy changes in release 8.16 (2012-03-26) [stable]
|
|
|
|
Index: THANKS.in
|
|
===================================================================
|
|
--- THANKS.in.orig 2012-05-07 14:20:52.079810454 +0200
|
|
+++ THANKS.in 2012-05-07 14:20:52.087810257 +0200
|
|
@@ -439,7 +439,7 @@ Minh Tran-Le tran
|
|
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
|
|
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
|
|
Index: src/copy.c
|
|
===================================================================
|
|
--- src/copy.c.orig 2012-03-24 21:26:51.000000000 +0100
|
|
+++ src/copy.c 2012-05-07 14:20:52.087810257 +0200
|
|
@@ -889,6 +889,8 @@ copy_reg (char const *src_name, char con
|
|
|
|
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);
|
|
@@ -939,6 +941,23 @@ copy_reg (char const *src_name, char con
|
|
|
|
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;
|
|
Index: tests/Makefile.am
|
|
===================================================================
|
|
--- tests/Makefile.am.orig 2012-05-07 14:20:52.080810429 +0200
|
|
+++ tests/Makefile.am 2012-05-07 14:20:52.087810257 +0200
|
|
@@ -347,6 +347,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 \
|
|
Index: tests/cp/nfs-removal-race
|
|
===================================================================
|
|
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
|
|
+++ tests/cp/nfs-removal-race 2012-05-07 14:20:52.087810257 +0200
|
|
@@ -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
|