From unknown Mon Jun 23 07:50:00 2025 X-Loop: help-debbugs@gnu.org Subject: bug#25680: [PATCH] copy: Avoid race when creating hard link over recently-created file Resent-From: Mike Crowe Original-Sender: "Debbugs-submit" Resent-CC: bug-coreutils@gnu.org Resent-Date: Fri, 10 Feb 2017 19:50:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: report 25680 X-GNU-PR-Package: coreutils X-GNU-PR-Keywords: patch To: 25680@debbugs.gnu.org Cc: Mike Crowe X-Debbugs-Original-To: bug-coreutils@gnu.org Received: via spool by submit@debbugs.gnu.org id=B.148675615915287 (code B ref -1); Fri, 10 Feb 2017 19:50:02 +0000 Received: (at submit) by debbugs.gnu.org; 10 Feb 2017 19:49:19 +0000 Received: from localhost ([127.0.0.1]:35011 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ccHC3-0003yV-0d for submit@debbugs.gnu.org; Fri, 10 Feb 2017 14:49:19 -0500 Received: from eggs.gnu.org ([208.118.235.92]:55923) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ccHC2-0003yJ-9E for submit@debbugs.gnu.org; Fri, 10 Feb 2017 14:49:18 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ccHBv-0006HI-Nb for submit@debbugs.gnu.org; Fri, 10 Feb 2017 14:49:13 -0500 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on eggs.gnu.org X-Spam-Level: *** X-Spam-Status: No, score=3.3 required=5.0 tests=BAYES_50, RECEIVED_FROM_WINDOWS_HOST autolearn=disabled version=3.3.2 Received: from lists.gnu.org ([2001:4830:134:3::11]:41381) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1ccHBv-0006HC-Ke for submit@debbugs.gnu.org; Fri, 10 Feb 2017 14:49:11 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46893) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ccHBu-0006P1-Bv for bug-coreutils@gnu.org; Fri, 10 Feb 2017 14:49:11 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ccHBr-0006Gt-8O for bug-coreutils@gnu.org; Fri, 10 Feb 2017 14:49:10 -0500 Received: from relay101a.appriver.com ([207.97.230.14]:61971 helo=relay.appriver.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1ccHBr-0006Go-4P for bug-coreutils@gnu.org; Fri, 10 Feb 2017 14:49:07 -0500 X-Note-AR-ScanTimeLocal: 02/10/2017 2:19:00 PM X-Policy: brightsign.biz - brightsign.biz X-Policy: brightsign.biz - brightsign.biz X-Primary: mcrowe@brightsign.biz X-Note: This Email was scanned by AppRiver SecureTide X-Note: SecureTide Build: 9/27/2016 9:09:05 PM UTC X-Virus-Scan: V- X-Note-SnifferID: 0 X-Note: TCH-CT/SI:0-43/SG:1 2/10/2017 2:18:26 PM X-GBUdb-Analysis: 0, 213.210.30.29, Ugly c=0.454114 p=-1 Source Normal X-Signature-Violations: 0-0-0-5359-c X-Note: Spam Tests Failed: X-Country-Path: ->PRIVATE->United Kingdom->United States X-Note-Sending-IP: 213.210.30.29 X-Note-Reverse-DNS: elite.brightsigndigital.co.uk X-Note-Return-Path: mcrowe@brightsign.biz X-Note: User Rule Hits: X-Note: Global Rule Hits: G273 G274 G275 G276 G280 G281 G409 X-Note: Encrypt Rule Hits: X-Note: Mail Class: VALID X-Note: Headers Injected Received: from [213.210.30.29] (HELO elite.brightsign6) by relay.appriver.com (CommuniGate Pro SMTP 6.1.7) with ESMTPS id 112956592; Fri, 10 Feb 2017 14:19:00 -0500 Received: from chuckie.brightsign ([172.30.1.25] helo=chuckie) by elite.brightsign6 with esmtp (Exim 4.84_2) (envelope-from ) id 1ccGih-000BZ3-3R; Fri, 10 Feb 2017 19:18:59 +0000 Received: from mac by chuckie with local (Exim 4.84_2) (envelope-from ) id 1ccGih-0005fk-2e; Fri, 10 Feb 2017 19:18:59 +0000 From: Mike Crowe Date: Fri, 10 Feb 2017 19:18:08 +0000 Message-Id: <20170210191808.21739-1-mac@mcrowe.com> X-Mailer: git-send-email 2.12.0.rc0 X-detected-operating-system: by eggs.gnu.org: Windows 7 or 8 [fuzzy] X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6.x X-Received-From: 2001:4830:134:3::11 X-Spam-Score: -5.0 (-----) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -5.0 (-----) Ensure that create_hard_link is permitted to overwrite the destination file if --force is passed to cp. It is possible for the file to appear between checking and creating the hard link. This may happen multiple times, so also loop inside create_hard_link each time it fails. The race is made _much_ easier to reproduce by inserting a sleep(1) as the line immediately prior to the call to modified create_hard_link call in copy_internal. Steps to reproduce (assuming the aforementioned sleep has been added): mkdir a date > a/test.txt rm -rf b && \ cp -afl a b & sleep 0.2 && \ cp -afl a b & sleep 0.4 && \ cp -afl a b One of the copies fails with: cp: cannot create hard link 'b/a/test.txt' to 'a/test.txt': File exists I found this happening in the wild without the sleep multiple times in the deep-dark depths of a build system. It's not obvious to me that this fix is the correct one, but it does seem to work. There is at least some risk that the while loop in create_hard_link might spin forever, but something else would have to be recreating the file again rather quickly. It might make sense for me to add a maximum number of retries. If this patch is acceptable then I believe that it is trivial enough that it does not require copyright assignment. --- src/copy.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/copy.c b/src/copy.c index e3832c23a..b72020ccf 100644 --- a/src/copy.c +++ b/src/copy.c @@ -1789,7 +1789,7 @@ create_hard_link (char const *src_name, char const *dst_name, /* If the link failed because of an existing destination, remove that file and then call link again. */ - if (link_failed && replace && errno == EEXIST) + while (link_failed && replace && errno == EEXIST) { if (unlink (dst_name) != 0) { @@ -2617,7 +2617,7 @@ copy_internal (char const *src_name, char const *dst_name, && !(! CAN_HARDLINK_SYMLINKS && S_ISLNK (src_mode) && x->dereference == DEREF_NEVER)) { - if (! create_hard_link (src_name, dst_name, false, false, dereference)) + if (! create_hard_link (src_name, dst_name, x->unlink_dest_after_failed_open, false, dereference)) goto un_backup; } else if (S_ISREG (src_mode) -- 2.12.0.rc0 From unknown Mon Jun 23 07:50:00 2025 X-Loop: help-debbugs@gnu.org Subject: bug#25680: [PATCH] copy: Avoid race when creating hard link over recently-created file Resent-From: Paul Eggert Original-Sender: "Debbugs-submit" Resent-CC: bug-coreutils@gnu.org Resent-Date: Fri, 10 Feb 2017 19:56:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 25680 X-GNU-PR-Package: coreutils X-GNU-PR-Keywords: patch To: Mike Crowe , 25680@debbugs.gnu.org Received: via spool by 25680-submit@debbugs.gnu.org id=B25680.148675654615861 (code B ref 25680); Fri, 10 Feb 2017 19:56:01 +0000 Received: (at 25680) by debbugs.gnu.org; 10 Feb 2017 19:55:46 +0000 Received: from localhost ([127.0.0.1]:35016 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ccHIH-00047l-OT for submit@debbugs.gnu.org; Fri, 10 Feb 2017 14:55:45 -0500 Received: from zimbra.cs.ucla.edu ([131.179.128.68]:36242) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ccHIF-00047Y-DM for 25680@debbugs.gnu.org; Fri, 10 Feb 2017 14:55:44 -0500 Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id 65A871600C2; Fri, 10 Feb 2017 11:55:36 -0800 (PST) Received: from zimbra.cs.ucla.edu ([127.0.0.1]) by localhost (zimbra.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10032) with ESMTP id KGHCiPBd4L2G; Fri, 10 Feb 2017 11:55:35 -0800 (PST) Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id B55DF1600C3; Fri, 10 Feb 2017 11:55:35 -0800 (PST) X-Virus-Scanned: amavisd-new at zimbra.cs.ucla.edu Received: from zimbra.cs.ucla.edu ([127.0.0.1]) by localhost (zimbra.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id bEB7WZY-Ipbb; Fri, 10 Feb 2017 11:55:35 -0800 (PST) Received: from Penguin.CS.UCLA.EDU (Penguin.CS.UCLA.EDU [131.179.64.200]) by zimbra.cs.ucla.edu (Postfix) with ESMTPSA id 9CBE41600C2; Fri, 10 Feb 2017 11:55:35 -0800 (PST) References: <20170210191808.21739-1-mac@mcrowe.com> From: Paul Eggert Organization: UCLA Computer Science Department Message-ID: <83a20390-7f94-6c3b-6dc3-4bb1887535b3@cs.ucla.edu> Date: Fri, 10 Feb 2017 11:55:35 -0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.0 MIME-Version: 1.0 In-Reply-To: <20170210191808.21739-1-mac@mcrowe.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Score: -0.0 (/) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -0.0 (/) On 02/10/2017 11:18 AM, Mike Crowe wrote: > - if (link_failed && replace && errno == EEXIST) > + while (link_failed && replace && errno == EEXIST) This could cause 'cp -f' to loop forever, if an attacker keeps creating hard links. Is this a new vulnerability? I don't recall any other way that copying from a finite source could take forever. One possible solution would be to loop for just a few times, and then give up with a diagnostic. From unknown Mon Jun 23 07:50:00 2025 X-Loop: help-debbugs@gnu.org Subject: bug#25680: [PATCH] copy: Avoid race when creating hard link over recently-created file Resent-From: Mike Crowe Original-Sender: "Debbugs-submit" Resent-CC: bug-coreutils@gnu.org Resent-Date: Fri, 10 Feb 2017 21:20:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 25680 X-GNU-PR-Package: coreutils X-GNU-PR-Keywords: patch To: Paul Eggert Cc: 25680@debbugs.gnu.org Received: via spool by 25680-submit@debbugs.gnu.org id=B25680.148676157323347 (code B ref 25680); Fri, 10 Feb 2017 21:20:02 +0000 Received: (at 25680) by debbugs.gnu.org; 10 Feb 2017 21:19:33 +0000 Received: from localhost ([127.0.0.1]:35074 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ccIbM-00064U-Us for submit@debbugs.gnu.org; Fri, 10 Feb 2017 16:19:33 -0500 Received: from avasout06.plus.net ([212.159.14.18]:55483) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ccIbK-00064C-Tn for 25680@debbugs.gnu.org; Fri, 10 Feb 2017 16:19:31 -0500 Received: from deneb ([80.229.24.9]) by avasout06 with smtp id j9KK1u0030BmcFC019KLB8; Fri, 10 Feb 2017 21:19:22 +0000 X-CM-Score: 0.00 X-CNFS-Analysis: v=2.2 cv=QoEu5R6d c=1 sm=1 tr=0 a=E/9URZZQ5L3bK/voZ0g0HQ==:117 a=E/9URZZQ5L3bK/voZ0g0HQ==:17 a=kj9zAlcOel0A:10 a=n2v9WMKugxEA:10 a=yqrxhGLhZqVckMTHZysA:9 a=CjuIK1q_8ugA:10 Received: from mac by deneb with local (Exim 4.84_2) (envelope-from ) id 1ccIb9-00054L-Iv; Fri, 10 Feb 2017 21:19:19 +0000 Date: Fri, 10 Feb 2017 21:19:19 +0000 From: Mike Crowe Message-ID: <20170210211919.GA19321@mcrowe.com> References: <20170210191808.21739-1-mac@mcrowe.com> <83a20390-7f94-6c3b-6dc3-4bb1887535b3@cs.ucla.edu> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <83a20390-7f94-6c3b-6dc3-4bb1887535b3@cs.ucla.edu> User-Agent: Mutt/1.5.23 (2014-03-12) X-Spam-Score: -0.7 (/) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -0.7 (/) On Friday 10 February 2017 at 11:55:35 -0800, Paul Eggert wrote: > On 02/10/2017 11:18 AM, Mike Crowe wrote: > >- if (link_failed && replace && errno == EEXIST) > >+ while (link_failed && replace && errno == EEXIST) > > This could cause 'cp -f' to loop forever, if an attacker keeps creating hard > links. Is this a new vulnerability? I don't recall any other way that > copying from a finite source could take forever. > > One possible solution would be to loop for just a few times, and then give > up with a diagnostic. Indeed, that's why I wrote: > There is at least some risk that the while loop in > create_hard_link might spin forever, but something else would have to be > recreating the file again rather quickly. It might make sense for me to > add a maximum number of retries. Do you think that if I added such a limit and diagnostic then the patch would be acceptable? Thanks for the review. Mike. From unknown Mon Jun 23 07:50:00 2025 MIME-Version: 1.0 X-Mailer: MIME-tools 5.505 (Entity 5.505) X-Loop: help-debbugs@gnu.org From: help-debbugs@gnu.org (GNU bug Tracking System) To: Mike Crowe Subject: bug#25680: closed (Re: bug#25680: [PATCH] copy: Avoid race when creating hard link over recently-created file) Message-ID: References: <20170210191808.21739-1-mac@mcrowe.com> X-Gnu-PR-Message: they-closed 25680 X-Gnu-PR-Package: coreutils X-Gnu-PR-Keywords: patch Reply-To: 25680@debbugs.gnu.org Date: Sun, 12 Feb 2017 07:21:02 +0000 Content-Type: multipart/mixed; boundary="----------=_1486884062-18488-1" This is a multi-part message in MIME format... ------------=_1486884062-18488-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" Your bug report #25680: [PATCH] copy: Avoid race when creating hard link over recently-crea= ted file which was filed against the coreutils package, has been closed. The explanation is attached below, along with your original report. If you require more details, please reply to 25680@debbugs.gnu.org. --=20 25680: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=3D25680 GNU Bug Tracking System Contact help-debbugs@gnu.org with problems ------------=_1486884062-18488-1 Content-Type: message/rfc822 Content-Disposition: inline Content-Transfer-Encoding: 7bit Received: (at 25680-done) by debbugs.gnu.org; 12 Feb 2017 07:20:41 +0000 Received: from localhost ([127.0.0.1]:36136 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ccoSe-0004nX-6d for submit@debbugs.gnu.org; Sun, 12 Feb 2017 02:20:40 -0500 Received: from zimbra.cs.ucla.edu ([131.179.128.68]:51174) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ccoSY-0004nE-Qp for 25680-done@debbugs.gnu.org; Sun, 12 Feb 2017 02:20:38 -0500 Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id D653B160052; Sat, 11 Feb 2017 23:20:27 -0800 (PST) Received: from zimbra.cs.ucla.edu ([127.0.0.1]) by localhost (zimbra.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10032) with ESMTP id YEk9VokQimel; Sat, 11 Feb 2017 23:20:25 -0800 (PST) Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id 45396160054; Sat, 11 Feb 2017 23:20:25 -0800 (PST) X-Virus-Scanned: amavisd-new at zimbra.cs.ucla.edu Received: from zimbra.cs.ucla.edu ([127.0.0.1]) by localhost (zimbra.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id wl5IqsGR2oK8; Sat, 11 Feb 2017 23:20:25 -0800 (PST) Received: from [192.168.1.9] (unknown [47.153.188.248]) by zimbra.cs.ucla.edu (Postfix) with ESMTPSA id 1C69A160052; Sat, 11 Feb 2017 23:20:25 -0800 (PST) Subject: Re: bug#25680: [PATCH] copy: Avoid race when creating hard link over recently-created file To: Mike Crowe References: <20170210191808.21739-1-mac@mcrowe.com> <83a20390-7f94-6c3b-6dc3-4bb1887535b3@cs.ucla.edu> <20170210211919.GA19321@mcrowe.com> From: Paul Eggert Organization: UCLA Computer Science Department Message-ID: Date: Sat, 11 Feb 2017 23:20:24 -0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.0 MIME-Version: 1.0 In-Reply-To: <20170210211919.GA19321@mcrowe.com> Content-Type: multipart/mixed; boundary="------------16B836DBB29153A915DBCC3E" X-Spam-Score: -0.0 (/) X-Debbugs-Envelope-To: 25680-done Cc: 25680-done@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -0.0 (/) This is a multi-part message in MIME format. --------------16B836DBB29153A915DBCC3E Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Mike Crowe wrote: > Do you think that if I added such a limit and diagnostic then the patch > would be acceptable? I'd rather fix the underlying problem more systematically. How about the attached patch instead? I've installed this on Savannah so you can give it a whirl easily. It fixes the problem for me so I'm boldly marking this bug report as done; we can unmark it later if it turns out that I'm wrong. --------------16B836DBB29153A915DBCC3E Content-Type: text/x-diff; name="0001-ln-replace-destination-links-more-atomically.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="0001-ln-replace-destination-links-more-atomically.patch" >From 376967889ed7ed561e46ff6d88a66779db62737a Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Sat, 11 Feb 2017 23:12:31 -0800 Subject: [PATCH] ln: replace destination links more atomically If the file B already exists, commands like 'ln -f A B' and 'cp -fl A B' no longer remove B before creating the new link. Instead, they arrange for the new link to replace B atomically. This should fix a race condition reported by Mike Crowe (Bug#25680). * NEWS, doc/coreutils.texi (cp invocation, ln invocation): Document this. * bootstrap.conf (gnulib_modules): Add symlinkat. * src/copy.c, src/ln.c: Include force-link.h. * src/copy.c (same_file_ok): It's also OK to remove a destination symlink when creating symbolic links, or when the source and destination are on the same file system and when creating hard links. * src/copy.c (create_hard_link, copy_internal): * src/ln.c (do_link): Rewrite using force_linkat and force_symlinkat, to close a window where the destination temporarily does not exist. * src/cp.c (main): Do not set x.unlink_dest_before_opening merely because we are in link-creation mode. * src/force-link.c, src/force-link.h: New files. * src/local.mk (copy_sources, src_ln_SOURCES): Add them. * tests/cp/same-file.sh: Adjust test case to match fixed behavior. --- NEWS | 10 +++ bootstrap.conf | 2 +- doc/coreutils.texi | 18 +++-- src/copy.c | 96 +++++++++++--------------- src/cp.c | 5 -- src/force-link.c | 187 ++++++++++++++++++++++++++++++++++++++++++++++++++ src/force-link.h | 3 + src/ln.c | 31 +++------ src/local.mk | 8 ++- tests/cp/same-file.sh | 2 +- 10 files changed, 268 insertions(+), 94 deletions(-) create mode 100644 src/force-link.c create mode 100644 src/force-link.h diff --git a/NEWS b/NEWS index deaab7b..7473e6e 100644 --- a/NEWS +++ b/NEWS @@ -2,12 +2,22 @@ GNU coreutils NEWS -*- outline -*- * Noteworthy changes in release ?.? (????-??-??) [?] +** Improvements + + If the file B already exists, commands like 'ln -f A B' and + 'cp -fl A B' no longer remove B before creating the new link. + That is, there is no longer a brief moment when B does not exist. + ** Bug fixes date again converts from a specified time zone. Previously output was not converted to the local time zone, and remained in the specified one. [bug introduced in coreutils-8.26] + Commands like 'cp --no-dereference -l A B' are no longer quiet no-ops + when A is a regular file and B is a symbolic link that points to A. + [bug introduced in fileutils-4.0] + factor no longer goes into an infinite loop for certain numbers like 158909489063877810457 and 222087527029934481871. [bug introduced in coreutils-8.20] diff --git a/bootstrap.conf b/bootstrap.conf index 59b3a91..acec6f0 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -234,7 +234,7 @@ gnulib_modules=" strtod strtoimax strtoumax - symlink + symlinkat sys_ioctl sys_resource sys_stat diff --git a/doc/coreutils.texi b/doc/coreutils.texi index 3eac96b..2dbfcce 100644 --- a/doc/coreutils.texi +++ b/doc/coreutils.texi @@ -8125,9 +8125,11 @@ Equivalent to @option{--no-dereference --preserve=links}. When copying without this option and an existing destination file cannot be opened for writing, the copy fails. However, with @option{--force}, when a destination file cannot be opened, @command{cp} then removes it and -tries to open it again. Contrast this behavior with that enabled by -@option{--link} and @option{--symbolic-link}, whereby the destination file -is never opened but rather is removed unconditionally. Also see the +tries to open it again. When this option is combined with +@option{--link} (@option{-l}) or @option{--symbolic-link} +(@option{-s}), the destination link is replaced, and unless +@option{--backup} (@option{-b}) is also given there is no brief +moment when the destination does not exist. Also see the description of @option{--remove-destination}. This option is independent of the @option{--interactive} or @@ -9825,11 +9827,13 @@ directory, using the @var{target}s' names. @end itemize -Normally @command{ln} does not remove existing files. Use the -@option{--force} (@option{-f}) option to remove them unconditionally, -the @option{--interactive} (@option{-i}) option to remove them +Normally @command{ln} does not replace existing files. Use the +@option{--force} (@option{-f}) option to replace them unconditionally, +the @option{--interactive} (@option{-i}) option to replace them conditionally, and the @option{--backup} (@option{-b}) option to -rename them. +rename them. Unless the @option{--backup} (@option{-b}) option is +used there is no brief moment when the destination does not exist; +this is an extension to POSIX. @cindex hard link, defined @cindex inode, and hard links diff --git a/src/copy.c b/src/copy.c index e3832c2..9dbd536 100644 --- a/src/copy.c +++ b/src/copy.c @@ -46,6 +46,7 @@ #include "file-set.h" #include "filemode.h" #include "filenamecat.h" +#include "force-link.h" #include "full-write.h" #include "hash.h" #include "hash-triple.h" @@ -1623,11 +1624,13 @@ same_file_ok (char const *src_name, struct stat const *src_sb, } } - /* It's ok to remove a destination symlink. But that works only when we - unlink before opening the destination and when the source and destination - files are on the same partition. */ - if (x->unlink_dest_before_opening - && S_ISLNK (dst_sb_link->st_mode)) + /* It's ok to remove a destination symlink. But that works only + when creating symbolic links, or when the source and destination + are on the same file system and when creating hard links or when + unlinking before opening the destination. */ + if (x->symbolic_link + || ((x->hard_link || x->unlink_dest_before_opening) + && S_ISLNK (dst_sb_link->st_mode))) return dst_sb_link->st_dev == src_sb_link->st_dev; if (x->dereference == DEREF_NEVER) @@ -1779,36 +1782,17 @@ static bool create_hard_link (char const *src_name, char const *dst_name, bool replace, bool verbose, bool dereference) { - /* We want to guarantee that symlinks are not followed, unless requested. */ - int flags = 0; - if (dereference) - flags = AT_SYMLINK_FOLLOW; - - bool link_failed = (linkat (AT_FDCWD, src_name, AT_FDCWD, dst_name, flags) - != 0); - - /* If the link failed because of an existing destination, - remove that file and then call link again. */ - if (link_failed && replace && errno == EEXIST) - { - if (unlink (dst_name) != 0) - { - error (0, errno, _("cannot remove %s"), quoteaf (dst_name)); - return false; - } - if (verbose) - printf (_("removed %s\n"), quoteaf (dst_name)); - link_failed = (linkat (AT_FDCWD, src_name, AT_FDCWD, dst_name, flags) - != 0); - } - - if (link_failed) + int status = force_linkat (AT_FDCWD, src_name, AT_FDCWD, dst_name, + dereference ? AT_SYMLINK_FOLLOW : 0, + replace); + if (status < 0) { error (0, errno, _("cannot create hard link %s to %s"), quoteaf_n (0, dst_name), quoteaf_n (1, src_name)); return false; } - + if (0 < status && verbose) + printf (_("removed %s\n"), quoteaf (dst_name)); return true; } @@ -2592,7 +2576,9 @@ copy_internal (char const *src_name, char const *dst_name, goto un_backup; } } - if (symlink (src_name, dst_name) != 0) + if (force_symlinkat (src_name, AT_FDCWD, dst_name, + x->unlink_dest_after_failed_open) + < 0) { error (0, errno, _("cannot create symbolic link %s to %s"), quoteaf_n (0, dst_name), quoteaf_n (1, src_name)); @@ -2617,7 +2603,9 @@ copy_internal (char const *src_name, char const *dst_name, && !(! CAN_HARDLINK_SYMLINKS && S_ISLNK (src_mode) && x->dereference == DEREF_NEVER)) { - if (! create_hard_link (src_name, dst_name, false, false, dereference)) + if (! create_hard_link (src_name, dst_name, + x->unlink_dest_after_failed_open, + false, dereference)) goto un_backup; } else if (S_ISREG (src_mode) @@ -2671,33 +2659,31 @@ copy_internal (char const *src_name, char const *dst_name, goto un_backup; } - if (symlink (src_link_val, dst_name) == 0) - free (src_link_val); - else + int symlink_r = force_symlinkat (src_link_val, AT_FDCWD, dst_name, + x->unlink_dest_after_failed_open); + int symlink_err = symlink_r < 0 ? errno : 0; + if (symlink_err && x->update && !new_dst && S_ISLNK (dst_sb.st_mode) + && dst_sb.st_size == strlen (src_link_val)) { - int saved_errno = errno; - bool same_link = false; - if (x->update && !new_dst && S_ISLNK (dst_sb.st_mode) - && dst_sb.st_size == strlen (src_link_val)) + /* See if the destination is already the desired symlink. + FIXME: This behavior isn't documented, and seems wrong + in some cases, e.g., if the destination symlink has the + wrong ownership, permissions, or timestamps. */ + char *dest_link_val = + areadlink_with_size (dst_name, dst_sb.st_size); + if (dest_link_val) { - /* See if the destination is already the desired symlink. - FIXME: This behavior isn't documented, and seems wrong - in some cases, e.g., if the destination symlink has the - wrong ownership, permissions, or timestamps. */ - char *dest_link_val = - areadlink_with_size (dst_name, dst_sb.st_size); - if (dest_link_val && STREQ (dest_link_val, src_link_val)) - same_link = true; + if (STREQ (dest_link_val, src_link_val)) + symlink_err = 0; free (dest_link_val); } - free (src_link_val); - - if (! same_link) - { - error (0, saved_errno, _("cannot create symbolic link %s"), - quoteaf (dst_name)); - goto un_backup; - } + } + free (src_link_val); + if (symlink_err) + { + error (0, symlink_err, _("cannot create symbolic link %s"), + quoteaf (dst_name)); + goto un_backup; } if (x->preserve_security_context) diff --git a/src/cp.c b/src/cp.c index 0805558..88db3a3 100644 --- a/src/cp.c +++ b/src/cp.c @@ -1155,11 +1155,6 @@ main (int argc, char **argv) if (x.recursive) x.copy_as_regular = copy_contents; - /* If --force (-f) was specified and we're in link-creation mode, - first remove any existing destination file. */ - if (x.unlink_dest_after_failed_open && (x.hard_link || x.symbolic_link)) - x.unlink_dest_before_opening = true; - /* Ensure -Z overrides -a. */ if ((x.set_security_context || scontext) && ! x.require_preserve_context) diff --git a/src/force-link.c b/src/force-link.c new file mode 100644 index 0000000..e0db075 --- /dev/null +++ b/src/force-link.c @@ -0,0 +1,187 @@ +/* Implement ln -f "atomically" + + Copyright 2017 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 . */ + +/* Written by Paul Eggert. */ + +/* A naive "ln -f A B" unlinks B and then links A to B. This module + instead links A to a randomly-named temporary T in B's directory, + and then renames T to B. This approach has a window with a + randomly-named temporary, which is safer for many applications than + a window where B does not exist. */ + +#include + +#include "force-link.h" + +#include +#include + +#include +#include +#include +#include + +/* A basename pattern suitable for a temporary file. It should work + even on file systems like FAT that support only short names. + "Cu" is short for "Coreutils" or for "Changeable unstable", + take your pick.... */ + +static char const simple_pattern[] = "CuXXXXXX"; +enum { x_suffix_len = sizeof "XXXXXX" - 1 }; + +/* A size for smallish buffers containing file names. Longer file + names can use malloc. */ + +enum { smallsize = 256 }; + +/* Return a template for a file in the same directory as DSTNAME. + Use BUF if the template fits, otherwise use malloc and return NULL + (setting errno) if unsuccessful. */ + +static char * +samedir_template (char const *dstname, char buf[smallsize]) +{ + ptrdiff_t dstdirlen = last_component (dstname) - dstname; + size_t dsttmpsize = dstdirlen + sizeof simple_pattern; + char *dsttmp; + if (dsttmpsize <= smallsize) + dsttmp = buf; + else + { + dsttmp = malloc (dsttmpsize); + if (!dsttmp) + return dsttmp; + } + strcpy (mempcpy (dsttmp, dstname, dstdirlen), simple_pattern); + return dsttmp; +} + + +/* Auxiliaries for force_linkat. */ + +struct link_arg +{ + int srcdir; + char const *srcname; + int dstdir; + int flags; +}; + +static int +try_link (char *dest, void *arg) +{ + struct link_arg *a = arg; + return linkat (a->srcdir, a->srcname, a->dstdir, dest, a->flags); +} + +/* Hard-link directory SRCDIR's file SRCNAME to directory DSTDIR's + file DSTNAME, using linkat-style FLAGS to control the linking. + If FORCE and DSTNAME already exists, replace it atomically. Return + 1 if successful and DSTNAME already existed, + 0 if successful and DSTNAME did not already exist, and + -1 (setting errno) on failure. */ +int +force_linkat (int srcdir, char const *srcname, + int dstdir, char const *dstname, int flags, bool force) +{ + int r = linkat (srcdir, srcname, dstdir, dstname, flags); + if (!force || r == 0 || errno != EEXIST) + return r; + + char buf[smallsize]; + char *dsttmp = samedir_template (dstname, buf); + if (! dsttmp) + return -1; + struct link_arg arg = { srcdir, srcname, dstdir, flags }; + int err; + + if (try_tempname_len (dsttmp, 0, &arg, try_link, x_suffix_len) != 0) + err = errno; + else + { + err = renameat (dstdir, dsttmp, dstdir, dstname) == 0 ? 0 : errno; + /* Unlink DSTTMP even if renameat succeeded, in case DSTTMP + and DSTNAME were already the same hard link and renameat + was a no-op. */ + unlinkat (dstdir, dsttmp, 0); + } + + if (dsttmp != buf) + free (dsttmp); + if (!err) + return 1; + errno = err; + return -1; +} + + +/* Auxiliaries for force_symlinkat. */ + +struct symlink_arg +{ + char const *srcname; + int dstdir; +}; + +static int +try_symlink (char *dest, void *arg) +{ + struct symlink_arg *a = arg; + return symlinkat (a->srcname, a->dstdir, dest); +} + +/* Create a symlink containing SRCNAME in directory DSTDIR's file DSTNAME. + If FORCE and DSTNAME already exists, replace it atomically. Return + 1 if successful and DSTNAME already existed, + 0 if successful and DSTNAME did not already exist, and + -1 (setting errno) on failure. */ +int +force_symlinkat (char const *srcname, int dstdir, char const *dstname, + bool force) +{ + int r = symlinkat (srcname, dstdir, dstname); + if (!force || r == 0 || errno != EEXIST) + return r; + + char buf[smallsize]; + char *dsttmp = samedir_template (dstname, buf); + if (!dsttmp) + return -1; + struct symlink_arg arg = { srcname, dstdir }; + int err; + + if (try_tempname_len (dsttmp, 0, &arg, try_symlink, x_suffix_len) != 0) + err = errno; + else if (renameat (dstdir, dsttmp, dstdir, dstname) != 0) + { + err = errno; + unlinkat (dstdir, dsttmp, 0); + } + else + { + /* Don't worry about renameat being a no-op, since DSTTMP is + newly created. */ + err = 0; + } + + if (dsttmp != buf) + free (dsttmp); + if (!err) + return 1; + errno = err; + return -1; +} diff --git a/src/force-link.h b/src/force-link.h new file mode 100644 index 0000000..2a690f6 --- /dev/null +++ b/src/force-link.h @@ -0,0 +1,3 @@ +#include +extern int force_linkat (int, char const *, int, char const *, int, bool); +extern int force_symlinkat (char const *, int, char const *, bool); diff --git a/src/ln.c b/src/ln.c index bacf5f8..539d238 100644 --- a/src/ln.c +++ b/src/ln.c @@ -27,6 +27,7 @@ #include "error.h" #include "filenamecat.h" #include "file-set.h" +#include "force-link.h" #include "hash.h" #include "hash-triple.h" #include "relpath.h" @@ -183,7 +184,6 @@ do_link (const char *source, const char *dest) char *rel_source = NULL; bool dest_lstat_ok = false; bool source_is_dir = false; - bool ok; if (!symbolic_link) { @@ -301,12 +301,7 @@ do_link (const char *source, const char *dest) if (relative) source = rel_source = convert_abs_rel (source, dest); - ok = ((symbolic_link ? symlink (source, dest) - : linkat (AT_FDCWD, source, AT_FDCWD, dest, - logical ? AT_SYMLINK_FOLLOW : 0)) - == 0); - - /* If the attempt to create a link failed and we are removing or + /* If the attempt to create a link fails and we are removing or backing up destinations, unlink the destination and try again. On the surface, POSIX describes an algorithm that states that @@ -324,22 +319,12 @@ do_link (const char *source, const char *dest) that refer to the same file), rename succeeds and DEST remains. If we didn't remove DEST in that case, the subsequent symlink or link call would fail. */ - - if (!ok && errno == EEXIST && (remove_existing_files || dest_backup)) - { - if (unlink (dest) != 0) - { - error (0, errno, _("cannot remove %s"), quoteaf (dest)); - free (dest_backup); - free (rel_source); - return false; - } - - ok = ((symbolic_link ? symlink (source, dest) - : linkat (AT_FDCWD, source, AT_FDCWD, dest, - logical ? AT_SYMLINK_FOLLOW : 0)) - == 0); - } + bool ok_to_remove = remove_existing_files || dest_backup; + bool ok = 0 <= (symbolic_link + ? force_symlinkat (source, AT_FDCWD, dest, ok_to_remove) + : force_linkat (AT_FDCWD, source, AT_FDCWD, dest, + logical ? AT_SYMLINK_FOLLOW : 0, + ok_to_remove)); if (ok) { diff --git a/src/local.mk b/src/local.mk index 5b25fcb..84df099 100644 --- a/src/local.mk +++ b/src/local.mk @@ -328,7 +328,9 @@ copy_sources = \ src/copy.c \ src/cp-hash.c \ src/extent-scan.c \ - src/extent-scan.h + src/extent-scan.h \ + src/force-link.c \ + src/force-link.h # Use 'ginstall' in the definition of PROGRAMS and in dependencies to avoid # confusion with the 'install' target. The install rule transforms 'ginstall' @@ -357,7 +359,9 @@ src_vdir_SOURCES = src/ls.c src/ls-vdir.c src_id_SOURCES = src/id.c src/group-list.c src_groups_SOURCES = src/groups.c src/group-list.c src_ls_SOURCES = src/ls.c src/ls-ls.c -src_ln_SOURCES = src/ln.c src/relpath.c src/relpath.h +src_ln_SOURCES = src/ln.c \ + src/force-link.c src/force-link.h \ + src/relpath.c src/relpath.h src_chown_SOURCES = src/chown.c src/chown-core.c src_chgrp_SOURCES = src/chgrp.c src/chown-core.c src_kill_SOURCES = src/kill.c src/operand2sig.c diff --git a/tests/cp/same-file.sh b/tests/cp/same-file.sh index 978bba8..9aa6a21 100755 --- a/tests/cp/same-file.sh +++ b/tests/cp/same-file.sh @@ -142,7 +142,7 @@ cat <<\EOF | sed "$remove_these_sed" > expected 0 -bf (foo symlink symlink.~1~ -> foo) 0 -bdf (foo symlink symlink.~1~ -> foo) 1 -l [cp: cannot create hard link 'symlink' to 'foo'] (foo symlink -> foo) -0 -dl (foo symlink -> foo) +1 -dl [cp: cannot create hard link 'symlink' to 'foo'] (foo symlink -> foo) 0 -fl (foo symlink) 0 -dfl (foo symlink) 0 -bl (foo symlink symlink.~1~ -> foo) -- 2.7.4 --------------16B836DBB29153A915DBCC3E-- ------------=_1486884062-18488-1 Content-Type: message/rfc822 Content-Disposition: inline Content-Transfer-Encoding: 7bit Received: (at submit) by debbugs.gnu.org; 10 Feb 2017 19:49:19 +0000 Received: from localhost ([127.0.0.1]:35011 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ccHC3-0003yV-0d for submit@debbugs.gnu.org; Fri, 10 Feb 2017 14:49:19 -0500 Received: from eggs.gnu.org ([208.118.235.92]:55923) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ccHC2-0003yJ-9E for submit@debbugs.gnu.org; Fri, 10 Feb 2017 14:49:18 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ccHBv-0006HI-Nb for submit@debbugs.gnu.org; Fri, 10 Feb 2017 14:49:13 -0500 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on eggs.gnu.org X-Spam-Level: *** X-Spam-Status: No, score=3.3 required=5.0 tests=BAYES_50, RECEIVED_FROM_WINDOWS_HOST autolearn=disabled version=3.3.2 Received: from lists.gnu.org ([2001:4830:134:3::11]:41381) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1ccHBv-0006HC-Ke for submit@debbugs.gnu.org; Fri, 10 Feb 2017 14:49:11 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46893) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ccHBu-0006P1-Bv for bug-coreutils@gnu.org; Fri, 10 Feb 2017 14:49:11 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ccHBr-0006Gt-8O for bug-coreutils@gnu.org; Fri, 10 Feb 2017 14:49:10 -0500 Received: from relay101a.appriver.com ([207.97.230.14]:61971 helo=relay.appriver.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1ccHBr-0006Go-4P for bug-coreutils@gnu.org; Fri, 10 Feb 2017 14:49:07 -0500 X-Note-AR-ScanTimeLocal: 02/10/2017 2:19:00 PM X-Policy: brightsign.biz - brightsign.biz X-Policy: brightsign.biz - brightsign.biz X-Primary: mcrowe@brightsign.biz X-Note: This Email was scanned by AppRiver SecureTide X-Note: SecureTide Build: 9/27/2016 9:09:05 PM UTC X-Virus-Scan: V- X-Note-SnifferID: 0 X-Note: TCH-CT/SI:0-43/SG:1 2/10/2017 2:18:26 PM X-GBUdb-Analysis: 0, 213.210.30.29, Ugly c=0.454114 p=-1 Source Normal X-Signature-Violations: 0-0-0-5359-c X-Note: Spam Tests Failed: X-Country-Path: ->PRIVATE->United Kingdom->United States X-Note-Sending-IP: 213.210.30.29 X-Note-Reverse-DNS: elite.brightsigndigital.co.uk X-Note-Return-Path: mcrowe@brightsign.biz X-Note: User Rule Hits: X-Note: Global Rule Hits: G273 G274 G275 G276 G280 G281 G409 X-Note: Encrypt Rule Hits: X-Note: Mail Class: VALID X-Note: Headers Injected Received: from [213.210.30.29] (HELO elite.brightsign6) by relay.appriver.com (CommuniGate Pro SMTP 6.1.7) with ESMTPS id 112956592; Fri, 10 Feb 2017 14:19:00 -0500 Received: from chuckie.brightsign ([172.30.1.25] helo=chuckie) by elite.brightsign6 with esmtp (Exim 4.84_2) (envelope-from ) id 1ccGih-000BZ3-3R; Fri, 10 Feb 2017 19:18:59 +0000 Received: from mac by chuckie with local (Exim 4.84_2) (envelope-from ) id 1ccGih-0005fk-2e; Fri, 10 Feb 2017 19:18:59 +0000 From: Mike Crowe To: bug-coreutils@gnu.org Subject: [PATCH] copy: Avoid race when creating hard link over recently-created file Date: Fri, 10 Feb 2017 19:18:08 +0000 Message-Id: <20170210191808.21739-1-mac@mcrowe.com> X-Mailer: git-send-email 2.12.0.rc0 X-detected-operating-system: by eggs.gnu.org: Windows 7 or 8 [fuzzy] X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6.x X-Received-From: 2001:4830:134:3::11 X-Spam-Score: -5.0 (-----) X-Debbugs-Envelope-To: submit Cc: Mike Crowe X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -5.0 (-----) Ensure that create_hard_link is permitted to overwrite the destination file if --force is passed to cp. It is possible for the file to appear between checking and creating the hard link. This may happen multiple times, so also loop inside create_hard_link each time it fails. The race is made _much_ easier to reproduce by inserting a sleep(1) as the line immediately prior to the call to modified create_hard_link call in copy_internal. Steps to reproduce (assuming the aforementioned sleep has been added): mkdir a date > a/test.txt rm -rf b && \ cp -afl a b & sleep 0.2 && \ cp -afl a b & sleep 0.4 && \ cp -afl a b One of the copies fails with: cp: cannot create hard link 'b/a/test.txt' to 'a/test.txt': File exists I found this happening in the wild without the sleep multiple times in the deep-dark depths of a build system. It's not obvious to me that this fix is the correct one, but it does seem to work. There is at least some risk that the while loop in create_hard_link might spin forever, but something else would have to be recreating the file again rather quickly. It might make sense for me to add a maximum number of retries. If this patch is acceptable then I believe that it is trivial enough that it does not require copyright assignment. --- src/copy.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/copy.c b/src/copy.c index e3832c23a..b72020ccf 100644 --- a/src/copy.c +++ b/src/copy.c @@ -1789,7 +1789,7 @@ create_hard_link (char const *src_name, char const *dst_name, /* If the link failed because of an existing destination, remove that file and then call link again. */ - if (link_failed && replace && errno == EEXIST) + while (link_failed && replace && errno == EEXIST) { if (unlink (dst_name) != 0) { @@ -2617,7 +2617,7 @@ copy_internal (char const *src_name, char const *dst_name, && !(! CAN_HARDLINK_SYMLINKS && S_ISLNK (src_mode) && x->dereference == DEREF_NEVER)) { - if (! create_hard_link (src_name, dst_name, false, false, dereference)) + if (! create_hard_link (src_name, dst_name, x->unlink_dest_after_failed_open, false, dereference)) goto un_backup; } else if (S_ISREG (src_mode) -- 2.12.0.rc0 ------------=_1486884062-18488-1-- From unknown Mon Jun 23 07:50:00 2025 X-Loop: help-debbugs@gnu.org Subject: bug#25680: [PATCH] copy: Avoid race when creating hard link over recently-created file Resent-From: Mike Crowe Original-Sender: "Debbugs-submit" Resent-CC: bug-coreutils@gnu.org Resent-Date: Mon, 13 Feb 2017 12:29:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 25680 X-GNU-PR-Package: coreutils X-GNU-PR-Keywords: patch To: Paul Eggert Cc: 25680@debbugs.gnu.org Received: via spool by 25680-submit@debbugs.gnu.org id=B25680.148698890520205 (code B ref 25680); Mon, 13 Feb 2017 12:29:01 +0000 Received: (at 25680) by debbugs.gnu.org; 13 Feb 2017 12:28:25 +0000 Received: from localhost ([127.0.0.1]:37905 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1cdFk1-0005Fp-DT for submit@debbugs.gnu.org; Mon, 13 Feb 2017 07:28:25 -0500 Received: from avasout05.plus.net ([84.93.230.250]:37819) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1cdFjz-0005Ff-S7 for 25680@debbugs.gnu.org; Mon, 13 Feb 2017 07:28:24 -0500 Received: from deneb ([80.229.24.9]) by avasout05 with smtp id kCUL1u0030BmcFC01CUMJK; Mon, 13 Feb 2017 12:28:21 +0000 X-CM-Score: 0.00 X-CNFS-Analysis: v=2.2 cv=Hr8GIwbS c=1 sm=1 tr=0 a=E/9URZZQ5L3bK/voZ0g0HQ==:117 a=E/9URZZQ5L3bK/voZ0g0HQ==:17 a=kj9zAlcOel0A:10 a=n2v9WMKugxEA:10 a=jzMYVYQzWKngv6HS9mUA:9 a=CjuIK1q_8ugA:10 Received: from mac by deneb with local (Exim 4.84_2) (envelope-from ) id 1cdFjv-0000ht-UK; Mon, 13 Feb 2017 12:28:19 +0000 Date: Mon, 13 Feb 2017 12:28:19 +0000 From: Mike Crowe Message-ID: <20170213122819.GB887@mcrowe.com> References: <20170210191808.21739-1-mac@mcrowe.com> <83a20390-7f94-6c3b-6dc3-4bb1887535b3@cs.ucla.edu> <20170210211919.GA19321@mcrowe.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) X-Spam-Score: -0.7 (/) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -0.7 (/) On Saturday 11 February 2017 at 23:20:24 -0800, Paul Eggert wrote: > Mike Crowe wrote: > >Do you think that if I added such a limit and diagnostic then the patch > >would be acceptable? > > I'd rather fix the underlying problem more systematically. How about the > attached patch instead? I've installed this on Savannah so you can give it a > whirl easily. It fixes the problem for me so I'm boldly marking this bug > report as done; we can unmark it later if it turns out that I'm wrong. My recipe for reproducing problem on demand relied on inserting sleeps in the code. After your fix, the potential race now occurs inside the renameat system call so there's nowhere appropriate to place such sleeps. I think this means that your fix is clearly much better. Thanks! Mike. From unknown Mon Jun 23 07:50:00 2025 X-Loop: help-debbugs@gnu.org Subject: bug#25680: maint: tweaks so syntax tests pass for previous commit References: <20170210191808.21739-1-mac@mcrowe.com> In-Reply-To: <20170210191808.21739-1-mac@mcrowe.com> Resent-From: Paul Eggert Original-Sender: "Debbugs-submit" Resent-CC: bug-coreutils@gnu.org Resent-Date: Wed, 15 Feb 2017 19:47:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 25680 X-GNU-PR-Package: coreutils X-GNU-PR-Keywords: patch To: =?UTF-8?Q?P=C3=A1draig?= Brady Cc: 25680@debbugs.gnu.org Received: via spool by 25680-submit@debbugs.gnu.org id=B25680.148718799225942 (code B ref 25680); Wed, 15 Feb 2017 19:47:02 +0000 Received: (at 25680) by debbugs.gnu.org; 15 Feb 2017 19:46:32 +0000 Received: from localhost ([127.0.0.1]:41364 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ce5X6-0006kM-Mh for submit@debbugs.gnu.org; Wed, 15 Feb 2017 14:46:32 -0500 Received: from zimbra.cs.ucla.edu ([131.179.128.68]:40402) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ce5X4-0006k6-RV for 25680@debbugs.gnu.org; Wed, 15 Feb 2017 14:46:31 -0500 Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id 7F9871600EE; Wed, 15 Feb 2017 11:46:24 -0800 (PST) Received: from zimbra.cs.ucla.edu ([127.0.0.1]) by localhost (zimbra.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10032) with ESMTP id ssc4DOhvtKiK; Wed, 15 Feb 2017 11:46:22 -0800 (PST) Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id E319A160107; Wed, 15 Feb 2017 11:46:22 -0800 (PST) X-Virus-Scanned: amavisd-new at zimbra.cs.ucla.edu Received: from zimbra.cs.ucla.edu ([127.0.0.1]) by localhost (zimbra.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id Q0d5ITzApJZi; Wed, 15 Feb 2017 11:46:22 -0800 (PST) Received: from Penguin.CS.UCLA.EDU (Penguin.CS.UCLA.EDU [131.179.64.200]) by zimbra.cs.ucla.edu (Postfix) with ESMTPSA id C708B1600EE; Wed, 15 Feb 2017 11:46:22 -0800 (PST) From: Paul Eggert Organization: UCLA Computer Science Department Message-ID: Date: Wed, 15 Feb 2017 11:46:22 -0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.0 MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Score: 0.5 (/) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: 0.5 (/) I see some problems with this followup patch: http://git.savannah.gnu.org/cgit/coreutils.git/commit/?id=bd4bb42d65aac6591277066739ca42d1ddcc2d0e in that it will make force-link.c and force-link.h harder to move to gnulib. Is there some way that we can have the code pass syntax checks without tying it so tightly to coreutils? Also, why do the syntax checks require those redundant 'extern's? They're not that common in practice and I'd rather avoid them. From unknown Mon Jun 23 07:50:00 2025 X-Loop: help-debbugs@gnu.org Subject: bug#25680: maint: tweaks so syntax tests pass for previous commit Resent-From: =?UTF-8?Q?P=C3=A1draig?= Brady Original-Sender: "Debbugs-submit" Resent-CC: bug-coreutils@gnu.org Resent-Date: Thu, 16 Feb 2017 01:33:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 25680 X-GNU-PR-Package: coreutils X-GNU-PR-Keywords: patch To: Paul Eggert Cc: 25680@debbugs.gnu.org Received: via spool by 25680-submit@debbugs.gnu.org id=B25680.14872087494566 (code B ref 25680); Thu, 16 Feb 2017 01:33:01 +0000 Received: (at 25680) by debbugs.gnu.org; 16 Feb 2017 01:32:29 +0000 Received: from localhost ([127.0.0.1]:41488 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ceAvt-0001Ba-2L for submit@debbugs.gnu.org; Wed, 15 Feb 2017 20:32:29 -0500 Received: from midir.magicbluesmoke.com ([82.195.144.46]:39850 helo=mail.magicbluesmoke.com) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ceAvr-0001BR-50 for 25680@debbugs.gnu.org; Wed, 15 Feb 2017 20:32:27 -0500 Received: from [10.0.0.126] (c-73-170-254-78.hsd1.ca.comcast.net [73.170.254.78]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.magicbluesmoke.com (Postfix) with ESMTPSA id 6353F143; Thu, 16 Feb 2017 01:32:25 +0000 (GMT) References: <20170210191808.21739-1-mac@mcrowe.com> From: =?UTF-8?Q?P=C3=A1draig?= Brady Message-ID: <5193c95b-e3b9-bbcf-8406-f219bac48ca8@draigBrady.com> Date: Wed, 15 Feb 2017 17:32:23 -0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Spam-Score: 0.0 (/) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: 0.0 (/) On 15/02/17 11:46, Paul Eggert wrote: > I see some problems with this followup patch: > > http://git.savannah.gnu.org/cgit/coreutils.git/commit/?id=bd4bb42d65aac6591277066739ca42d1ddcc2d0e > > in that it will make force-link.c and force-link.h harder to move to > gnulib. Is there some way that we can have the code pass syntax checks > without tying it so tightly to coreutils? Generally we put stuff in gl/ that's intended for gnulib at some stage. > Also, why do the syntax checks require those redundant 'extern's? > They're not that common in practice and I'd rather avoid them. I guess to match up with the declaration in the header. Existing modules in gl/ don't use the explicit extern in either the header or the c file. thanks, Pádraig From unknown Mon Jun 23 07:50:00 2025 X-Loop: help-debbugs@gnu.org Subject: bug#25680: maint: tweaks so syntax tests pass for previous commit Resent-From: Jim Meyering Original-Sender: "Debbugs-submit" Resent-CC: bug-coreutils@gnu.org Resent-Date: Thu, 16 Feb 2017 06:04:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 25680 X-GNU-PR-Package: coreutils X-GNU-PR-Keywords: patch To: Paul Eggert Cc: 25680@debbugs.gnu.org, =?UTF-8?Q?P=C3=A1draig?= Brady Received: via spool by 25680-submit@debbugs.gnu.org id=B25680.14872250182705 (code B ref 25680); Thu, 16 Feb 2017 06:04:01 +0000 Received: (at 25680) by debbugs.gnu.org; 16 Feb 2017 06:03:38 +0000 Received: from localhost ([127.0.0.1]:41599 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ceFAH-0000hY-Rv for submit@debbugs.gnu.org; Thu, 16 Feb 2017 01:03:38 -0500 Received: from mail-ua0-f196.google.com ([209.85.217.196]:34884) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ceFAF-0000hK-I4 for 25680@debbugs.gnu.org; Thu, 16 Feb 2017 01:03:36 -0500 Received: by mail-ua0-f196.google.com with SMTP id 96so624734uaq.2 for <25680@debbugs.gnu.org>; Wed, 15 Feb 2017 22:03:35 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:sender:in-reply-to:references:from:date:message-id :subject:to:cc; bh=M1VbMSNR/bBUXZnEw7PKlF7xSyVBOU2GQVZGb0nzV3U=; b=sWq3mxAjGkx5A4pDURW7jIKI+qhURmat5jxpkSRHFlsVcEKHUX813JDh++1dspsBL9 TouiiH9xKG1Hobyr2Ba8b7y6qe7W14PucKT8KjAcoer6rlAUZT8OZy1uKuILbCrQC8wy 8QLB3m3BsZ4chNfAvN29uMkTPUXD2XWIZH50Cl1PipSTBFyXWZCo2Dtx9JqAgMsklDjB H99h+uBTPBMjVdohJQLn3Vk0OcToBG3FRZZywVbCtQWSxwilqmmO4xOrUNKHhAf8j4sg eLX479JSKo8GB62i2FuV87Bt4Q7NVHgxu9aRgzvdFjWPXrpj3OFuuqG+h5+JnDAE+tKX W2Fg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:sender:in-reply-to:references:from :date:message-id:subject:to:cc; bh=M1VbMSNR/bBUXZnEw7PKlF7xSyVBOU2GQVZGb0nzV3U=; b=lSQWxPm7F3OIu0zq2AraOASChN+f853M37pC/wkphhlwLqTR39Ux0k0t4aAu+BZ0F0 ryrVIhnIy3Pk2m3gZOyBQCXM1MKCOWmz5x0XkgOqftc5pstKREiRwXultI+RgSUXGbM1 wX4h4FKLF1Y2qb3DRdl9rD+pi0KcCR2k1b2sLkikpkPPL8l8Q3xCAVDYGXm5F4e7fukc wm7TBAO3+vv6njcfliNtMLHTI+V6NXWX7Vw+HDCHCYQB6xarWBY48zBm3FoJVjOAuH8x TygwBmHQ3BJe9zBurgSYBQJF0D6Qy7Al1b9Bon5JDhD5lrSdM62JbqfKtLn2ysnwuEjv 0B0w== X-Gm-Message-State: AMke39n0cIwU7JrvUFxds8rmDRvvVHeN6Svyc6YebMc+npbwpOEhuXG3dcQjS1Lb7Oe2WyhEZ9gLHhUBMugk5A== X-Received: by 10.176.6.167 with SMTP id g36mr269292uag.97.1487225009881; Wed, 15 Feb 2017 22:03:29 -0800 (PST) MIME-Version: 1.0 Received: by 10.176.5.198 with HTTP; Wed, 15 Feb 2017 22:03:09 -0800 (PST) In-Reply-To: References: <20170210191808.21739-1-mac@mcrowe.com> From: Jim Meyering Date: Wed, 15 Feb 2017 22:03:09 -0800 X-Google-Sender-Auth: TrrnX0OnSmHod6tOiJlG7JeUZ4Q Message-ID: Content-Type: text/plain; charset=UTF-8 X-Spam-Score: 0.7 (/) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: 0.7 (/) On Wed, Feb 15, 2017 at 11:46 AM, Paul Eggert wrote: > I see some problems with this followup patch: > > http://git.savannah.gnu.org/cgit/coreutils.git/commit/?id=bd4bb42d65aac6591277066739ca42d1ddcc2d0e > > in that it will make force-link.c and force-link.h harder to move to gnulib. > Is there some way that we can have the code pass syntax checks without tying > it so tightly to coreutils? > > Also, why do the syntax checks require those redundant 'extern's? They're > not that common in practice and I'd rather avoid them. Hi Paul, Those "extern" directives are the sole clue that these symbols are deliberately given external scope. That clue is used by the sc_tight_scope syntax check rule to ensure that no symbol is accidentally made external. If/when it moves to gnulib, those directives should be removed.