testing/coreutils
SHA256
3
0
Fork 0
forked from pool/coreutils
Code Pull Requests Activity
684831c59a
coreutils/coreutils-race_in_cp.patch
Philipp Thomas 684831c59a - Two new upstream patches: 
* id and groups, when invoked with no user name argument, would
    print the default group ID listed in the password database, and
    sometimes that ID would be neither real nor effective.  For
    example, when run set-GID, or in a session for which the default
    group has just been 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".] (bnc#760926).

OBS-URL: https://build.opensuse.org/package/show/Base:System/coreutils?expand=0&rev=152
2012-05-07 12:25:45 +00:00

179 lines
7.2 KiB
Diff
Raw Blame History

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
View Git Blame Copy Permalink