From debbugs-submit-bounces@debbugs.gnu.org Thu Sep 18 06:53:05 2014 Received: (at submit) by debbugs.gnu.org; 18 Sep 2014 10:53:06 +0000 Received: from localhost ([127.0.0.1]:43981 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1XUZKj-0005RG-HY for submit@debbugs.gnu.org; Thu, 18 Sep 2014 06:53:05 -0400 Received: from eggs.gnu.org ([208.118.235.92]:36891) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1XUZKh-0005R8-5D for submit@debbugs.gnu.org; Thu, 18 Sep 2014 06:53:04 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XUZKa-0007xq-W2 for submit@debbugs.gnu.org; Thu, 18 Sep 2014 06:53:02 -0400 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on eggs.gnu.org X-Spam-Level: X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00 autolearn=disabled version=3.3.2 Received: from lists.gnu.org ([2001:4830:134:3::11]:58025) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XUZKa-0007ww-SZ for submit@debbugs.gnu.org; Thu, 18 Sep 2014 06:52:56 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38568) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XUZKQ-0007GT-Pv for bug-coreutils@gnu.org; Thu, 18 Sep 2014 06:52:51 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XUZKL-0007uz-Hi for bug-coreutils@gnu.org; Thu, 18 Sep 2014 06:52:46 -0400 Received: from mx1.redhat.com ([209.132.183.28]:7432) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XUZKL-0007uG-9r for bug-coreutils@gnu.org; Thu, 18 Sep 2014 06:52:41 -0400 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s8IAqYhx003890 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Thu, 18 Sep 2014 06:52:35 -0400 Received: from [10.34.4.161] (unused-4-161.brq.redhat.com [10.34.4.161]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s8IAqXsg009314 for ; Thu, 18 Sep 2014 06:52:34 -0400 Subject: Possible mv race for hardlinks (rhbz #1141368 ) From: Ondrej Vasik To: CoreutilsBugs Content-Type: text/plain; charset="UTF-8" Organization: Red Hat, Inc. Date: Thu, 18 Sep 2014 12:52:32 +0200 Message-ID: <1411037552.10808.19.camel@dhcp-24-196.brq.redhat.com> Mime-Version: 1.0 Content-Transfer-Encoding: 7bit X-Scanned-By: MIMEDefang 2.68 on 10.5.11.22 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-detected-operating-system: by eggs.gnu.org: Error: Malformed IPv6 address (bad octet value). X-Received-From: 2001:4830:134:3::11 X-Spam-Score: -5.0 (-----) X-Debbugs-Envelope-To: submit X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.15 Precedence: list Reply-To: ovasik@redhat.com 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 (-----) Hi, as reported in https://bugzilla.redhat.com/show_bug.cgi?id=1141368 , there is a possible race condition in mv in the case of hardlinks. Bug is reproducible even on ext4 filesystem (I guess it is filesystem independent), even with the latest coreutils. There was already attempt to fix the non-atomicity of mv for hardlinks ( http://thread.gmane.org/gmane.comp.gnu.coreutils.bugs/12985 ), from the current reproducer it looks like the fix is probably incomplete. Regards, Ondrej Vasik From debbugs-submit-bounces@debbugs.gnu.org Wed Sep 24 12:18:46 2014 Received: (at 18499) by debbugs.gnu.org; 24 Sep 2014 16:18:46 +0000 Received: from localhost ([127.0.0.1]:50951 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1XWpHB-0002T6-R0 for submit@debbugs.gnu.org; Wed, 24 Sep 2014 12:18:46 -0400 Received: from mx1.redhat.com ([209.132.183.28]:33577) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1XWpH8-0002Sw-Nz for 18499@debbugs.gnu.org; Wed, 24 Sep 2014 12:18:43 -0400 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s8OGIfJ3014644 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Wed, 24 Sep 2014 12:18:41 -0400 Received: from [10.36.116.73] (ovpn-116-73.ams2.redhat.com [10.36.116.73]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s8OGIa0p017261 (version=TLSv1/SSLv3 cipher=DHE-RSA-CAMELLIA256-SHA bits=256 verify=NO); Wed, 24 Sep 2014 12:18:39 -0400 Message-ID: <5422EEDC.80706@draigBrady.com> Date: Wed, 24 Sep 2014 17:18:36 +0100 From: =?UTF-8?B?UMOhZHJhaWcgQnJhZHk=?= User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130110 Thunderbird/17.0.2 MIME-Version: 1.0 To: ovasik@redhat.com, Miklos Szeredi Subject: Re: bug#18499: Possible mv race for hardlinks (rhbz #1141368 ) References: <1411037552.10808.19.camel@dhcp-24-196.brq.redhat.com> In-Reply-To: <1411037552.10808.19.camel@dhcp-24-196.brq.redhat.com> X-Enigmail-Version: 1.6 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Scanned-By: MIMEDefang 2.68 on 10.5.11.26 X-Spam-Score: -5.0 (-----) X-Debbugs-Envelope-To: 18499 Cc: 18499@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.15 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 (-----) On 09/18/2014 11:52 AM, Ondrej Vasik wrote: > Hi, > as reported in https://bugzilla.redhat.com/show_bug.cgi?id=1141368 , > there is a possible race condition in mv in the case of hardlinks. > > Bug is reproducible even on ext4 filesystem (I guess it is filesystem > independent), even with the latest coreutils. There was already attempt > to fix the non-atomicity of mv for hardlinks > ( http://thread.gmane.org/gmane.comp.gnu.coreutils.bugs/12985 ), from > the current reproducer it looks like the fix is probably incomplete. The reason mv does the problematic unlink() is because it needs to simulate the rename, as rename() has this IMHO incorrect, though defined and documented behavior: "If oldpath and newpath are existing hard links referring to the same file, then rename() does nothing, and returns a success status." For arbitration of the rename between separate processes, the kernel needs to be involved. I noticed the very recent renameat2() system call added to Linux which supports flags. Miklos, do you think this is something that could be handled by renameat2() either implicitly or explicitly with a flag? thanks, Pádraig. From debbugs-submit-bounces@debbugs.gnu.org Thu Nov 13 10:41:19 2014 Received: (at 18499) by debbugs.gnu.org; 13 Nov 2014 15:41:20 +0000 Received: from localhost ([127.0.0.1]:59517 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1XowWM-0007Ao-S5 for submit@debbugs.gnu.org; Thu, 13 Nov 2014 10:41:19 -0500 Received: from mail2.vodafone.ie ([213.233.128.44]:16134) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1XowWG-0007AX-IZ for 18499@debbugs.gnu.org; Thu, 13 Nov 2014 10:41:13 -0500 X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: At4IAOrPZFRtTdH4/2dsb2JhbABbgw5UWQGDBclsh0wBAgKBHRYBAQEBAX2EAgEBAQMBIw8BRgULCw0BCgICBRYLAgIJAwIBAgFFBg0BBwEBiDQNAQi6cYYNkA8BAQEHAQEBAQEBHIEtj2cHgneBVAWXDYkVhjWEVYlsg3w9MAEBAQGCRwEBAQ Received: from unknown (HELO localhost.localdomain) ([109.77.209.248]) by mail2.vodafone.ie with ESMTP; 13 Nov 2014 15:41:10 +0000 Message-ID: <5464D115.3060008@draigBrady.com> Date: Thu, 13 Nov 2014 15:41:09 +0000 From: =?UTF-8?B?UMOhZHJhaWcgQnJhZHk=?= User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0 MIME-Version: 1.0 To: Boris Ranto Subject: Re: bug#18499: Possible mv race for hardlinks (rhbz #1141368 ) References: <1411037552.10808.19.camel@dhcp-24-196.brq.redhat.com> <5422EEDC.80706@draigBrady.com> <1415887133.5037.6.camel@unused-4-145.brq.redhat.com> In-Reply-To: <1415887133.5037.6.camel@unused-4-145.brq.redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Spam-Score: 0.0 (/) X-Debbugs-Envelope-To: 18499 Cc: 18499@debbugs.gnu.org, ovasik@redhat.com, Miklos Szeredi X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.15 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 13/11/14 13:58, Boris Ranto wrote: > On Wed, 2014-09-24 at 17:18 +0100, Pádraig Brady wrote: >> On 09/18/2014 11:52 AM, Ondrej Vasik wrote: >>> Hi, >>> as reported in https://bugzilla.redhat.com/show_bug.cgi?id=1141368 , >>> there is a possible race condition in mv in the case of hardlinks. >>> >>> Bug is reproducible even on ext4 filesystem (I guess it is filesystem >>> independent), even with the latest coreutils. There was already attempt >>> to fix the non-atomicity of mv for hardlinks >>> ( http://thread.gmane.org/gmane.comp.gnu.coreutils.bugs/12985 ), from >>> the current reproducer it looks like the fix is probably incomplete. >> >> The reason mv does the problematic unlink() is because it needs to simulate >> the rename, as rename() has this IMHO incorrect, though defined and documented behavior: >> >> "If oldpath and newpath are existing hard links referring to the same >> file, then rename() does nothing, and returns a success status." >> >> For arbitration of the rename between separate processes, >> the kernel needs to be involved. I noticed the very recent >> renameat2() system call added to Linux which supports flags. >> Miklos, do you think this is something that could be >> handled by renameat2() either implicitly or explicitly with a flag? >> >> thanks, >> Pádraig. >> >> >> > > Hi all, > > I've looked into this and I believe that the whole idea that 'mv a b' > where a and b are the same file (but different hard links) would unlink > one of the files is flawed -- I think we should return an error message > (something like we do for mv a a) and exit in this case. I'll add a > little more of my reasoning, here: > > As Pádraig stated in his comment (and as is stated in rename man page): > > "If oldpath and newpath are existing hard links referring to the same > file, then rename() does nothing, and returns a success status." > > This is probably due to this documented behaviour from rename man page: > > "rename() renames a file, moving it between directories if > required. Any other hard links to the file (as created using > link(2)) are unaffected." > > I.e. rename does not touch other hard links directly from its definition > (hence, it can't do anything if it gets the same hard linked file > twice). This actually means that (without a locking mechanism) there is > no way for us to do mv a b for hard linked files atomically -- the two > separate manual unlinks would race no matter what we do. > > The renameat2() function does not help here either (at least from what I > could find about it). It only allows us to do atomic switch of the files > which does not really help in this case. A flag could be added for this case though right? I.E. to handle the case of racing renames, one file would always be left in place. In other words to support this functionality we would need the kernel support from renameat(). > Hence, I think that the best thing for us to do here is to print the > "mv: 'a' and 'b' are the same hard linked file' and exit with an error > code if we detect this behaviour. At least if we want to preserve the > atomicity of the operation. We've three options that I see: 1. Simulate but be susceptible to data loss races (what we do now) 2. Ignore rename() issue and leave both files present (what FreeBSD does) 3. Fail with a "files are identical" error (what Solaris does) Now 1 is also an "issue" when the source and dest files are on separate file systems. I.E. mv will need to unlink() in that case. So from a high level, mv is foregoing the atomicity of the rename in this edge case to provide consistent behavior across all cases. The question boils down to whether atomicity in this edge case is required. Could you expand on a use case that requires this atomicity? thanks, Pádraig. From debbugs-submit-bounces@debbugs.gnu.org Thu Nov 13 11:08:24 2014 Received: (at 18499) by debbugs.gnu.org; 13 Nov 2014 16:08:24 +0000 Received: from localhost ([127.0.0.1]:59543 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1XowwY-0007sv-QX for submit@debbugs.gnu.org; Thu, 13 Nov 2014 11:08:24 -0500 Received: from mx1.redhat.com ([209.132.183.28]:43765) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1XouvL-0003Re-6b for 18499@debbugs.gnu.org; Thu, 13 Nov 2014 08:59:00 -0500 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id sADDwuYK028045 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Thu, 13 Nov 2014 08:58:57 -0500 Received: from [10.34.4.234] (unused-4-234.brq.redhat.com [10.34.4.234]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id sADDwrqQ023222 (version=TLSv1/SSLv3 cipher=AES256-SHA bits=256 verify=NO); Thu, 13 Nov 2014 08:58:54 -0500 Message-ID: <1415887133.5037.6.camel@unused-4-145.brq.redhat.com> Subject: Re: bug#18499: Possible mv race for hardlinks (rhbz #1141368 ) From: Boris Ranto To: =?ISO-8859-1?Q?P=E1draig?= Brady Date: Thu, 13 Nov 2014 14:58:53 +0100 In-Reply-To: <5422EEDC.80706@draigBrady.com> References: <1411037552.10808.19.camel@dhcp-24-196.brq.redhat.com> <5422EEDC.80706@draigBrady.com> Organization: Red Hat Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 8bit X-Scanned-By: MIMEDefang 2.68 on 10.5.11.24 X-Spam-Score: -6.0 (------) X-Debbugs-Envelope-To: 18499 X-Mailman-Approved-At: Thu, 13 Nov 2014 11:08:19 -0500 Cc: 18499@debbugs.gnu.org, ovasik@redhat.com, Miklos Szeredi X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.15 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: -6.0 (------) On Wed, 2014-09-24 at 17:18 +0100, Pádraig Brady wrote: > On 09/18/2014 11:52 AM, Ondrej Vasik wrote: > > Hi, > > as reported in https://bugzilla.redhat.com/show_bug.cgi?id=1141368 , > > there is a possible race condition in mv in the case of hardlinks. > > > > Bug is reproducible even on ext4 filesystem (I guess it is filesystem > > independent), even with the latest coreutils. There was already attempt > > to fix the non-atomicity of mv for hardlinks > > ( http://thread.gmane.org/gmane.comp.gnu.coreutils.bugs/12985 ), from > > the current reproducer it looks like the fix is probably incomplete. > > The reason mv does the problematic unlink() is because it needs to simulate > the rename, as rename() has this IMHO incorrect, though defined and documented behavior: > > "If oldpath and newpath are existing hard links referring to the same > file, then rename() does nothing, and returns a success status." > > For arbitration of the rename between separate processes, > the kernel needs to be involved. I noticed the very recent > renameat2() system call added to Linux which supports flags. > Miklos, do you think this is something that could be > handled by renameat2() either implicitly or explicitly with a flag? > > thanks, > Pádraig. > > > Hi all, I've looked into this and I believe that the whole idea that 'mv a b' where a and b are the same file (but different hard links) would unlink one of the files is flawed -- I think we should return an error message (something like we do for mv a a) and exit in this case. I'll add a little more of my reasoning, here: As Pádraig stated in his comment (and as is stated in rename man page): "If oldpath and newpath are existing hard links referring to the same file, then rename() does nothing, and returns a success status." This is probably due to this documented behaviour from rename man page: "rename() renames a file, moving it between directories if required. Any other hard links to the file (as created using link(2)) are unaffected." I.e. rename does not touch other hard links directly from its definition (hence, it can't do anything if it gets the same hard linked file twice). This actually means that (without a locking mechanism) there is no way for us to do mv a b for hard linked files atomically -- the two separate manual unlinks would race no matter what we do. The renameat2() function does not help here either (at least from what I could find about it). It only allows us to do atomic switch of the files which does not really help in this case. Hence, I think that the best thing for us to do here is to print the "mv: 'a' and 'b' are the same hard linked file' and exit with an error code if we detect this behaviour. At least if we want to preserve the atomicity of the operation. -Boris From debbugs-submit-bounces@debbugs.gnu.org Thu Nov 13 11:08:25 2014 Received: (at 18499) by debbugs.gnu.org; 13 Nov 2014 16:08:25 +0000 Received: from localhost ([127.0.0.1]:59545 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1Xowwa-0007t2-BI for submit@debbugs.gnu.org; Thu, 13 Nov 2014 11:08:25 -0500 Received: from mx1.redhat.com ([209.132.183.28]:50761) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1XowuY-0007pP-1W for 18499@debbugs.gnu.org; Thu, 13 Nov 2014 11:06:18 -0500 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id sADG6FWe024476 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Thu, 13 Nov 2014 11:06:16 -0500 Received: from [10.34.4.234] (unused-4-234.brq.redhat.com [10.34.4.234]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id sADG6Dvd002167 (version=TLSv1/SSLv3 cipher=AES256-SHA bits=256 verify=NO); Thu, 13 Nov 2014 11:06:14 -0500 Message-ID: <1415894773.5037.19.camel@unused-4-145.brq.redhat.com> Subject: Re: bug#18499: Possible mv race for hardlinks (rhbz #1141368 ) From: Boris Ranto To: =?ISO-8859-1?Q?P=E1draig?= Brady Date: Thu, 13 Nov 2014 17:06:13 +0100 In-Reply-To: <5464D115.3060008@draigBrady.com> References: <1411037552.10808.19.camel@dhcp-24-196.brq.redhat.com> <5422EEDC.80706@draigBrady.com> <1415887133.5037.6.camel@unused-4-145.brq.redhat.com> <5464D115.3060008@draigBrady.com> Organization: Red Hat Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 8bit X-Scanned-By: MIMEDefang 2.68 on 10.5.11.24 X-Spam-Score: -6.0 (------) X-Debbugs-Envelope-To: 18499 X-Mailman-Approved-At: Thu, 13 Nov 2014 11:08:19 -0500 Cc: 18499@debbugs.gnu.org, ovasik@redhat.com, Miklos Szeredi X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.15 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: -6.0 (------) On Thu, 2014-11-13 at 15:41 +0000, Pádraig Brady wrote: > On 13/11/14 13:58, Boris Ranto wrote: > > On Wed, 2014-09-24 at 17:18 +0100, Pádraig Brady wrote: > >> On 09/18/2014 11:52 AM, Ondrej Vasik wrote: > >>> Hi, > >>> as reported in https://bugzilla.redhat.com/show_bug.cgi?id=1141368 , > >>> there is a possible race condition in mv in the case of hardlinks. > >>> > >>> Bug is reproducible even on ext4 filesystem (I guess it is filesystem > >>> independent), even with the latest coreutils. There was already attempt > >>> to fix the non-atomicity of mv for hardlinks > >>> ( http://thread.gmane.org/gmane.comp.gnu.coreutils.bugs/12985 ), from > >>> the current reproducer it looks like the fix is probably incomplete. > >> > >> The reason mv does the problematic unlink() is because it needs to simulate > >> the rename, as rename() has this IMHO incorrect, though defined and documented behavior: > >> > >> "If oldpath and newpath are existing hard links referring to the same > >> file, then rename() does nothing, and returns a success status." > >> > >> For arbitration of the rename between separate processes, > >> the kernel needs to be involved. I noticed the very recent > >> renameat2() system call added to Linux which supports flags. > >> Miklos, do you think this is something that could be > >> handled by renameat2() either implicitly or explicitly with a flag? > >> > >> thanks, > >> Pádraig. > >> > >> > >> > > > > Hi all, > > > > I've looked into this and I believe that the whole idea that 'mv a b' > > where a and b are the same file (but different hard links) would unlink > > one of the files is flawed -- I think we should return an error message > > (something like we do for mv a a) and exit in this case. I'll add a > > little more of my reasoning, here: > > > > As Pádraig stated in his comment (and as is stated in rename man page): > > > > "If oldpath and newpath are existing hard links referring to the same > > file, then rename() does nothing, and returns a success status." > > > > This is probably due to this documented behaviour from rename man page: > > > > "rename() renames a file, moving it between directories if > > required. Any other hard links to the file (as created using > > link(2)) are unaffected." > > > > I.e. rename does not touch other hard links directly from its definition > > (hence, it can't do anything if it gets the same hard linked file > > twice). This actually means that (without a locking mechanism) there is > > no way for us to do mv a b for hard linked files atomically -- the two > > separate manual unlinks would race no matter what we do. > > > > The renameat2() function does not help here either (at least from what I > > could find about it). It only allows us to do atomic switch of the files > > which does not really help in this case. > > A flag could be added for this case though right? > I.E. to handle the case of racing renames, > one file would always be left in place. > In other words to support this functionality we would need > the kernel support from renameat(). > Yes, technically it could. Maybe, it would be worth a try to propose it for kernel. > > Hence, I think that the best thing for us to do here is to print the > > "mv: 'a' and 'b' are the same hard linked file' and exit with an error > > code if we detect this behaviour. At least if we want to preserve the > > atomicity of the operation. > > We've three options that I see: > > 1. Simulate but be susceptible to data loss races (what we do now) > 2. Ignore rename() issue and leave both files present (what FreeBSD does) > 3. Fail with a "files are identical" error (what Solaris does) > Personally, I like the Solaris way the best (if we can't get the system call that would assure the atomicity in this case) -- i.e. let the user know that we can't do that. It is also in tune with what the link man page says for hard links: "This new name may be used exactly as the old one for any operation;" As we error out on 'mv a a', we should probably also threat 'mv a b' where a and b are the same file that way. As opposed to FreeBSD solution, the user will know that he needs to run 'rm a' to accomplish what he actually wanted to do in the first place. > Now 1 is also an "issue" when the source and dest files are on separate file systems. > I.E. mv will need to unlink() in that case. So from a high level, mv is foregoing the > atomicity of the rename in this edge case to provide consistent behavior across > all cases. The question boils down to whether atomicity in this edge case is required. > Could you expand on a use case that requires this atomicity? > There actually is a real world use case. The original red hat bugzilla (1141368) was filed because people were hitting this issue in a distributed environment (via GlusterFS) -- i.e. two people were renaming files at the same time from different locations. -Boris > thanks, > Pádraig. From debbugs-submit-bounces@debbugs.gnu.org Thu Nov 13 15:52:08 2014 Received: (at 18499) by debbugs.gnu.org; 13 Nov 2014 20:52:08 +0000 Received: from localhost ([127.0.0.1]:59811 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1Xp1NA-0002Wz-Hl for submit@debbugs.gnu.org; Thu, 13 Nov 2014 15:52:08 -0500 Received: from smtp.cs.ucla.edu ([131.179.128.62]:36049) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1Xp1N8-0002Wp-L6 for 18499@debbugs.gnu.org; Thu, 13 Nov 2014 15:52:07 -0500 Received: from localhost (localhost.localdomain [127.0.0.1]) by smtp.cs.ucla.edu (Postfix) with ESMTP id BC5FCA600CB; Thu, 13 Nov 2014 12:52:04 -0800 (PST) X-Virus-Scanned: amavisd-new at smtp.cs.ucla.edu Received: from smtp.cs.ucla.edu ([127.0.0.1]) by localhost (smtp.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id kdwDNPsnWLE4; Thu, 13 Nov 2014 12:51:56 -0800 (PST) Received: from penguin.cs.ucla.edu (Penguin.CS.UCLA.EDU [131.179.64.200]) by smtp.cs.ucla.edu (Postfix) with ESMTPSA id 157B5A600C7; Thu, 13 Nov 2014 12:51:56 -0800 (PST) Message-ID: <546519EB.3050509@cs.ucla.edu> Date: Thu, 13 Nov 2014 12:51:55 -0800 From: Paul Eggert Organization: UCLA Computer Science Department User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0 MIME-Version: 1.0 To: Boris Ranto , =?UTF-8?B?UMOhZHJhaWcgQnJhZHk=?= Subject: Re: bug#18499: Possible mv race for hardlinks (rhbz #1141368 ) References: <1411037552.10808.19.camel@dhcp-24-196.brq.redhat.com> <5422EEDC.80706@draigBrady.com> <1415887133.5037.6.camel@unused-4-145.brq.redhat.com> <5464D115.3060008@draigBrady.com> <1415894773.5037.19.camel@unused-4-145.brq.redhat.com> In-Reply-To: <1415894773.5037.19.camel@unused-4-145.brq.redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Score: -3.3 (---) X-Debbugs-Envelope-To: 18499 Cc: 18499@debbugs.gnu.org, ovasik@redhat.com, Miklos Szeredi X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.15 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: -3.3 (---) On 11/13/2014 08:06 AM, Boris Ranto wrote: > Personally, I like the Solaris way the best (if we can't get the system > call that would assure the atomicity in this case) I tend to agree. The Solaris way is racy too, butthe race doesn't lose data, and the warning is helpful. Data loss is a Big Deal and we really should fix this one way or another. From debbugs-submit-bounces@debbugs.gnu.org Fri Nov 14 18:36:55 2014 Received: (at 18499) by debbugs.gnu.org; 14 Nov 2014 23:36:55 +0000 Received: from localhost ([127.0.0.1]:32927 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1XpQQA-00074w-5Z for submit@debbugs.gnu.org; Fri, 14 Nov 2014 18:36:54 -0500 Received: from mail5.vodafone.ie ([213.233.128.176]:48786) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1XpQQ6-00074l-VN for 18499@debbugs.gnu.org; Fri, 14 Nov 2014 18:36:52 -0500 X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: AlwJAEGRZlRtT6OH/2dsb2JhbABbgw5VVAUBgjVQyX+HSwECAoEbFgEBAQEBfYQCAQEBAwEjDwFGBQsLDQEKAgIFFgsCAgkDAgECAUUGDQEHAQEXiB0NAQMFuz2GDZAtAQEBBwEBAQEBARyBLYlQhiUHgneBVAWXIokbhjuEWYMDhnKDfD0wAQEBAYJHAQEB Received: from unknown (HELO localhost.localdomain) ([109.79.163.135]) by mail3.vodafone.ie with ESMTP; 14 Nov 2014 23:36:47 +0000 Message-ID: <5466920F.6010606@draigBrady.com> Date: Fri, 14 Nov 2014 23:36:47 +0000 From: =?UTF-8?B?UMOhZHJhaWcgQnJhZHk=?= User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0 MIME-Version: 1.0 To: Boris Ranto Subject: Re: bug#18499: Possible mv race for hardlinks (rhbz #1141368 ) References: <1411037552.10808.19.camel@dhcp-24-196.brq.redhat.com> <5422EEDC.80706@draigBrady.com> <1415887133.5037.6.camel@unused-4-145.brq.redhat.com> <5464D115.3060008@draigBrady.com> <1415894773.5037.19.camel@unused-4-145.brq.redhat.com> In-Reply-To: <1415894773.5037.19.camel@unused-4-145.brq.redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Spam-Score: 0.0 (/) X-Debbugs-Envelope-To: 18499 Cc: 18499@debbugs.gnu.org, ovasik@redhat.com, Miklos Szeredi X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.15 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 13/11/14 16:06, Boris Ranto wrote: > On Thu, 2014-11-13 at 15:41 +0000, Pádraig Brady wrote: >> On 13/11/14 13:58, Boris Ranto wrote: >>> On Wed, 2014-09-24 at 17:18 +0100, Pádraig Brady wrote: >>>> On 09/18/2014 11:52 AM, Ondrej Vasik wrote: >>>>> Hi, >>>>> as reported in https://bugzilla.redhat.com/show_bug.cgi?id=1141368 , >>>>> there is a possible race condition in mv in the case of hardlinks. >>>>> >>>>> Bug is reproducible even on ext4 filesystem (I guess it is filesystem >>>>> independent), even with the latest coreutils. There was already attempt >>>>> to fix the non-atomicity of mv for hardlinks >>>>> ( http://thread.gmane.org/gmane.comp.gnu.coreutils.bugs/12985 ), from >>>>> the current reproducer it looks like the fix is probably incomplete. >>>> >>>> The reason mv does the problematic unlink() is because it needs to simulate >>>> the rename, as rename() has this IMHO incorrect, though defined and documented behavior: >>>> >>>> "If oldpath and newpath are existing hard links referring to the same >>>> file, then rename() does nothing, and returns a success status." >>>> >>>> For arbitration of the rename between separate processes, >>>> the kernel needs to be involved. I noticed the very recent >>>> renameat2() system call added to Linux which supports flags. >>>> Miklos, do you think this is something that could be >>>> handled by renameat2() either implicitly or explicitly with a flag? >>>> >>>> thanks, >>>> Pádraig. >>>> >>>> >>>> >>> >>> Hi all, >>> >>> I've looked into this and I believe that the whole idea that 'mv a b' >>> where a and b are the same file (but different hard links) would unlink >>> one of the files is flawed -- I think we should return an error message >>> (something like we do for mv a a) and exit in this case. I'll add a >>> little more of my reasoning, here: >>> >>> As Pádraig stated in his comment (and as is stated in rename man page): >>> >>> "If oldpath and newpath are existing hard links referring to the same >>> file, then rename() does nothing, and returns a success status." >>> >>> This is probably due to this documented behaviour from rename man page: >>> >>> "rename() renames a file, moving it between directories if >>> required. Any other hard links to the file (as created using >>> link(2)) are unaffected." >>> >>> I.e. rename does not touch other hard links directly from its definition >>> (hence, it can't do anything if it gets the same hard linked file >>> twice). This actually means that (without a locking mechanism) there is >>> no way for us to do mv a b for hard linked files atomically -- the two >>> separate manual unlinks would race no matter what we do. >>> >>> The renameat2() function does not help here either (at least from what I >>> could find about it). It only allows us to do atomic switch of the files >>> which does not really help in this case. >> >> A flag could be added for this case though right? >> I.E. to handle the case of racing renames, >> one file would always be left in place. >> In other words to support this functionality we would need >> the kernel support from renameat(). >> > > Yes, technically it could. Maybe, it would be worth a try to propose it > for kernel. > >>> Hence, I think that the best thing for us to do here is to print the >>> "mv: 'a' and 'b' are the same hard linked file' and exit with an error >>> code if we detect this behaviour. At least if we want to preserve the >>> atomicity of the operation. >> >> We've three options that I see: >> >> 1. Simulate but be susceptible to data loss races (what we do now) >> 2. Ignore rename() issue and leave both files present (what FreeBSD does) >> 3. Fail with a "files are identical" error (what Solaris does) >> I see this was previously discussed in http://bugs.gnu.org/10686 and it was mentioned there that the 2008 POSIX standard explicitly states that any of the three approaches above is appropriate. > Personally, I like the Solaris way the best (if we can't get the system > call that would assure the atomicity in this case) -- i.e. let the user > know that we can't do that. It is also in tune with what the link man > page says for hard links: > > "This new name may be used exactly as the old one for any > operation;" > > As we error out on 'mv a a', we should probably also threat 'mv a b' > where a and b are the same file that way. > > As opposed to FreeBSD solution, the user will know that he needs to run > 'rm a' to accomplish what he actually wanted to do in the first place. > >> Now 1 is also an "issue" when the source and dest files are on separate file systems. >> I.E. mv will need to unlink() in that case. So from a high level, mv is foregoing the >> atomicity of the rename in this edge case to provide consistent behavior across >> all cases. The question boils down to whether atomicity in this edge case is required. >> Could you expand on a use case that requires this atomicity? >> > There actually is a real world use case. The original red hat bugzilla > (1141368) was filed because people were hitting this issue in a > distributed environment (via GlusterFS) -- i.e. two people were renaming > files at the same time from different locations. Following those bugs indicates it was an internal test case that was failing. I.E. it's an unlikely edge case. I'm in a bit of a quandary with this one. Seeing as this (consistent high level) behavior has been in use for the last 11 years: http://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=fc6073d6 without a "real" customer complaint, I'm tending to leave as is, but push for better behavior from the kernel. I.E. add a RENAME_REPLACE flag to renameat2(), which will do the rename atomically for hardlinks, so we could do something like: #if !HAVE_RENAME_REPLACE if (same_file (a, b)) unlink (src); /* as we do now. */ #endif rename (a, b); thanks, Pádraig. From debbugs-submit-bounces@debbugs.gnu.org Sun Nov 16 06:38:16 2014 Received: (at 18499) by debbugs.gnu.org; 16 Nov 2014 11:38:17 +0000 Received: from localhost ([127.0.0.1]:34359 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1Xpy9n-0005cZ-Rw for submit@debbugs.gnu.org; Sun, 16 Nov 2014 06:38:16 -0500 Received: from mx1.redhat.com ([209.132.183.28]:37743) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1Xpy9i-0005cI-FB for 18499@debbugs.gnu.org; Sun, 16 Nov 2014 06:38:11 -0500 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id sAGBc8nb028187 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Sun, 16 Nov 2014 06:38:09 -0500 Received: from [10.36.7.43] (vpn1-7-43.ams2.redhat.com [10.36.7.43]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id sAGBc50U024040; Sun, 16 Nov 2014 06:38:06 -0500 Subject: Re: bug#18499: Possible mv race for hardlinks (rhbz #1141368 ) From: Ondrej Vasik To: =?ISO-8859-1?Q?P=E1draig?= Brady In-Reply-To: <5466920F.6010606@draigBrady.com> References: <1411037552.10808.19.camel@dhcp-24-196.brq.redhat.com> <5422EEDC.80706@draigBrady.com> <1415887133.5037.6.camel@unused-4-145.brq.redhat.com> <5464D115.3060008@draigBrady.com> <1415894773.5037.19.camel@unused-4-145.brq.redhat.com> <5466920F.6010606@draigBrady.com> Content-Type: text/plain; charset="UTF-8" Organization: Red Hat, Inc. Date: Sun, 16 Nov 2014 12:38:04 +0100 Message-ID: <1416137884.5049.25.camel@dhcp-24-196.brq.redhat.com> Mime-Version: 1.0 Content-Transfer-Encoding: 8bit X-Scanned-By: MIMEDefang 2.68 on 10.5.11.22 X-Spam-Score: -5.0 (-----) X-Debbugs-Envelope-To: 18499 Cc: 18499@debbugs.gnu.org, Boris Ranto , Miklos Szeredi X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.15 Precedence: list Reply-To: ovasik@redhat.com 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 (-----) On Fri, 2014-11-14 at 23:36 +0000, Pádraig Brady wrote: > On 13/11/14 16:06, Boris Ranto wrote: > > On Thu, 2014-11-13 at 15:41 +0000, Pádraig Brady wrote: > >> On 13/11/14 13:58, Boris Ranto wrote: > >>> On Wed, 2014-09-24 at 17:18 +0100, Pádraig Brady wrote: ... > >> We've three options that I see: > >> > >> 1. Simulate but be susceptible to data loss races (what we do now) > >> 2. Ignore rename() issue and leave both files present (what FreeBSD does) > >> 3. Fail with a "files are identical" error (what Solaris does) > >> > > I see this was previously discussed in http://bugs.gnu.org/10686 > and it was mentioned there that the 2008 POSIX standard explicitly > states that any of the three approaches above is appropriate. > > > Personally, I like the Solaris way the best (if we can't get the system > > call that would assure the atomicity in this case) -- i.e. let the user > > know that we can't do that. It is also in tune with what the link man > > page says for hard links: > > > > "This new name may be used exactly as the old one for any > > operation;" > > > > As we error out on 'mv a a', we should probably also threat 'mv a b' > > where a and b are the same file that way. > > > > As opposed to FreeBSD solution, the user will know that he needs to run > > 'rm a' to accomplish what he actually wanted to do in the first place. > > > >> Now 1 is also an "issue" when the source and dest files are on separate file systems. > >> I.E. mv will need to unlink() in that case. So from a high level, mv is foregoing the > >> atomicity of the rename in this edge case to provide consistent behavior across > >> all cases. The question boils down to whether atomicity in this edge case is required. > >> Could you expand on a use case that requires this atomicity? > >> > > There actually is a real world use case. The original red hat bugzilla > > (1141368) was filed because people were hitting this issue in a > > distributed environment (via GlusterFS) -- i.e. two people were renaming > > files at the same time from different locations. > > Following those bugs indicates it was an internal test case that was failing. > I.E. it's an unlikely edge case. > > I'm in a bit of a quandary with this one. > Seeing as this (consistent high level) behavior has been in use for the last 11 years: > http://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=fc6073d6 > without a "real" customer complaint, I'm tending to leave as is, > but push for better behavior from the kernel. There was at least one complaint about non-atomicity ( http://thread.gmane.org/gmane.comp.gnu.coreutils.bugs/12985 ). Jim tried to address this with http://git.savannah.gnu.org/cgit/coreutils.git/commit/?id=63feb84a2db5246fb71df45884589b914679110c , however this non-atomicity still remains at least on this one place shown by the test-case in the mentioned rhbz #1141368. I have asked if there are some real world usecases - other than this artificial one - in the original bugzilla. > > I.E. add a RENAME_REPLACE flag to renameat2(), which will > do the rename atomically for hardlinks, so we could do something like: > > #if !HAVE_RENAME_REPLACE > if (same_file (a, b)) > unlink (src); /* as we do now. */ > #endif > rename (a, b); > > thanks, > Pádraig. > Personally, although this one reproducer scenario is artificial, it still brings potential data loss - and this is always bad. Flag in kernel sounds like reasonable compromise - to not change existing behaviour allowed by POSIX. It may take some time, though. However - as it is relatively corner case - change to Solaris approach should be IMHO relatively safe (for other cases, behaviour will not change at all). Regards, Ondrej From debbugs-submit-bounces@debbugs.gnu.org Sun Nov 16 07:06:52 2014 Received: (at 18499) by debbugs.gnu.org; 16 Nov 2014 12:06:52 +0000 Received: from localhost ([127.0.0.1]:34381 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1XpybT-00085G-8q for submit@debbugs.gnu.org; Sun, 16 Nov 2014 07:06:51 -0500 Received: from mail5.vodafone.ie ([213.233.128.176]:38487) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1XpybQ-000855-ME for 18499@debbugs.gnu.org; Sun, 16 Nov 2014 07:06:49 -0500 X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: AkEKAHWSaFRtT2QZ/2dsb2JhbABbgw5VVAUBgjVQiDvAT4dJAQICgQkWAQEBAQF9hAIBAQEDASMPAUYFCwsNCwICBRYLAgIJAwIBAgFFEwEHAQEXiB0NAQMFulSGDY9AAQsggS2JUIYlB4J3gVQFlyKJG4Y7hFmDA4Zyg3w9MQEBAQGCRgEBAQ Received: from unknown (HELO localhost.localdomain) ([109.79.100.25]) by mail3.vodafone.ie with ESMTP; 16 Nov 2014 12:06:47 +0000 Message-ID: <54689356.8070806@draigBrady.com> Date: Sun, 16 Nov 2014 12:06:46 +0000 From: =?UTF-8?B?UMOhZHJhaWcgQnJhZHk=?= User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0 MIME-Version: 1.0 To: ovasik@redhat.com Subject: Re: bug#18499: Possible mv race for hardlinks (rhbz #1141368 ) References: <1411037552.10808.19.camel@dhcp-24-196.brq.redhat.com> <5422EEDC.80706@draigBrady.com> <1415887133.5037.6.camel@unused-4-145.brq.redhat.com> <5464D115.3060008@draigBrady.com> <1415894773.5037.19.camel@unused-4-145.brq.redhat.com> <5466920F.6010606@draigBrady.com> <1416137884.5049.25.camel@dhcp-24-196.brq.redhat.com> In-Reply-To: <1416137884.5049.25.camel@dhcp-24-196.brq.redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Spam-Score: 0.0 (/) X-Debbugs-Envelope-To: 18499 Cc: 18499@debbugs.gnu.org, Boris Ranto , Miklos Szeredi X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.15 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 16/11/14 11:38, Ondrej Vasik wrote: > On Fri, 2014-11-14 at 23:36 +0000, Pádraig Brady wrote: >> On 13/11/14 16:06, Boris Ranto wrote: >>> On Thu, 2014-11-13 at 15:41 +0000, Pádraig Brady wrote: >>>> On 13/11/14 13:58, Boris Ranto wrote: >>>>> On Wed, 2014-09-24 at 17:18 +0100, Pádraig Brady wrote: > ... >>>> We've three options that I see: >>>> >>>> 1. Simulate but be susceptible to data loss races (what we do now) >>>> 2. Ignore rename() issue and leave both files present (what FreeBSD does) >>>> 3. Fail with a "files are identical" error (what Solaris does) >>>> >> >> I see this was previously discussed in http://bugs.gnu.org/10686 >> and it was mentioned there that the 2008 POSIX standard explicitly >> states that any of the three approaches above is appropriate. >> >>> Personally, I like the Solaris way the best (if we can't get the system >>> call that would assure the atomicity in this case) -- i.e. let the user >>> know that we can't do that. It is also in tune with what the link man >>> page says for hard links: >>> >>> "This new name may be used exactly as the old one for any >>> operation;" >>> >>> As we error out on 'mv a a', we should probably also threat 'mv a b' >>> where a and b are the same file that way. >>> >>> As opposed to FreeBSD solution, the user will know that he needs to run >>> 'rm a' to accomplish what he actually wanted to do in the first place. >>> >>>> Now 1 is also an "issue" when the source and dest files are on separate file systems. >>>> I.E. mv will need to unlink() in that case. So from a high level, mv is foregoing the >>>> atomicity of the rename in this edge case to provide consistent behavior across >>>> all cases. The question boils down to whether atomicity in this edge case is required. >>>> Could you expand on a use case that requires this atomicity? >>>> >>> There actually is a real world use case. The original red hat bugzilla >>> (1141368) was filed because people were hitting this issue in a >>> distributed environment (via GlusterFS) -- i.e. two people were renaming >>> files at the same time from different locations. >> >> Following those bugs indicates it was an internal test case that was failing. >> I.E. it's an unlikely edge case. >> >> I'm in a bit of a quandary with this one. >> Seeing as this (consistent high level) behavior has been in use for the last 11 years: >> http://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=fc6073d6 >> without a "real" customer complaint, I'm tending to leave as is, >> but push for better behavior from the kernel. > > There was at least one complaint about non-atomicity > ( http://thread.gmane.org/gmane.comp.gnu.coreutils.bugs/12985 ). Jim > tried to address this with > http://git.savannah.gnu.org/cgit/coreutils.git/commit/?id=63feb84a2db5246fb71df45884589b914679110c , however this non-atomicity still remains at least on this one place shown by the test-case in the mentioned rhbz #1141368. I have asked if there are some real world usecases - other than this artificial one - in the original bugzilla. Yes that was for a much more important case of _any_ hardlinked file, rather than two that are hardlinked to each other. >> I.E. add a RENAME_REPLACE flag to renameat2(), which will >> do the rename atomically for hardlinks, so we could do something like: >> >> #if !HAVE_RENAME_REPLACE >> if (same_file (a, b)) >> unlink (src); /* as we do now. */ >> #endif >> rename (a, b); >> >> thanks, >> Pádraig. >> > > Personally, although this one reproducer scenario is artificial, it > still brings potential data loss - and this is always bad. Flag in > kernel sounds like reasonable compromise - to not change existing > behaviour allowed by POSIX. It may take some time, though. Right, though my point was we have time to fix this correctly, since no-one has reported this particular case in a practical use case in the 10 years it was possible. Also all cases would still not be handled if we changed this, for example: mv /backup1/file /backup2/file & mv /backup2/file /backup1/file > However - as it is relatively corner case - change to Solaris approach > should be IMHO relatively safe (for other cases, behaviour will not > change at all). If we change this, it's much more likely that people will start complaining about their non overlapping mv instances failing. Hence I'm 55:45 for leaving as is and fixing in the right place (the kernel). thanks, Pádraig. From debbugs-submit-bounces@debbugs.gnu.org Sun Nov 16 11:35:30 2014 Received: (at 18499) by debbugs.gnu.org; 16 Nov 2014 16:35:30 +0000 Received: from localhost ([127.0.0.1]:35489 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1Xq2nS-0003Sk-BE for submit@debbugs.gnu.org; Sun, 16 Nov 2014 11:35:30 -0500 Received: from smtp.cs.ucla.edu ([131.179.128.62]:42723) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1Xq2nR-0003Sc-1M for 18499@debbugs.gnu.org; Sun, 16 Nov 2014 11:35:29 -0500 Received: from localhost (localhost.localdomain [127.0.0.1]) by smtp.cs.ucla.edu (Postfix) with ESMTP id D795DA60016; Sun, 16 Nov 2014 08:35:27 -0800 (PST) X-Virus-Scanned: amavisd-new at smtp.cs.ucla.edu Received: from smtp.cs.ucla.edu ([127.0.0.1]) by localhost (smtp.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id V55vJuhGEnz9; Sun, 16 Nov 2014 08:35:19 -0800 (PST) Received: from [192.168.1.9] (pool-71-177-17-123.lsanca.dsl-w.verizon.net [71.177.17.123]) by smtp.cs.ucla.edu (Postfix) with ESMTPSA id 2B684A6001A; Sun, 16 Nov 2014 08:35:19 -0800 (PST) Message-ID: <5468D246.2010808@cs.ucla.edu> Date: Sun, 16 Nov 2014 08:35:18 -0800 From: Paul Eggert Organization: UCLA Computer Science Department User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0 MIME-Version: 1.0 To: =?UTF-8?B?UMOhZHJhaWcgQnJhZHk=?= , ovasik@redhat.com Subject: Re: bug#18499: Possible mv race for hardlinks (rhbz #1141368 ) References: <1411037552.10808.19.camel@dhcp-24-196.brq.redhat.com> <5422EEDC.80706@draigBrady.com> <1415887133.5037.6.camel@unused-4-145.brq.redhat.com> <5464D115.3060008@draigBrady.com> <1415894773.5037.19.camel@unused-4-145.brq.redhat.com> <5466920F.6010606@draigBrady.com> <1416137884.5049.25.camel@dhcp-24-196.brq.redhat.com> <54689356.8070806@draigBrady.com> In-Reply-To: <54689356.8070806@draigBrady.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Score: -2.3 (--) X-Debbugs-Envelope-To: 18499 Cc: 18499@debbugs.gnu.org, Boris Ranto , Miklos Szeredi X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.15 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: -2.3 (--) Pádraig Brady wrote: > If we change this, it's much more likely that people will start complaining > about their non overlapping mv instances failing. I'd far rather deal with those complaints than deal with complaints about 'mv' silently discarding files. Either the FreeBSD or the Solaris behavior would be a real improvement over what we're doing now. Neither behavior is as good as having support in the kernel for doing the right thing, but one step at a time. From debbugs-submit-bounces@debbugs.gnu.org Sun Nov 16 19:28:45 2014 Received: (at 18499) by debbugs.gnu.org; 17 Nov 2014 00:28:45 +0000 Received: from localhost ([127.0.0.1]:35645 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1XqABQ-0002Kr-PO for submit@debbugs.gnu.org; Sun, 16 Nov 2014 19:28:45 -0500 Received: from mail3.vodafone.ie ([213.233.128.45]:16669) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1XqABM-0002Kg-L8 for 18499@debbugs.gnu.org; Sun, 16 Nov 2014 19:28:41 -0500 X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: ArYGAGZAaVRtT2QZ/2dsb2JhbABbDoMAglGBE1CIO8R8gyACgRIWAQEBAQF9hAMBAQQjDwFGEAsNCwICBRYLAgIJAwIBAgFFBgEMAQcBAYhBAbp+hg2PPwEBAQEGAQEBAQEBHIEtj3UHgneBVAEEpniEWYl1gzxAPYJ7AQEB Received: from unknown (HELO localhost.localdomain) ([109.79.100.25]) by mail3.vodafone.ie with ESMTP; 17 Nov 2014 00:28:38 +0000 Message-ID: <54694136.7090002@draigBrady.com> Date: Mon, 17 Nov 2014 00:28:38 +0000 From: =?UTF-8?B?UMOhZHJhaWcgQnJhZHk=?= User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0 MIME-Version: 1.0 To: Paul Eggert , ovasik@redhat.com Subject: Re: bug#18499: Possible mv race for hardlinks (rhbz #1141368 ) References: <1411037552.10808.19.camel@dhcp-24-196.brq.redhat.com> <5422EEDC.80706@draigBrady.com> <1415887133.5037.6.camel@unused-4-145.brq.redhat.com> <5464D115.3060008@draigBrady.com> <1415894773.5037.19.camel@unused-4-145.brq.redhat.com> <5466920F.6010606@draigBrady.com> <1416137884.5049.25.camel@dhcp-24-196.brq.redhat.com> <54689356.8070806@draigBrady.com> <5468D246.2010808@cs.ucla.edu> In-Reply-To: <5468D246.2010808@cs.ucla.edu> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Spam-Score: 0.0 (/) X-Debbugs-Envelope-To: 18499 Cc: 18499@debbugs.gnu.org, Boris Ranto , Miklos Szeredi X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.15 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 16/11/14 16:35, Paul Eggert wrote: > Pádraig Brady wrote: >> If we change this, it's much more likely that people will start complaining >> about their non overlapping mv instances failing. > > I'd far rather deal with those complaints than deal with complaints about 'mv' silently discarding files. Either the FreeBSD or the Solaris behavior would be a real improvement over what we're doing now. Neither behavior is as good as having support in the kernel for doing the right thing, but one step at a time. Fair enough. That's 3 votes for changing this. I'll work on a patch to fail in this case. thanks, Pádraig. From debbugs-submit-bounces@debbugs.gnu.org Tue Nov 18 11:29:52 2014 Received: (at 18499) by debbugs.gnu.org; 18 Nov 2014 16:29:52 +0000 Received: from localhost ([127.0.0.1]:37415 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1Xqlf5-0000w6-Cw for submit@debbugs.gnu.org; Tue, 18 Nov 2014 11:29:51 -0500 Received: from mx1.redhat.com ([209.132.183.28]:52614) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1Xqlf2-0000vx-OR for 18499@debbugs.gnu.org; Tue, 18 Nov 2014 11:29:49 -0500 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id sAIGTkPJ004139 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Tue, 18 Nov 2014 11:29:46 -0500 Received: from vpn-61-174.rdu2.redhat.com (vpn-61-174.rdu2.redhat.com [10.10.61.174]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id sAIGTgJU031106 (version=TLSv1/SSLv3 cipher=AES128-GCM-SHA256 bits=128 verify=NO); Tue, 18 Nov 2014 11:29:44 -0500 Message-ID: <1416328166.7154.3.camel@redhat.com> Subject: Re: bug#18499: Possible mv race for hardlinks (rhbz #1141368 ) From: Boris Ranto To: =?ISO-8859-1?Q?P=E1draig?= Brady Date: Tue, 18 Nov 2014 17:29:26 +0100 In-Reply-To: <54694136.7090002@draigBrady.com> References: <1411037552.10808.19.camel@dhcp-24-196.brq.redhat.com> <5422EEDC.80706@draigBrady.com> <1415887133.5037.6.camel@unused-4-145.brq.redhat.com> <5464D115.3060008@draigBrady.com> <1415894773.5037.19.camel@unused-4-145.brq.redhat.com> <5466920F.6010606@draigBrady.com> <1416137884.5049.25.camel@dhcp-24-196.brq.redhat.com> <54689356.8070806@draigBrady.com> <5468D246.2010808@cs.ucla.edu> <54694136.7090002@draigBrady.com> Organization: Red Hat Content-Type: multipart/mixed; boundary="=-m8vhrSEsEk+32gv2jJBw" Mime-Version: 1.0 X-Scanned-By: MIMEDefang 2.68 on 10.5.11.26 X-Spam-Score: -5.0 (-----) X-Debbugs-Envelope-To: 18499 Cc: Paul Eggert , 18499@debbugs.gnu.org, ovasik@redhat.com, Miklos Szeredi X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.15 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 (-----) --=-m8vhrSEsEk+32gv2jJBw Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8bit On Mon, 2014-11-17 at 00:28 +0000, Pádraig Brady wrote: > On 16/11/14 16:35, Paul Eggert wrote: > > Pádraig Brady wrote: > >> If we change this, it's much more likely that people will start complaining > >> about their non overlapping mv instances failing. > > > > I'd far rather deal with those complaints than deal with complaints about 'mv' silently discarding files. Either the FreeBSD or the Solaris behavior would be a real improvement over what we're doing now. Neither behavior is as good as having support in the kernel for doing the right thing, but one step at a time. > > Fair enough. That's 3 votes for changing this. > I'll work on a patch to fail in this case. > > thanks, > Pádraig. > I've looked at the code and I was able to identify the part that deals with the symlinks. I'm attaching the patch that makes mv fail in this case. btw: I noticed that cp on two distinct hard links already has this behaviour, i.e.: 'cp a b' will already exit with cp: 'a' and 'b' are the same file. -Boris --=-m8vhrSEsEk+32gv2jJBw Content-Disposition: attachment; filename*0=0001-copy-treat-hard-links-to-the-same-file-as-same-file.patc; filename*1=h Content-Transfer-Encoding: base64 Content-Type: text/x-patch; name="0001-copy-treat-hard-links-to-the-same-file-as-same-file.patch"; charset="UTF-8" RnJvbSA0NDg0NmI0MTFlMTg0MWQyMDg2NDY1NjNhMjUwOTk0YzE5NzZlZjAzIE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBCb3JpcyBSYW50byA8YnJhbnRvQHJlZGhhdC5jb20+CkRhdGU6 IFR1ZSwgMTggTm92IDIwMTQgMTc6MTU6NTAgKzAxMDAKU3ViamVjdDogW1BBVENIXSBjb3B5OiB0 cmVhdCBoYXJkIGxpbmtzIHRvIHRoZSBzYW1lIGZpbGUgYXMgc2FtZSBmaWxlCgoqIHNyYy9jb3B5 LmMgKHNhbWVfZmlsZV9vaykgOiBXZSBtYXkgcnVuIGludG8gYSByYWNlIGNvbmRpdGlvbiBpZgp3 ZSB0cmVhdCBoYXJkIGxpbmtzIHRvIHRoZSBzYW1lIGZpbGUgYXMgZGlzdGluY3QgZmlsZXMuIElm IHdlIGRvCidtdiBhIGInIGFuZCAnbXYgYiBhJyBpbiBwYXJhbGxlbCwgYm90aCBhIGFuZCBiIGNh biBkaXNhcHBlYXIgZnJvbQp0aGUgZmlsZSBzeXN0ZW0uIFRoZSByZWFzb24gaXMgdGhhdCBpbiB0 aGlzIGNhc2UgdGhlIHVubGluayBvbiBzcmMKaXMgY2FsbGVkIGFuZCB0aGUgc3lzdGVtIGNhbGxz IGNhbiBlbmQgdXAgYmVpbmcgcnVuIGluIHRoZSBvcmRlcgp3aGVyZSB1bmxpbmsoYSkgYW5kIHVu bGluayhiKSBhcmUgdGhlIGxhc3QgdHdvIHN5c3RlbSBjYWxscy4gVGhpcwpwYXRjaCBtYWtlcyB0 aGUgbXYgY2FsbHMgZXhpdCB3aXRoIGFuIGVycm9yIGNvZGUgc28gdGhhdCB3ZSBjYW4KYXZvaWQg dGhlIHBvdGVudGlhbCBkYXRhIGxvc3MgcHJvYmxlbS4KLS0tCiBzcmMvY29weS5jIHwgMjAgKysr KysrKy0tLS0tLS0tLS0tLS0KIDEgZmlsZSBjaGFuZ2VkLCA3IGluc2VydGlvbnMoKyksIDEzIGRl bGV0aW9ucygtKQoKZGlmZiAtLWdpdCBhL3NyYy9jb3B5LmMgYi9zcmMvY29weS5jCmluZGV4IDQ0 NmU3MmQuLjJmNGUyMTEgMTAwNjQ0Ci0tLSBhL3NyYy9jb3B5LmMKKysrIGIvc3JjL2NvcHkuYwpA QCAtMTU1OCwyNyArMTU1OCwyMSBAQCBzYW1lX2ZpbGVfb2sgKGNoYXIgY29uc3QgKnNyY19uYW1l LCBzdHJ1Y3Qgc3RhdCBjb25zdCAqc3JjX3NiLAogICAgIHJldHVybiB0cnVlOwogI2VuZGlmCiAK LSAgLyogVGhleSBtYXkgcmVmZXIgdG8gdGhlIHNhbWUgZmlsZSBpZiB3ZSdyZSBpbiBtb3ZlIG1v ZGUgYW5kIHRoZQotICAgICB0YXJnZXQgaXMgYSBzeW1saW5rLiAgVGhhdCBpcyBvaywgc2luY2Ug d2UgcmVtb3ZlIGFueSBleGlzdGluZwotICAgICBkZXN0aW5hdGlvbiBmaWxlIGJlZm9yZSBvcGVu aW5nIGl0IC0tIHZpYSAncmVuYW1lJyBpZiB0aGV5J3JlIG9uCi0gICAgIHRoZSBzYW1lIGZpbGUg c3lzdGVtLCB2aWEgJ3VubGluayAoRFNUX05BTUUpJyBvdGhlcndpc2UuCi0gICAgIEl0J3MgYWxz byBvayBpZiB0aGV5J3JlIGRpc3RpbmN0IGhhcmQgbGlua3MgdG8gdGhlIHNhbWUgZmlsZS4gICov CiAgIGlmICh4LT5tb3ZlX21vZGUgfHwgeC0+dW5saW5rX2Rlc3RfYmVmb3JlX29wZW5pbmcpCiAg ICAgeworICAgICAgLyogVGhleSBtYXkgcmVmZXIgdG8gdGhlIHNhbWUgZmlsZSBpZiB3ZSdyZSBp biBtb3ZlIG1vZGUgYW5kIHRoZQorICAgICAgICAgdGFyZ2V0IGlzIGEgc3ltbGluay4gIFRoYXQg aXMgb2ssIHNpbmNlIHdlIHJlbW92ZSBhbnkgZXhpc3RpbmcKKyAgICAgICAgIGRlc3RpbmF0aW9u IGZpbGUgYmVmb3JlIG9wZW5pbmcgaXQgLS0gdmlhICdyZW5hbWUnIGlmIHRoZXkncmUgb24KKyAg ICAgICAgIHRoZSBzYW1lIGZpbGUgc3lzdGVtLCB2aWEgJ3VubGluayAoRFNUX05BTUUpJyBvdGhl cndpc2UuICovCiAgICAgICBpZiAoU19JU0xOSyAoZHN0X3NiX2xpbmstPnN0X21vZGUpKQogICAg ICAgICByZXR1cm4gdHJ1ZTsKIAorICAgICAgLyogSXQncyBub3Qgb2sgaWYgdGhleSdyZSBkaXN0 aW5jdCBoYXJkIGxpbmtzIHRvIHRoZSBzYW1lIGZpbGUgYXMKKyAgICAgICAgIHRoaXMgY2F1c2Vz IGEgcmFjZSBjb25kaXRpb24gYW5kIHdlIG1heSBsb3NlIGRhdGEgaW4gdGhpcyBjYXNlLiAqLwog ICAgICAgaWYgKHNhbWVfbGluawogICAgICAgICAgICYmIDEgPCBkc3Rfc2JfbGluay0+c3Rfbmxp bmsKICAgICAgICAgICAmJiAhIHNhbWVfbmFtZSAoc3JjX25hbWUsIGRzdF9uYW1lKSkKLSAgICAg ICAgewotICAgICAgICAgIGlmICh4LT5tb3ZlX21vZGUpCi0gICAgICAgICAgICB7Ci0gICAgICAg ICAgICAgICp1bmxpbmtfc3JjID0gdHJ1ZTsKLSAgICAgICAgICAgICAgKnJldHVybl9ub3cgPSB0 cnVlOwotICAgICAgICAgICAgfQotICAgICAgICAgIHJldHVybiB0cnVlOwotICAgICAgICB9Cisg ICAgICAgIHJldHVybiBmYWxzZTsKICAgICB9CiAKICAgLyogSWYgbmVpdGhlciBpcyBhIHN5bWxp bmssIHRoZW4gaXQncyBvayBhcyBsb25nIGFzIHRoZXkgYXJlbid0Ci0tIAoxLjkuMwoK --=-m8vhrSEsEk+32gv2jJBw-- From debbugs-submit-bounces@debbugs.gnu.org Tue Nov 18 11:46:56 2014 Received: (at 18499) by debbugs.gnu.org; 18 Nov 2014 16:46:56 +0000 Received: from localhost ([127.0.0.1]:37436 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1Xqlvb-0001N5-Lr for submit@debbugs.gnu.org; Tue, 18 Nov 2014 11:46:55 -0500 Received: from mail6.vodafone.ie ([213.233.128.184]:2527) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1XqlvY-0001Mt-9r for 18499@debbugs.gnu.org; Tue, 18 Nov 2014 11:46:53 -0500 X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: Ag4FAKV2a1RtTk5C/2dsb2JhbABRCoMOg2RQzUeDHAKBDRYBAQEBAX2EAwEBBCMPAUYQCw0BCgICBRYLAgIJAwIBAgFFBg0BBwEBiEEBuh+GDJBeAQEIAQEBAR6BLY5/XAeCd4FUBacAhFeJeIN7PYJ7AQEB Received: from unknown (HELO localhost.localdomain) ([109.78.78.66]) by mail3.vodafone.ie with ESMTP; 18 Nov 2014 16:46:47 +0000 Message-ID: <546B77F7.3010900@draigBrady.com> Date: Tue, 18 Nov 2014 16:46:47 +0000 From: =?UTF-8?B?UMOhZHJhaWcgQnJhZHk=?= User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0 MIME-Version: 1.0 To: Boris Ranto Subject: Re: bug#18499: Possible mv race for hardlinks (rhbz #1141368 ) References: <1411037552.10808.19.camel@dhcp-24-196.brq.redhat.com> <5422EEDC.80706@draigBrady.com> <1415887133.5037.6.camel@unused-4-145.brq.redhat.com> <5464D115.3060008@draigBrady.com> <1415894773.5037.19.camel@unused-4-145.brq.redhat.com> <5466920F.6010606@draigBrady.com> <1416137884.5049.25.camel@dhcp-24-196.brq.redhat.com> <54689356.8070806@draigBrady.com> <5468D246.2010808@cs.ucla.edu> <54694136.7090002@draigBrady.com> <1416328166.7154.3.camel@redhat.com> In-Reply-To: <1416328166.7154.3.camel@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Spam-Score: 0.0 (/) X-Debbugs-Envelope-To: 18499 Cc: Paul Eggert , 18499@debbugs.gnu.org, ovasik@redhat.com, Miklos Szeredi X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.15 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 18/11/14 16:29, Boris Ranto wrote: > On Mon, 2014-11-17 at 00:28 +0000, Pádraig Brady wrote: >> On 16/11/14 16:35, Paul Eggert wrote: >>> Pádraig Brady wrote: >>>> If we change this, it's much more likely that people will start complaining >>>> about their non overlapping mv instances failing. >>> >>> I'd far rather deal with those complaints than deal with complaints about 'mv' silently discarding files. Either the FreeBSD or the Solaris behavior would be a real improvement over what we're doing now. Neither behavior is as good as having support in the kernel for doing the right thing, but one step at a time. >> >> Fair enough. That's 3 votes for changing this. >> I'll work on a patch to fail in this case. >> >> thanks, >> Pádraig. >> > > I've looked at the code and I was able to identify the part that deals > with the symlinks. I'm attaching the patch that makes mv fail in this > case. I'm not sure symlinks should be treated differently here. I.E. it may be best to remove the whole unlink_src logic. I'll look later. thanks, Pádraig. From debbugs-submit-bounces@debbugs.gnu.org Tue Nov 18 14:28:30 2014 Received: (at 18499) by debbugs.gnu.org; 18 Nov 2014 19:28:30 +0000 Received: from localhost ([127.0.0.1]:37535 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1XqoRx-0005NG-6m for submit@debbugs.gnu.org; Tue, 18 Nov 2014 14:28:29 -0500 Received: from mx1.redhat.com ([209.132.183.28]:51531) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1XqoRu-0005N5-By for 18499@debbugs.gnu.org; Tue, 18 Nov 2014 14:28:27 -0500 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id sAIJSNQ8010955 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Tue, 18 Nov 2014 14:28:23 -0500 Received: from vpn-49-113.rdu2.redhat.com (vpn-49-113.rdu2.redhat.com [10.10.49.113]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id sAIJSJu6015881 (version=TLSv1/SSLv3 cipher=AES128-GCM-SHA256 bits=128 verify=NO); Tue, 18 Nov 2014 14:28:21 -0500 Message-ID: <1416338883.7154.5.camel@redhat.com> Subject: Re: bug#18499: Possible mv race for hardlinks (rhbz #1141368 ) From: Boris Ranto To: =?ISO-8859-1?Q?P=E1draig?= Brady Date: Tue, 18 Nov 2014 20:28:03 +0100 In-Reply-To: <546B77F7.3010900@draigBrady.com> References: <1411037552.10808.19.camel@dhcp-24-196.brq.redhat.com> <5422EEDC.80706@draigBrady.com> <1415887133.5037.6.camel@unused-4-145.brq.redhat.com> <5464D115.3060008@draigBrady.com> <1415894773.5037.19.camel@unused-4-145.brq.redhat.com> <5466920F.6010606@draigBrady.com> <1416137884.5049.25.camel@dhcp-24-196.brq.redhat.com> <54689356.8070806@draigBrady.com> <5468D246.2010808@cs.ucla.edu> <54694136.7090002@draigBrady.com> <1416328166.7154.3.camel@redhat.com> <546B77F7.3010900@draigBrady.com> Organization: Red Hat Content-Type: multipart/mixed; boundary="=-16LHYTTwpHDyNT/IGAJr" Mime-Version: 1.0 X-Scanned-By: MIMEDefang 2.68 on 10.5.11.26 X-Spam-Score: -5.0 (-----) X-Debbugs-Envelope-To: 18499 Cc: Paul Eggert , 18499@debbugs.gnu.org, ovasik@redhat.com, Miklos Szeredi X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.15 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 (-----) --=-16LHYTTwpHDyNT/IGAJr Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8bit On Tue, 2014-11-18 at 16:46 +0000, Pádraig Brady wrote: > On 18/11/14 16:29, Boris Ranto wrote: > > On Mon, 2014-11-17 at 00:28 +0000, Pádraig Brady wrote: > >> On 16/11/14 16:35, Paul Eggert wrote: > >>> Pádraig Brady wrote: > >>>> If we change this, it's much more likely that people will start complaining > >>>> about their non overlapping mv instances failing. > >>> > >>> I'd far rather deal with those complaints than deal with complaints about 'mv' silently discarding files. Either the FreeBSD or the Solaris behavior would be a real improvement over what we're doing now. Neither behavior is as good as having support in the kernel for doing the right thing, but one step at a time. > >> > >> Fair enough. That's 3 votes for changing this. > >> I'll work on a patch to fail in this case. > >> > >> thanks, > >> Pádraig. > >> > > > > I've looked at the code and I was able to identify the part that deals > > with the symlinks. I'm attaching the patch that makes mv fail in this > > case. > > I'm not sure symlinks should be treated differently here. > I.E. it may be best to remove the whole unlink_src logic. > I'll look later. > > thanks, > Pádraig. You were right, the only other place that used the unlink_src logic was the case handling mv for symlinks that were hard links to the same file (this case was handled separetely from the normal files) -- i.e. the case where you do this: touch a; ln -s a b; ln b c; mv b c I'm attaching the revised patch that removes the whole unlink_src logic altogether. -Boris --=-16LHYTTwpHDyNT/IGAJr Content-Disposition: attachment; filename*0=0001-copy-treat-hard-links-to-the-same-file-as-same-file.patc; filename*1=h Content-Transfer-Encoding: base64 Content-Type: text/x-patch; name="0001-copy-treat-hard-links-to-the-same-file-as-same-file.patch"; charset="UTF-8" RnJvbSBkNTVmMThhNTBlMmU3Mjk4YWU3MjEwNmZhNTNmZmViN2Y3MzAzZjVmIE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBCb3JpcyBSYW50byA8YnJhbnRvQHJlZGhhdC5jb20+CkRhdGU6 IFR1ZSwgMTggTm92IDIwMTQgMjA6MjA6NTAgKzAxMDAKU3ViamVjdDogW1BBVENIXSBjb3B5OiB0 cmVhdCBoYXJkIGxpbmtzIHRvIHRoZSBzYW1lIGZpbGUgYXMgc2FtZSBmaWxlCgoqIHNyYy9jb3B5 LmMgKHNhbWVfZmlsZV9vaykgOiBXZSBtYXkgcnVuIGludG8gYSByYWNlIGNvbmRpdGlvbiBpZgp3 ZSB0cmVhdCBoYXJkIGxpbmtzIHRvIHRoZSBzYW1lIGZpbGUgYXMgZGlzdGluY3QgZmlsZXMuIElm IHdlIGRvCidtdiBhIGInIGFuZCAnbXYgYiBhJyBpbiBwYXJhbGxlbCwgYm90aCBhIGFuZCBiIGNh biBkaXNhcHBlYXIgZnJvbQp0aGUgZmlsZSBzeXN0ZW0uIFRoZSByZWFzb24gaXMgdGhhdCBpbiB0 aGlzIGNhc2UgdGhlIHVubGluayBvbiBzcmMKaXMgY2FsbGVkIGFuZCB0aGUgc3lzdGVtIGNhbGxz IGNhbiBlbmQgdXAgYmVpbmcgcnVuIGluIHRoZSBvcmRlcgp3aGVyZSB1bmxpbmsoYSkgYW5kIHVu bGluayhiKSBhcmUgdGhlIGxhc3QgdHdvIHN5c3RlbSBjYWxscy4gVGhpcwpwYXRjaCBtYWtlcyB0 aGUgbXYgY2FsbHMgZXhpdCB3aXRoIGFuIGVycm9yIGNvZGUgc28gdGhhdCB3ZSBjYW4KYXZvaWQg dGhlIHBvdGVudGlhbCBkYXRhIGxvc3MgcHJvYmxlbS4gVGhlIHBhdGNoIGFsc28gcmVtb3ZlcyB0 aGUKdW5saW5rX3NyYyBvcHRpb24gdG8gc2FtZV9maWxlX29rIHdoaWNoIGlzIG5vIGxvbmdlciBu ZWNlc3NhcnkuCipzcmMvY29weS5jIChjb3B5X2ludGVybmFsKTogV2UgbmVlZCB0byBwYXRjaCB1 cCB0aGlzIGZ1bmN0aW9uIHRvCm5vdCB1c2UgdGhlIHVubGlua19zcmMgb3B0aW9uIHdoaWNoIHdl IHJlbW92ZWQgZnJvbSBzYW1lX2ZpbGVfb2suCi0tLQogc3JjL2NvcHkuYyB8IDUwICsrKysrKysr KysrKy0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tCiAxIGZpbGUgY2hhbmdl ZCwgMTIgaW5zZXJ0aW9ucygrKSwgMzggZGVsZXRpb25zKC0pCgpkaWZmIC0tZ2l0IGEvc3JjL2Nv cHkuYyBiL3NyYy9jb3B5LmMKaW5kZXggNDQ2ZTcyZC4uMDljYmJhNyAxMDA2NDQKLS0tIGEvc3Jj L2NvcHkuYworKysgYi9zcmMvY29weS5jCkBAIC0xNDA3LDIwICsxNDA3LDEyIEBAIGNsb3NlX3Ny Y19kZXNjOgogICAgY29weSBhIHJlZ3VsYXIgZmlsZSBvbnRvIGEgc3ltbGluayB0aGF0IHBvaW50 cyB0byBpdC4KICAgIFRyeSB0byBtaW5pbWl6ZSB0aGUgY29zdCBvZiB0aGlzIGZ1bmN0aW9uIGlu IHRoZSBjb21tb24gY2FzZS4KICAgIFNldCAqUkVUVVJOX05PVyBpZiB3ZSd2ZSBkZXRlcm1pbmVk IHRoYXQgdGhlIGNhbGxlciBoYXMgbm8gbW9yZQotICAgd29yayB0byBkbyBhbmQgc2hvdWxkIHJl dHVybiBzdWNjZXNzZnVsbHksIHJpZ2h0IGF3YXkuCi0KLSAgIFNldCAqVU5MSU5LX1NSQyBpZiB3 ZSd2ZSBkZXRlcm1pbmVkIHRoYXQgdGhlIGNhbGxlciB3YW50cyB0byBkbwotICAgJ3JlbmFtZSAo YSwgYiknIHdoZXJlICdhJyBhbmQgJ2InIGFyZSBkaXN0aW5jdCBoYXJkIGxpbmtzIHRvIHRoZSBz YW1lCi0gICBmaWxlLiBJbiB0aGF0IGNhc2UsIHRoZSBjYWxsZXIgc2hvdWxkIHRyeSB0byB1bmxp bmsgJ2EnIGFuZCB0aGVuIHJldHVybgotICAgc3VjY2Vzc2Z1bGx5LiAgSWRlYWxseSwgd2Ugd291 bGRuJ3QgaGF2ZSB0byBkbyB0aGF0LCBhbmQgd2UnZCBiZQotICAgYWJsZSB0byByZWx5IG9uIHJl bmFtZSB0byByZW1vdmUgdGhlIHNvdXJjZSBmaWxlLiAgSG93ZXZlciwgUE9TSVgKLSAgIG1pc3Rh a2VubHkgcmVxdWlyZXMgdGhhdCBzdWNoIGEgcmVuYW1lIGNhbGwgZG8gKm5vdGhpbmcqIGFuZCBy ZXR1cm4KLSAgIHN1Y2Nlc3NmdWxseS4gICovCisgICB3b3JrIHRvIGRvIGFuZCBzaG91bGQgcmV0 dXJuIHN1Y2Nlc3NmdWxseSwgcmlnaHQgYXdheS4gKi8KIAogc3RhdGljIGJvb2wKIHNhbWVfZmls ZV9vayAoY2hhciBjb25zdCAqc3JjX25hbWUsIHN0cnVjdCBzdGF0IGNvbnN0ICpzcmNfc2IsCiAg ICAgICAgICAgICAgIGNoYXIgY29uc3QgKmRzdF9uYW1lLCBzdHJ1Y3Qgc3RhdCBjb25zdCAqZHN0 X3NiLAotICAgICAgICAgICAgICBjb25zdCBzdHJ1Y3QgY3Bfb3B0aW9ucyAqeCwgYm9vbCAqcmV0 dXJuX25vdywgYm9vbCAqdW5saW5rX3NyYykKKyAgICAgICAgICAgICAgY29uc3Qgc3RydWN0IGNw X29wdGlvbnMgKngsIGJvb2wgKnJldHVybl9ub3cpCiB7CiAgIGNvbnN0IHN0cnVjdCBzdGF0ICpz cmNfc2JfbGluazsKICAgY29uc3Qgc3RydWN0IHN0YXQgKmRzdF9zYl9saW5rOwpAQCAtMTQzMSw3 ICsxNDIzLDYgQEAgc2FtZV9maWxlX29rIChjaGFyIGNvbnN0ICpzcmNfbmFtZSwgc3RydWN0IHN0 YXQgY29uc3QgKnNyY19zYiwKICAgYm9vbCBzYW1lID0gU0FNRV9JTk9ERSAoKnNyY19zYiwgKmRz dF9zYik7CiAKICAgKnJldHVybl9ub3cgPSBmYWxzZTsKLSAgKnVubGlua19zcmMgPSBmYWxzZTsK IAogICAvKiBGSVhNRTogdGhpcyBzaG91bGQgKGF0IHRoZSB2ZXJ5IGxlYXN0KSBiZSBtb3ZlZCBp bnRvIHRoZSBmb2xsb3dpbmcKICAgICAgaWYtYmxvY2suICBNb3JlIGxpa2VseSwgaXQgc2hvdWxk IGJlIHJlbW92ZWQsIGJlY2F1c2UgaXQgaW5oaWJpdHMKQEAgLTE0NjgsOSArMTQ1OSw3IEBAIHNh bWVfZmlsZV9vayAoY2hhciBjb25zdCAqc3JjX25hbWUsIHN0cnVjdCBzdGF0IGNvbnN0ICpzcmNf c2IsCiAgICAgICAgICAgICAgICAgIGNhbGxlciB0aGF0IGFsbCBpdCBtdXN0IGRvIGlzIHVubGlu ayBBIGFuZCByZXR1cm4uICAqLwogICAgICAgICAgICAgICBpZiAoc2FtZV9saW5rKQogICAgICAg ICAgICAgICAgIHsKLSAgICAgICAgICAgICAgICAgICp1bmxpbmtfc3JjID0gdHJ1ZTsKLSAgICAg ICAgICAgICAgICAgICpyZXR1cm5fbm93ID0gdHJ1ZTsKLSAgICAgICAgICAgICAgICAgIHJldHVy biB0cnVlOworICAgICAgICAgICAgICAgICAgcmV0dXJuIGZhbHNlOwogICAgICAgICAgICAgICAg IH0KICAgICAgICAgICAgIH0KIApAQCAtMTU1OCwyNyArMTU0NywyMSBAQCBzYW1lX2ZpbGVfb2sg KGNoYXIgY29uc3QgKnNyY19uYW1lLCBzdHJ1Y3Qgc3RhdCBjb25zdCAqc3JjX3NiLAogICAgIHJl dHVybiB0cnVlOwogI2VuZGlmCiAKLSAgLyogVGhleSBtYXkgcmVmZXIgdG8gdGhlIHNhbWUgZmls ZSBpZiB3ZSdyZSBpbiBtb3ZlIG1vZGUgYW5kIHRoZQotICAgICB0YXJnZXQgaXMgYSBzeW1saW5r LiAgVGhhdCBpcyBvaywgc2luY2Ugd2UgcmVtb3ZlIGFueSBleGlzdGluZwotICAgICBkZXN0aW5h dGlvbiBmaWxlIGJlZm9yZSBvcGVuaW5nIGl0IC0tIHZpYSAncmVuYW1lJyBpZiB0aGV5J3JlIG9u Ci0gICAgIHRoZSBzYW1lIGZpbGUgc3lzdGVtLCB2aWEgJ3VubGluayAoRFNUX05BTUUpJyBvdGhl cndpc2UuCi0gICAgIEl0J3MgYWxzbyBvayBpZiB0aGV5J3JlIGRpc3RpbmN0IGhhcmQgbGlua3Mg dG8gdGhlIHNhbWUgZmlsZS4gICovCiAgIGlmICh4LT5tb3ZlX21vZGUgfHwgeC0+dW5saW5rX2Rl c3RfYmVmb3JlX29wZW5pbmcpCiAgICAgeworICAgICAgLyogVGhleSBtYXkgcmVmZXIgdG8gdGhl IHNhbWUgZmlsZSBpZiB3ZSdyZSBpbiBtb3ZlIG1vZGUgYW5kIHRoZQorICAgICAgICAgdGFyZ2V0 IGlzIGEgc3ltbGluay4gIFRoYXQgaXMgb2ssIHNpbmNlIHdlIHJlbW92ZSBhbnkgZXhpc3RpbmcK KyAgICAgICAgIGRlc3RpbmF0aW9uIGZpbGUgYmVmb3JlIG9wZW5pbmcgaXQgLS0gdmlhICdyZW5h bWUnIGlmIHRoZXkncmUgb24KKyAgICAgICAgIHRoZSBzYW1lIGZpbGUgc3lzdGVtLCB2aWEgJ3Vu bGluayAoRFNUX05BTUUpJyBvdGhlcndpc2UuICovCiAgICAgICBpZiAoU19JU0xOSyAoZHN0X3Ni X2xpbmstPnN0X21vZGUpKQogICAgICAgICByZXR1cm4gdHJ1ZTsKIAorICAgICAgLyogSXQncyBu b3Qgb2sgaWYgdGhleSdyZSBkaXN0aW5jdCBoYXJkIGxpbmtzIHRvIHRoZSBzYW1lIGZpbGUgYXMK KyAgICAgICAgIHRoaXMgY2F1c2VzIGEgcmFjZSBjb25kaXRpb24gYW5kIHdlIG1heSBsb3NlIGRh dGEgaW4gdGhpcyBjYXNlLiAqLwogICAgICAgaWYgKHNhbWVfbGluawogICAgICAgICAgICYmIDEg PCBkc3Rfc2JfbGluay0+c3RfbmxpbmsKICAgICAgICAgICAmJiAhIHNhbWVfbmFtZSAoc3JjX25h bWUsIGRzdF9uYW1lKSkKLSAgICAgICAgewotICAgICAgICAgIGlmICh4LT5tb3ZlX21vZGUpCi0g ICAgICAgICAgICB7Ci0gICAgICAgICAgICAgICp1bmxpbmtfc3JjID0gdHJ1ZTsKLSAgICAgICAg ICAgICAgKnJldHVybl9ub3cgPSB0cnVlOwotICAgICAgICAgICAgfQotICAgICAgICAgIHJldHVy biB0cnVlOwotICAgICAgICB9CisgICAgICAgIHJldHVybiBmYWxzZTsKICAgICB9CiAKICAgLyog SWYgbmVpdGhlciBpcyBhIHN5bWxpbmssIHRoZW4gaXQncyBvayBhcyBsb25nIGFzIHRoZXkgYXJl bid0CkBAIC0xOTM5LDExICsxOTIyLDEwIEBAIGNvcHlfaW50ZXJuYWwgKGNoYXIgY29uc3QgKnNy Y19uYW1lLCBjaGFyIGNvbnN0ICpkc3RfbmFtZSwKICAgICAgICAgeyAvKiBIZXJlLCB3ZSBrbm93 IHRoYXQgZHN0X25hbWUgZXhpc3RzLCBhdCBsZWFzdCB0byB0aGUgcG9pbnQKICAgICAgICAgICAg ICB0aGF0IGl0IGlzIHN0YXQnYWJsZSBvciBsc3RhdCdhYmxlLiAgKi8KICAgICAgICAgICBib29s IHJldHVybl9ub3c7Ci0gICAgICAgICAgYm9vbCB1bmxpbmtfc3JjOwogCiAgICAgICAgICAgaGF2 ZV9kc3RfbHN0YXQgPSAhdXNlX3N0YXQ7CiAgICAgICAgICAgaWYgKCEgc2FtZV9maWxlX29rIChz cmNfbmFtZSwgJnNyY19zYiwgZHN0X25hbWUsICZkc3Rfc2IsCi0gICAgICAgICAgICAgICAgICAg ICAgICAgICAgICB4LCAmcmV0dXJuX25vdywgJnVubGlua19zcmMpKQorICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgeCwgJnJldHVybl9ub3cpKQogICAgICAgICAgICAgewogICAgICAgICAg ICAgICBlcnJvciAoMCwgMCwgXygiJXMgYW5kICVzIGFyZSB0aGUgc2FtZSBmaWxlIiksCiAgICAg ICAgICAgICAgICAgICAgICBxdW90ZV9uICgwLCBzcmNfbmFtZSksIHF1b3RlX24gKDEsIGRzdF9u YW1lKSk7CkBAIC0yMDAyLDIyICsxOTg0LDE0IEBAIGNvcHlfaW50ZXJuYWwgKGNoYXIgY29uc3Qg KnNyY19uYW1lLCBjaGFyIGNvbnN0ICpkc3RfbmFtZSwKICAgICAgICAgICAgICBjcCBhbmQgbXYg dHJlYXQgLWkgYW5kIC1mIGRpZmZlcmVudGx5LiAgKi8KICAgICAgICAgICBpZiAoeC0+bW92ZV9t b2RlKQogICAgICAgICAgICAgewotICAgICAgICAgICAgICBpZiAoYWJhbmRvbl9tb3ZlICh4LCBk c3RfbmFtZSwgJmRzdF9zYikKLSAgICAgICAgICAgICAgICAgIHx8ICh1bmxpbmtfc3JjICYmIHVu bGluayAoc3JjX25hbWUpID09IDApKQorICAgICAgICAgICAgICBpZiAoYWJhbmRvbl9tb3ZlICh4 LCBkc3RfbmFtZSwgJmRzdF9zYikpCiAgICAgICAgICAgICAgICAgewogICAgICAgICAgICAgICAg ICAgLyogUHJldGVuZCB0aGUgcmVuYW1lIHN1Y2NlZWRlZCwgc28gdGhlIGNhbGxlciAobXYpCiAg ICAgICAgICAgICAgICAgICAgICBkb2Vzbid0IGVuZCB1cCByZW1vdmluZyB0aGUgc291cmNlIGZp bGUuICAqLwogICAgICAgICAgICAgICAgICAgaWYgKHJlbmFtZV9zdWNjZWVkZWQpCiAgICAgICAg ICAgICAgICAgICAgICpyZW5hbWVfc3VjY2VlZGVkID0gdHJ1ZTsKLSAgICAgICAgICAgICAgICAg IGlmICh1bmxpbmtfc3JjICYmIHgtPnZlcmJvc2UpCi0gICAgICAgICAgICAgICAgICAgIHByaW50 ZiAoXygicmVtb3ZlZCAlc1xuIiksIHF1b3RlIChzcmNfbmFtZSkpOwogICAgICAgICAgICAgICAg ICAgcmV0dXJuIHRydWU7CiAgICAgICAgICAgICAgICAgfQotICAgICAgICAgICAgICBpZiAodW5s aW5rX3NyYykKLSAgICAgICAgICAgICAgICB7Ci0gICAgICAgICAgICAgICAgICBlcnJvciAoMCwg ZXJybm8sIF8oImNhbm5vdCByZW1vdmUgJXMiKSwgcXVvdGUgKHNyY19uYW1lKSk7Ci0gICAgICAg ICAgICAgICAgICByZXR1cm4gZmFsc2U7Ci0gICAgICAgICAgICAgICAgfQogICAgICAgICAgICAg fQogICAgICAgICAgIGVsc2UKICAgICAgICAgICAgIHsKLS0gCjEuOS4zCgo= --=-16LHYTTwpHDyNT/IGAJr-- From debbugs-submit-bounces@debbugs.gnu.org Thu Nov 20 22:30:59 2014 Received: (at 18499) by debbugs.gnu.org; 21 Nov 2014 03:30:59 +0000 Received: from localhost ([127.0.0.1]:40082 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1Xrevx-0002re-UM for submit@debbugs.gnu.org; Thu, 20 Nov 2014 22:30:59 -0500 Received: from mail2.vodafone.ie ([213.233.128.44]:43473) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1Xrevt-0002rU-Eo for 18499@debbugs.gnu.org; Thu, 20 Nov 2014 22:30:56 -0500 X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: Am4JAF6wblRtTZiq/2dsb2JhbABQBAaDDlVZgjZQvmaJeodGAQICgQYWAQEBAQF9hAIBAQEEIwRSEAsNAQMDAQIBCRQCCwICCQMCAQIBPQgGCgMBAwICAQEFEQGIKgEIvUmGC5BTAQEBAQEFAQEBAQEBAQEBGZAdDxkyEQcJgm6BVAWGRotngkuBUmuCTYYSPYY6hFeDB4JqhAmDez0ELIJLAQEB Received: from unknown (HELO localhost.localdomain) ([109.77.152.170]) by mail2.vodafone.ie with ESMTP; 21 Nov 2014 03:30:51 +0000 Message-ID: <546EB1EA.7030307@draigBrady.com> Date: Fri, 21 Nov 2014 03:30:50 +0000 From: =?UTF-8?B?UMOhZHJhaWcgQnJhZHk=?= User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0 MIME-Version: 1.0 To: Boris Ranto Subject: Re: bug#18499: Possible mv race for hardlinks (rhbz #1141368 ) References: <1411037552.10808.19.camel@dhcp-24-196.brq.redhat.com> <5422EEDC.80706@draigBrady.com> <1415887133.5037.6.camel@unused-4-145.brq.redhat.com> <5464D115.3060008@draigBrady.com> <1415894773.5037.19.camel@unused-4-145.brq.redhat.com> <5466920F.6010606@draigBrady.com> <1416137884.5049.25.camel@dhcp-24-196.brq.redhat.com> <54689356.8070806@draigBrady.com> <5468D246.2010808@cs.ucla.edu> <54694136.7090002@draigBrady.com> <1416328166.7154.3.camel@redhat.com> <546B77F7.3010900@draigBrady.com> <1416338883.7154.5.camel@redhat.com> In-Reply-To: <1416338883.7154.5.camel@redhat.com> Content-Type: multipart/mixed; boundary="------------000603040905010301080800" X-Spam-Score: 0.0 (/) X-Debbugs-Envelope-To: 18499 Cc: Paul Eggert , 18499@debbugs.gnu.org, ovasik@redhat.com, Miklos Szeredi X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.15 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. --------------000603040905010301080800 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit On 18/11/14 19:28, Boris Ranto wrote: > On Tue, 2014-11-18 at 16:46 +0000, Pádraig Brady wrote: >> On 18/11/14 16:29, Boris Ranto wrote: >>> On Mon, 2014-11-17 at 00:28 +0000, Pádraig Brady wrote: >>>> On 16/11/14 16:35, Paul Eggert wrote: >>>>> Pádraig Brady wrote: >>>>>> If we change this, it's much more likely that people will start complaining >>>>>> about their non overlapping mv instances failing. >>>>> >>>>> I'd far rather deal with those complaints than deal with complaints about 'mv' silently discarding files. Either the FreeBSD or the Solaris behavior would be a real improvement over what we're doing now. Neither behavior is as good as having support in the kernel for doing the right thing, but one step at a time. >>>> >>>> Fair enough. That's 3 votes for changing this. >>>> I'll work on a patch to fail in this case. >>>> >>>> thanks, >>>> Pádraig. >>>> >>> >>> I've looked at the code and I was able to identify the part that deals >>> with the symlinks. I'm attaching the patch that makes mv fail in this >>> case. >> >> I'm not sure symlinks should be treated differently here. >> I.E. it may be best to remove the whole unlink_src logic. >> I'll look later. >> >> thanks, >> Pádraig. > > You were right, the only other place that used the unlink_src logic was > the case handling mv for symlinks that were hard links to the same file > (this case was handled separetely from the normal files) -- i.e. the > case where you do this: > touch a; ln -s a b; ln b c; mv b c > > I'm attaching the revised patch that removes the whole unlink_src logic > altogether. We want to leave the logic in place for cp and install though, and I've adjusted your patch accordingly. I've also adjusted the tests to pass and augmented the tests to cover one of the cases missed in the previous patch. I'll push this tomorrow. thanks, Pádraig. --------------000603040905010301080800 Content-Type: text/x-patch; name="mv-hardlink-race.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="mv-hardlink-race.patch" =46rom 222d7ac0c4f5f005438c534f3aba62fd94d96dc2 Mon Sep 17 00:00:00 2001 From: Boris Ranto Date: Tue, 18 Nov 2014 20:20:50 +0100 Subject: [PATCH] mv: fail when moving a file to a hardlink We may run into a race condition if we treat hard links to the same file as distinct files. If we do 'mv a b' and 'mv b a' in parallel, both a and b can disappear from the file system. The reason is that in this case the unlink on src is called and the system calls can end up being run in the order where unlink(a) and unlink(b) are the last two system calls. Therefore exit with an error code so that we avoid the potential data loss. * src/copy.c (same_file_ok): Don't set unlink_src that was used by mv, and return false for two hardlinks to a file in move_mode. *src/copy.c (copy_internal): No longer honor the unlink_src option, used only by mv. NEWS: Mention the change in behavior. * tests/cp/same-file.sh: Augment to cover the `cp -a hlsl1 sl1` case. * tests/mv/hard-verbose.sh: Remove no longer needed test. * tests/local.mk: Remove the reference to hard-verbose.sh. * tests/mv/hard-4.sh: Adjust so we fail in this case. * tests/mv/i-4.sh: Likewise. * tests/mv/symlink-onto-hardlink-to-self.sh: Likewise. --- NEWS | 7 ++++ src/copy.c | 53 ++++++++-----------------= ------ tests/cp/same-file.sh | 28 +++++++++++++--- tests/local.mk | 1 - tests/mv/force.sh | 21 ++++++------ tests/mv/hard-4.sh | 17 ++++++---- tests/mv/hard-verbose.sh | 33 ------------------- tests/mv/i-4.sh | 14 ++++---- tests/mv/symlink-onto-hardlink-to-self.sh | 35 ++++++++++++-------- 9 files changed, 92 insertions(+), 117 deletions(-) delete mode 100755 tests/mv/hard-verbose.sh diff --git a/NEWS b/NEWS index 5fbdc6a..6a84c48 100644 --- a/NEWS +++ b/NEWS @@ -30,6 +30,13 @@ GNU coreutils NEWS = -*- outline -*- dd accepts a new status=3Dprogress level to print data transfer statis= tics on stderr approximately every second. =20 +** Changes in behavior + + mv no longer supports moving a file to a hardlink, as currently that + is only supported through unlinking the source file. That's racy + in the presence of multiple mv instances, which could result in both + hardlinks being deleted. This feature was added in coreutils-5.0.1. + ** Improvements =20 cp,install,mv will convert smaller runs of NULs in the input to holes,= diff --git a/src/copy.c b/src/copy.c index 446e72d..01cee32 100644 --- a/src/copy.c +++ b/src/copy.c @@ -1407,20 +1407,12 @@ close_src_desc: copy a regular file onto a symlink that points to it. Try to minimize the cost of this function in the common case. Set *RETURN_NOW if we've determined that the caller has no more - work to do and should return successfully, right away. - - Set *UNLINK_SRC if we've determined that the caller wants to do - 'rename (a, b)' where 'a' and 'b' are distinct hard links to the same= - file. In that case, the caller should try to unlink 'a' and then retu= rn - successfully. Ideally, we wouldn't have to do that, and we'd be - able to rely on rename to remove the source file. However, POSIX - mistakenly requires that such a rename call do *nothing* and return - successfully. */ + work to do and should return successfully, right away. */ =20 static bool same_file_ok (char const *src_name, struct stat const *src_sb, char const *dst_name, struct stat const *dst_sb, - const struct cp_options *x, bool *return_now, bool *unlink= _src) + const struct cp_options *x, bool *return_now) { const struct stat *src_sb_link; const struct stat *dst_sb_link; @@ -1431,7 +1423,6 @@ same_file_ok (char const *src_name, struct stat con= st *src_sb, bool same =3D SAME_INODE (*src_sb, *dst_sb); =20 *return_now =3D false; - *unlink_src =3D false; =20 /* FIXME: this should (at the very least) be moved into the following if-block. More likely, it should be removed, because it inhibits @@ -1463,14 +1454,11 @@ same_file_ok (char const *src_name, struct stat c= onst *src_sb, /* Here we have two symlinks that are hard-linked together= , and we're not making backups. In this unusual case, si= mply returning true would lead to mv calling "rename(A,B)", - which would do nothing and return 0. I.e., A would - not be removed. Hence, the solution is to tell the - caller that all it must do is unlink A and return. */ + which would do nothing and return 0. */ if (same_link) { - *unlink_src =3D true; *return_now =3D true; - return true; + return ! x->move_mode; } } =20 @@ -1558,27 +1546,21 @@ same_file_ok (char const *src_name, struct stat c= onst *src_sb, return true; #endif =20 - /* They may refer to the same file if we're in move mode and the - target is a symlink. That is ok, since we remove any existing - destination file before opening it -- via 'rename' if they're on - the same file system, via 'unlink (DST_NAME)' otherwise. - It's also ok if they're distinct hard links to the same file. */ if (x->move_mode || x->unlink_dest_before_opening) { + /* They may refer to the same file if we're in move mode and the + target is a symlink. That is ok, since we remove any existing + destination file before opening it -- via 'rename' if they're o= n + the same file system, via 'unlink (DST_NAME)' otherwise. */ if (S_ISLNK (dst_sb_link->st_mode)) return true; =20 + /* It's not ok if they're distinct hard links to the same file as + this causes a race condition and we may lose data in this case.= */ if (same_link && 1 < dst_sb_link->st_nlink && ! same_name (src_name, dst_name)) - { - if (x->move_mode) - { - *unlink_src =3D true; - *return_now =3D true; - } - return true; - } + return ! x->move_mode; } =20 /* If neither is a symlink, then it's ok as long as they aren't @@ -1939,11 +1921,10 @@ copy_internal (char const *src_name, char const *= dst_name, { /* Here, we know that dst_name exists, at least to the point that it is stat'able or lstat'able. */ bool return_now; - bool unlink_src; =20 have_dst_lstat =3D !use_stat; if (! same_file_ok (src_name, &src_sb, dst_name, &dst_sb, - x, &return_now, &unlink_src)) + x, &return_now)) { error (0, 0, _("%s and %s are the same file"), quote_n (0, src_name), quote_n (1, dst_name)); @@ -2002,22 +1983,14 @@ copy_internal (char const *src_name, char const *= dst_name, cp and mv treat -i and -f differently. */ if (x->move_mode) { - if (abandon_move (x, dst_name, &dst_sb) - || (unlink_src && unlink (src_name) =3D=3D 0)) + if (abandon_move (x, dst_name, &dst_sb)) { /* Pretend the rename succeeded, so the caller (mv) doesn't end up removing the source file. */ if (rename_succeeded) *rename_succeeded =3D true; - if (unlink_src && x->verbose) - printf (_("removed %s\n"), quote (src_name)); return true; } - if (unlink_src) - { - error (0, errno, _("cannot remove %s"), quote (src_nam= e)); - return false; - } } else { diff --git a/tests/cp/same-file.sh b/tests/cp/same-file.sh index f62a9a7..54d23a5 100755 --- a/tests/cp/same-file.sh +++ b/tests/cp/same-file.sh @@ -44,7 +44,8 @@ exec 3>&1 1> actual # FIXME: This should be bigger: like more than 8k contents=3DXYZ =20 -for args in 'foo symlink' 'symlink foo' 'foo foo' 'sl1 sl2' 'foo hardlin= k'; do +for args in 'foo symlink' 'symlink foo' 'foo foo' 'sl1 sl2' \ + 'foo hardlink' 'hlsl sl2'; do for options in '' -d -f -df --rem -b -bd -bf -bdf \ -l -dl -fl -dfl -bl -bdl -bfl -bdfl; do case $args$options in @@ -70,11 +71,11 @@ for args in 'foo symlink' 'symlink foo' 'foo foo' 'sl= 1 sl2' 'foo hardlink'; do # cont'd Instead, skip them only on systems for which link does # dereference a symlink. Detect and skip such tests here. case $hard_link_to_symlink_does_the_deref:$args:$options in - 'yes:sl1 sl2:-fl') + yes:*sl2:-fl) continue ;; - 'yes:sl1 sl2:-bl') + yes:*sl2:-bl) continue ;; - 'yes:sl1 sl2:-bfl') + yes:*sl2:-bfl) continue ;; esac =20 @@ -86,6 +87,7 @@ for args in 'foo symlink' 'symlink foo' 'foo foo' 'sl1 = sl2' 'foo hardlink'; do case "$args" in *hardlink*) ln foo hardlink ;; esac case "$args" in *sl1*) ln -s foo sl1;; esac case "$args" in *sl2*) ln -s foo sl2;; esac + case "$args" in *hlsl*) ln sl2 hlsl;; esac ( ( # echo 1>&2 cp $options $args @@ -211,6 +213,24 @@ cat <<\EOF | sed "$remove_these_sed" > expected 0 -bfl (foo hardlink) 0 -bdfl (foo hardlink) =20 +1 [cp: 'hlsl' and 'sl2' are the same file] (foo hlsl -> foo sl2 -> foo) +0 -d (foo hlsl -> foo sl2 -> foo) +1 -f [cp: 'hlsl' and 'sl2' are the same file] (foo hlsl -> foo sl2 -> fo= o) +0 -df (foo hlsl -> foo sl2 -> foo) +0 --rem (foo hlsl -> foo sl2) +0 -b (foo hlsl -> foo sl2 sl2.~1~ -> foo) +0 -bd (foo hlsl -> foo sl2 -> foo sl2.~1~ -> foo) +0 -bf (foo hlsl -> foo sl2 sl2.~1~ -> foo) +0 -bdf (foo hlsl -> foo sl2 -> foo sl2.~1~ -> foo) +1 -l [cp: cannot create hard link 'sl2' to 'hlsl'] (foo hlsl -> foo sl2 = -> foo) +0 -dl (foo hlsl -> foo sl2 -> foo) +0 -fl (foo hlsl -> foo sl2) +0 -dfl (foo hlsl -> foo sl2 -> foo) +0 -bl (foo hlsl -> foo sl2 sl2.~1~ -> foo) +0 -bdl (foo hlsl -> foo sl2 -> foo) +0 -bfl (foo hlsl -> foo sl2 sl2.~1~ -> foo) +0 -bdfl (foo hlsl -> foo sl2 -> foo) + EOF =20 exec 1>&3 3>&- diff --git a/tests/local.mk b/tests/local.mk index e01f4d8..22e8b86 100644 --- a/tests/local.mk +++ b/tests/local.mk @@ -605,7 +605,6 @@ all_tests =3D \ tests/mv/hard-3.sh \ tests/mv/hard-4.sh \ tests/mv/hard-link-1.sh \ - tests/mv/hard-verbose.sh \ tests/mv/i-1.pl \ tests/mv/i-2.sh \ tests/mv/i-3.sh \ diff --git a/tests/mv/force.sh b/tests/mv/force.sh index 05adabc..be97665 100755 --- a/tests/mv/force.sh +++ b/tests/mv/force.sh @@ -25,18 +25,19 @@ ff2=3Dmvforce2 echo force-contents > $ff || framework_failure_ ln $ff $ff2 || framework_failure_ =20 -# This mv command should exit nonzero. -mv $ff $ff > out 2>&1 && fail=3D1 +# mv should fail for the same name, or separate hardlinks as in +# both cases rename() will do nothing and return success. +# One could unlink(src) in the hardlink case, but that would +# introduce races with overlapping mv instances removing both hardlinks.= =20 -cat > exp < out 2>&1 && fail=3D1 =20 -compare exp out || fail=3D1 -test $(cat $ff) =3D force-contents || fail=3D1 + printf "mv: '$ff' and '$dest' are the same file\n" > exp + compare exp out || fail=3D1 =20 -# This should succeed, even though the source and destination -# device and inodes are the same. -mv $ff $ff2 || fail=3D1 + test $(cat $ff) =3D force-contents || fail=3D1 +done =20 Exit $fail diff --git a/tests/mv/hard-4.sh b/tests/mv/hard-4.sh index d518e3b..8b03c45 100755 --- a/tests/mv/hard-4.sh +++ b/tests/mv/hard-4.sh @@ -1,5 +1,5 @@ #!/bin/sh -# ensure that mv removes a in this case: touch a; ln a b; mv a b +# ensure that mv maintains a in this case: touch a; ln a b; mv a b =20 # Copyright (C) 2003-2014 Free Software Foundation, Inc. =20 @@ -21,15 +21,18 @@ print_ver_ mv touch a || framework_failure_ ln a b || framework_failure_ =20 +# Between coreutils-5.0 and coreutils-8.24, 'a' would be removed. +# Before coreutils-5.0.1 the issue would not have been diagnosed. +# We don't emulate the rename(a,b) with unlink(a) as that would +# introduce races with overlapping mv instances removing both links. +mv a b 2>err && fail=3D1 +printf "mv: 'a' and 'b' are the same file\n" > exp +compare exp err || fail=3D1 =20 -mv a b || fail=3D1 - -# In coreutils-5.0 and earlier, a would not be removed. -test -r a && fail=3D1 +test -r a || fail=3D1 test -r b || fail=3D1 =20 -# Make sure it works also with --backup. -ln b a +# Make sure it works with --backup. mv --backup=3Dsimple a b || fail=3D1 test -r a && fail=3D1 test -r b || fail=3D1 diff --git a/tests/mv/hard-verbose.sh b/tests/mv/hard-verbose.sh deleted file mode 100755 index 45491ab..0000000 --- a/tests/mv/hard-verbose.sh +++ /dev/null @@ -1,33 +0,0 @@ -#!/bin/sh -# ensure that mv's --verbose options works even in this unusual case - -# Copyright (C) 2006-2014 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 . - -. "${srcdir=3D.}/tests/init.sh"; path_prepend_ ./src -print_ver_ mv - -touch x || framework_failure_ -ln x y || framework_failure_ - - -mv --verbose x y > out || fail=3D1 -cat <<\EOF > exp || fail=3D1 -removed 'x' -EOF - -compare exp out || fail=3D1 - -Exit $fail diff --git a/tests/mv/i-4.sh b/tests/mv/i-4.sh index b366bc4..ad79389 100755 --- a/tests/mv/i-4.sh +++ b/tests/mv/i-4.sh @@ -23,6 +23,7 @@ for i in a b; do echo $i > $i || framework_failure_ done echo y > y || framework_failure_ +echo n > n || framework_failure_ =20 mv -i a b < y >/dev/null 2>&1 || fail=3D1 =20 @@ -32,18 +33,15 @@ case "$(cat b)" in *) fail=3D1 ;; esac =20 -# Ensure that mv -i a b works properly with 'n' and 'y' -# responses, even when a and b are hard links to the same file. -# This 'n' test would fail (no prompt) for coreutils-5.0.1 through 5.3.0= =2E -echo n > n +# Ensure that mv -i a b works properly with 'n' and 'y' responses, +# when a and b are hard links to the same file. rm -f a b echo a > a ln a b -mv -i a b < n >/dev/null 2>&1 || fail=3D1 +mv -i a b < y 2>err && fail=3D1 test -r a || fail=3D1 test -r b || fail=3D1 -mv -i a b < y >/dev/null 2>&1 || fail=3D1 -test -r a && fail=3D1 -test -r b || fail=3D1 +printf "mv: 'a' and 'b' are the same file\n" > exp +compare exp err || fail=3D1 =20 Exit $fail diff --git a/tests/mv/symlink-onto-hardlink-to-self.sh b/tests/mv/symlink= -onto-hardlink-to-self.sh index f3e8ff9..1a567df 100755 --- a/tests/mv/symlink-onto-hardlink-to-self.sh +++ b/tests/mv/symlink-onto-hardlink-to-self.sh @@ -1,6 +1,6 @@ #!/bin/sh -# Demonstrate that when moving a symlink onto a hardlink-to-that-symlink= , the -# source symlink is removed. Depending on your kernel (e.g., Linux, Sol= aris, +# Demonstrate that when moving a symlink onto a hardlink-to-that-symlink= , +# an error is presented. Depending on your kernel (e.g., Linux, Solaris= , # but not NetBSD), prior to coreutils-8.16, the mv would successfully pe= rform # a no-op. I.e., surprisingly, mv s1 s2 would succeed, yet fail to remo= ve s1. =20 @@ -26,27 +26,34 @@ print_ver_ mv touch f || framework_failure_ ln -s f s2 || framework_failure_ =20 -for opt in '' --backup; do +# Attempt to create a hard link to that symlink. +# On some systems, it's not possible: they create a hard link to the ref= erent. +ln s2 s1 || framework_failure_ =20 - # Attempt to create a hard link to that symlink. - # On some systems, it's not possible: they create a hard link to the r= eferent. - ln s2 s1 || framework_failure_ +# If s1 is not a symlink, skip this test. +test -h s1 \ + || skip_ your kernel or file system cannot create a hard link to a sym= link =20 - # If s1 is not a symlink, skip this test. - test -h s1 \ - || skip_ your kernel or file system cannot create a hard link to a s= ymlink +for opt in '' --backup; do =20 - mv $opt s1 s2 > out 2>&1 || fail=3D1 - compare /dev/null out || fail=3D1 + if test "$opt" =3D --backup; then + mv $opt s1 s2 > out 2>&1 || fail=3D1 + compare /dev/null out || fail=3D1 =20 - # Ensure that s1 is gone. - test -e s1 && fail=3D1 + # Ensure that s1 is gone. + test -e s1 && fail=3D1 =20 - if test "$opt" =3D --backup; then # With --backup, ensure that the backup file was created. ref=3D$(readlink s2~) || fail=3D1 test "$ref" =3D f || fail=3D1 else + echo "mv: 's1' and 's2' are the same file" > exp + mv $opt s1 s2 2>err && fail=3D1 + compare exp err || fail=3D1 + + # Ensure that s1 is still present. + test -e s1 || fail=3D1 + # Without --backup, ensure there is no backup file. test -e s2~ && fail=3D1 fi --=20 2.1.0 --------------000603040905010301080800-- From debbugs-submit-bounces@debbugs.gnu.org Fri Nov 21 03:29:52 2014 Received: (at 18499) by debbugs.gnu.org; 21 Nov 2014 08:29:52 +0000 Received: from localhost ([127.0.0.1]:40206 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1XrjbE-0001jK-Bt for submit@debbugs.gnu.org; Fri, 21 Nov 2014 03:29:52 -0500 Received: from mx1.redhat.com ([209.132.183.28]:39495) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1XrjbB-0001jB-Qq for 18499@debbugs.gnu.org; Fri, 21 Nov 2014 03:29:50 -0500 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id sAL8TfKx011265 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Fri, 21 Nov 2014 03:29:42 -0500 Received: from [10.10.54.109] (vpn-54-109.rdu2.redhat.com [10.10.54.109]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id sAL8TcBg021881 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-GCM-SHA256 bits=128 verify=NO); Fri, 21 Nov 2014 03:29:39 -0500 Message-ID: <1416558578.12565.3.camel@unused-4-145.brq.redhat.com> Subject: Re: bug#18499: Possible mv race for hardlinks (rhbz #1141368 ) From: Boris Ranto To: =?ISO-8859-1?Q?P=E1draig?= Brady Date: Fri, 21 Nov 2014 09:29:38 +0100 In-Reply-To: <546EB1EA.7030307@draigBrady.com> References: <1411037552.10808.19.camel@dhcp-24-196.brq.redhat.com> <5422EEDC.80706@draigBrady.com> <1415887133.5037.6.camel@unused-4-145.brq.redhat.com> <5464D115.3060008@draigBrady.com> <1415894773.5037.19.camel@unused-4-145.brq.redhat.com> <5466920F.6010606@draigBrady.com> <1416137884.5049.25.camel@dhcp-24-196.brq.redhat.com> <54689356.8070806@draigBrady.com> <5468D246.2010808@cs.ucla.edu> <54694136.7090002@draigBrady.com> <1416328166.7154.3.camel@redhat.com> <546B77F7.3010900@draigBrady.com> <1416338883.7154.5.camel@redhat.com> <546EB1EA.7030307@draigBrady.com> Organization: Red Hat Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 8bit X-Scanned-By: MIMEDefang 2.68 on 10.5.11.23 X-Spam-Score: -5.0 (-----) X-Debbugs-Envelope-To: 18499 Cc: Paul Eggert , 18499@debbugs.gnu.org, ovasik@redhat.com, Miklos Szeredi X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.15 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 (-----) On Fri, 2014-11-21 at 03:30 +0000, Pádraig Brady wrote: > On 18/11/14 19:28, Boris Ranto wrote: > > On Tue, 2014-11-18 at 16:46 +0000, Pádraig Brady wrote: > >> On 18/11/14 16:29, Boris Ranto wrote: > >>> On Mon, 2014-11-17 at 00:28 +0000, Pádraig Brady wrote: > >>>> On 16/11/14 16:35, Paul Eggert wrote: > >>>>> Pádraig Brady wrote: > >>>>>> If we change this, it's much more likely that people will start complaining > >>>>>> about their non overlapping mv instances failing. > >>>>> > >>>>> I'd far rather deal with those complaints than deal with complaints about 'mv' silently discarding files. Either the FreeBSD or the Solaris behavior would be a real improvement over what we're doing now. Neither behavior is as good as having support in the kernel for doing the right thing, but one step at a time. > >>>> > >>>> Fair enough. That's 3 votes for changing this. > >>>> I'll work on a patch to fail in this case. > >>>> > >>>> thanks, > >>>> Pádraig. > >>>> > >>> > >>> I've looked at the code and I was able to identify the part that deals > >>> with the symlinks. I'm attaching the patch that makes mv fail in this > >>> case. > >> > >> I'm not sure symlinks should be treated differently here. > >> I.E. it may be best to remove the whole unlink_src logic. > >> I'll look later. > >> > >> thanks, > >> Pádraig. > > > > You were right, the only other place that used the unlink_src logic was > > the case handling mv for symlinks that were hard links to the same file > > (this case was handled separetely from the normal files) -- i.e. the > > case where you do this: > > touch a; ln -s a b; ln b c; mv b c > > > > I'm attaching the revised patch that removes the whole unlink_src logic > > altogether. > > We want to leave the logic in place for cp and install though, > and I've adjusted your patch accordingly. I've also adjusted > the tests to pass and augmented the tests to cover one of > the cases missed in the previous patch. I'll push this tomorrow. > > thanks, > Pádraig. Just a note: cp already presented this behaviour before the patch, i.e. cp a b on hard links to the same file failed with cp: ‘a’ and ‘b’ are the same file On the other hand, install does not present it, it copies over b creating new inode for b. -Boris From debbugs-submit-bounces@debbugs.gnu.org Fri Nov 21 06:53:34 2014 Received: (at 18499-done) by debbugs.gnu.org; 21 Nov 2014 11:53:34 +0000 Received: from localhost ([127.0.0.1]:40408 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1XrmmM-00080N-BJ for submit@debbugs.gnu.org; Fri, 21 Nov 2014 06:53:34 -0500 Received: from mail2.vodafone.ie ([213.233.128.44]:17911) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1XrmmI-00080A-Q6 for 18499-done@debbugs.gnu.org; Fri, 21 Nov 2014 06:53:32 -0500 X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: ApIIAC0nb1RtTZiq/2dsb2JhbABbgw6CUYETCEjNBoMYAgICgQQWAQEBAQF9hAMBAQQMFw8BRhAJAg0BCgICBRYLAgIJAwIBAgFFBg0BBwEBiEEBnGCccoYLkFUBAQgBAQEBHoEtj1sHgneBVAEEoBSGd4RXhXGECYN7PYJ7AQEB Received: from unknown (HELO localhost.localdomain) ([109.77.152.170]) by mail2.vodafone.ie with ESMTP; 21 Nov 2014 11:53:28 +0000 Message-ID: <546F27B8.5080307@draigBrady.com> Date: Fri, 21 Nov 2014 11:53:28 +0000 From: =?UTF-8?B?UMOhZHJhaWcgQnJhZHk=?= User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0 MIME-Version: 1.0 To: Boris Ranto Subject: Re: bug#18499: Possible mv race for hardlinks (rhbz #1141368 ) References: <1411037552.10808.19.camel@dhcp-24-196.brq.redhat.com> <5422EEDC.80706@draigBrady.com> <1415887133.5037.6.camel@unused-4-145.brq.redhat.com> <5464D115.3060008@draigBrady.com> <1415894773.5037.19.camel@unused-4-145.brq.redhat.com> <5466920F.6010606@draigBrady.com> <1416137884.5049.25.camel@dhcp-24-196.brq.redhat.com> <54689356.8070806@draigBrady.com> <5468D246.2010808@cs.ucla.edu> <54694136.7090002@draigBrady.com> <1416328166.7154.3.camel@redhat.com> <546B77F7.3010900@draigBrady.com> <1416338883.7154.5.camel@redhat.com> <546EB1EA.7030307@draigBrady.com> <1416558578.12565.3.camel@unused-4-145.brq.redhat.com> In-Reply-To: <1416558578.12565.3.camel@unused-4-145.brq.redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Spam-Score: 0.0 (/) X-Debbugs-Envelope-To: 18499-done Cc: Paul Eggert , 18499-done@debbugs.gnu.org, ovasik@redhat.com, Miklos Szeredi X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.15 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 21/11/14 08:29, Boris Ranto wrote: > On Fri, 2014-11-21 at 03:30 +0000, Pádraig Brady wrote: >> We want to leave the logic in place for cp and install though, >> and I've adjusted your patch accordingly. I've also adjusted >> the tests to pass and augmented the tests to cover one of >> the cases missed in the previous patch. I'll push this tomorrow. >> >> thanks, >> Pádraig. > > Just a note: cp already presented this behaviour before the patch, i.e. > > cp a b > > on hard links to the same file failed with > > cp: ‘a’ and ‘b’ are the same file > > On the other hand, install does not present it, it copies over b > creating new inode for b. Yep, but there were other cases with `cp -a` with hardlinks to symlinks, and cp --remove-destination a b. I've pushed that now. thanks! Pádraig. From debbugs-submit-bounces@debbugs.gnu.org Fri Nov 21 09:39:44 2014 Received: (at 18499) by debbugs.gnu.org; 21 Nov 2014 14:39:44 +0000 Received: from localhost ([127.0.0.1]:40489 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1XrpNA-0004qG-5z for submit@debbugs.gnu.org; Fri, 21 Nov 2014 09:39:44 -0500 Received: from mail2.vodafone.ie ([213.233.128.44]:28104) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1XrpN5-0004q5-K3 for 18499@debbugs.gnu.org; Fri, 21 Nov 2014 09:39:40 -0500 X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: AgURAFJOb1RtTZiq/2dsb2JhbABcgw5VWoI1CEjIX4dFAQEBAQKBAhYBAQEBAX2EAwEBBAwXDwFGEAkCDQEKAgIFFgsCAgkDAgECAUUGDQEHAQGIQQEImjyccoYLkGIBCwEfgS2PWweCd4FUBZc1iF89hjqEV4VxhAmDez2CewEBAQ Received: from unknown (HELO localhost.localdomain) ([109.77.152.170]) by mail2.vodafone.ie with ESMTP; 21 Nov 2014 14:39:35 +0000 Message-ID: <546F4EA7.2000100@draigBrady.com> Date: Fri, 21 Nov 2014 14:39:35 +0000 From: =?UTF-8?B?UMOhZHJhaWcgQnJhZHk=?= User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0 MIME-Version: 1.0 To: Boris Ranto Subject: Re: bug#18499: Possible mv race for hardlinks (rhbz #1141368 ) References: <1411037552.10808.19.camel@dhcp-24-196.brq.redhat.com> <5422EEDC.80706@draigBrady.com> <1415887133.5037.6.camel@unused-4-145.brq.redhat.com> <5464D115.3060008@draigBrady.com> <1415894773.5037.19.camel@unused-4-145.brq.redhat.com> <5466920F.6010606@draigBrady.com> <1416137884.5049.25.camel@dhcp-24-196.brq.redhat.com> <54689356.8070806@draigBrady.com> <5468D246.2010808@cs.ucla.edu> <54694136.7090002@draigBrady.com> <1416328166.7154.3.camel@redhat.com> <546B77F7.3010900@draigBrady.com> <1416338883.7154.5.camel@redhat.com> <546EB1EA.7030307@draigBrady.com> <1416558578.12565.3.camel@unused-4-145.brq.redhat.com> <546F27B8.5080307@draigBrady.com> In-Reply-To: <546F27B8.5080307@draigBrady.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Spam-Score: 0.0 (/) X-Debbugs-Envelope-To: 18499 Cc: Paul Eggert , 18499@debbugs.gnu.org, ovasik@redhat.com, Miklos Szeredi X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.15 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 21/11/14 11:53, Pádraig Brady wrote: > On 21/11/14 08:29, Boris Ranto wrote: >> On Fri, 2014-11-21 at 03:30 +0000, Pádraig Brady wrote: >>> We want to leave the logic in place for cp and install though, >>> and I've adjusted your patch accordingly. I've also adjusted >>> the tests to pass and augmented the tests to cover one of >>> the cases missed in the previous patch. I'll push this tomorrow. >>> >>> thanks, >>> Pádraig. >> >> Just a note: cp already presented this behaviour before the patch, i.e. >> >> cp a b >> >> on hard links to the same file failed with >> >> cp: ‘a’ and ‘b’ are the same file >> >> On the other hand, install does not present it, it copies over b >> creating new inode for b. > > Yep, but there were other cases with `cp -a` with hardlinks > to symlinks, and cp --remove-destination a b. > > I've pushed that now. For reference I've made the kernel renameat() suggestion at: http://marc.info/?l=linux-api&m=141658005205610&w=2 Pádraig. From debbugs-submit-bounces@debbugs.gnu.org Fri Nov 21 22:49:41 2014 Received: (at 18499) by debbugs.gnu.org; 22 Nov 2014 03:49:41 +0000 Received: from localhost ([127.0.0.1]:41375 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1Xs1hc-0003vs-90 for submit@debbugs.gnu.org; Fri, 21 Nov 2014 22:49:40 -0500 Received: from mail2.vodafone.ie ([213.233.128.44]:19053) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1Xs1hY-0003vi-VV for 18499@debbugs.gnu.org; Fri, 21 Nov 2014 22:49:38 -0500 X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: Ag0JAM8GcFRtTyO7/2dsb2JhbABcgw5VWYMGvnaKHIZsAQICgQcWAQEBAQF9hAIBAQEEI1YQCw0BAwMBAgoWCwICCQMCAQIBPQgGDQEFAgEBBRGIKwEItkiGC5B+AQEBAQEBAQMBAQEBAQEBAQEBAReQehEHCYJwgVUFhk2LbIJQgVVriGU/hkCEYoI9S4JthAmDfUgwgksBAQE Received: from unknown (HELO localhost.localdomain) ([109.79.35.187]) by mail2.vodafone.ie with ESMTP; 22 Nov 2014 03:49:33 +0000 Message-ID: <547007CD.1060707@draigBrady.com> Date: Sat, 22 Nov 2014 03:49:33 +0000 From: =?UTF-8?B?UMOhZHJhaWcgQnJhZHk=?= User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0 MIME-Version: 1.0 To: Boris Ranto Subject: Re: bug#18499: Possible mv race for hardlinks (rhbz #1141368 ) References: <1411037552.10808.19.camel@dhcp-24-196.brq.redhat.com> <5422EEDC.80706@draigBrady.com> <1415887133.5037.6.camel@unused-4-145.brq.redhat.com> <5464D115.3060008@draigBrady.com> <1415894773.5037.19.camel@unused-4-145.brq.redhat.com> <5466920F.6010606@draigBrady.com> <1416137884.5049.25.camel@dhcp-24-196.brq.redhat.com> <54689356.8070806@draigBrady.com> <5468D246.2010808@cs.ucla.edu> <54694136.7090002@draigBrady.com> <1416328166.7154.3.camel@redhat.com> <546B77F7.3010900@draigBrady.com> <1416338883.7154.5.camel@redhat.com> <546EB1EA.7030307@draigBrady.com> In-Reply-To: <546EB1EA.7030307@draigBrady.com> Content-Type: multipart/mixed; boundary="------------010807010302050609080008" X-Spam-Score: 0.0 (/) X-Debbugs-Envelope-To: 18499 Cc: Paul Eggert , 18499@debbugs.gnu.org, ovasik@redhat.com, Miklos Szeredi X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.15 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. --------------010807010302050609080008 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit I just realised that this change avoids a more problematic issue. The loss of files on case insensitive file systems supporting hardlinks. Consider hfs... $ truncate -s1G hfs.img $ mkfs.hfsplus hfs.img $ mkdir hfs $ sudo mount hfs.img hfs $ sudo su # cd hfs # touch foo # ln foo blah # mv foo Foo # foo is removed! # ls -l -rw-r--r--. 1 root root 0 Nov 22 02:42 blah In the attached patch I've added a test case, removed an old test case that pondered the issue in 5.0.91?, updated NEWS and mentioned the issue in copy.c cheers, Pádraig. --------------010807010302050609080008 Content-Type: text/x-patch; name="mv-hardlink-hfs.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="mv-hardlink-hfs.patch" =46rom a78819af2873ea7b104366bf72af7c3e5b30782b Mon Sep 17 00:00:00 2001 From: =3D?UTF-8?q?P=3DC3=3DA1draig=3D20Brady?=3D Date: Sat, 22 Nov 2014 03:41:55 +0000 Subject: [PATCH] tests: add a case verifying mv on case insensitive file systems * NEWS: Update the recent entry to also mention the avoidance of incorrectly unlinking a multi-hardlinked "source" file when presented with source and dest that only differ in case. * src/copy.c (same_file_ok): Mention the case issue with same_name(). * tests/mv/hardlink-case.sh: Test the issue on HFS+. * tests/local.mk: Reference the new test case. * tests/mv/vfat: Remove an old related but unused test case. --- NEWS | 9 ++++--- src/copy.c | 1 + tests/local.mk | 1 + tests/mv/hardlink-case.sh | 37 +++++++++++++++++++++++++++ tests/mv/vfat | 64 -----------------------------------------= ------ 5 files changed, 44 insertions(+), 68 deletions(-) create mode 100755 tests/mv/hardlink-case.sh delete mode 100644 tests/mv/vfat diff --git a/NEWS b/NEWS index 6a84c48..5d3bc58 100644 --- a/NEWS +++ b/NEWS @@ -32,10 +32,11 @@ GNU coreutils NEWS = -*- outline -*- =20 ** Changes in behavior =20 - mv no longer supports moving a file to a hardlink, as currently that - is only supported through unlinking the source file. That's racy - in the presence of multiple mv instances, which could result in both - hardlinks being deleted. This feature was added in coreutils-5.0.1. + mv no longer supports moving a file to a hardlink, instead issuing an = error. + The implementation was susceptible to races in the presence of multipl= e mv + instances, which could result in both hardlinks being deleted. Also o= n case + insensitive file systems like HFS, mv would just remove a hardlinked '= file' + if called like `mv file File`. The feature was added in coreutils-5.0= =2E1. =20 ** Improvements =20 diff --git a/src/copy.c b/src/copy.c index 01cee32..f316f05 100644 --- a/src/copy.c +++ b/src/copy.c @@ -1526,6 +1526,7 @@ same_file_ok (char const *src_name, struct stat con= st *src_sb, return true; } =20 + /* FIXME: What about case insensitive file systems ? */ return ! same_name (src_name, dst_name); } =20 diff --git a/tests/local.mk b/tests/local.mk index 22e8b86..653c984 100644 --- a/tests/local.mk +++ b/tests/local.mk @@ -127,6 +127,7 @@ all_root_tests =3D \ tests/misc/truncate-owned-by-other.sh \ tests/mkdir/writable-under-readonly.sh \ tests/mkdir/smack-root.sh \ + tests/mv/hardlink-case.sh \ tests/mv/sticky-to-xpart.sh \ tests/rm/fail-2eperm.sh \ tests/rm/no-give-up.sh \ diff --git a/tests/mv/hardlink-case.sh b/tests/mv/hardlink-case.sh new file mode 100755 index 0000000..5ab8c95 --- /dev/null +++ b/tests/mv/hardlink-case.sh @@ -0,0 +1,37 @@ +#!/bin/sh +# Ensure multi-hardlinked files are not lost on case insensitive file sy= stems + +# Copyright (C) 2014 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 . + +. "${srcdir=3D.}/tests/init.sh"; path_prepend_ ./src +print_ver_ mv +require_root_ + +cwd=3D$(pwd) +cleanup_() { cd /; umount "$cwd/mnt"; } + +truncate -s100M hfs.img || framework_failure_ +mkfs -t hfsplus hfs.img || skip_ 'failed to create hfs file system' +mkdir mnt || framework_failure_ +mount hfs.img mnt || skip_ 'failed to mount hfs file system' + +cd mnt +touch foo +ln foo whatever +mv foo Foo && fail=3D1 +test -r foo || fail=3D1 + +Exit $fail diff --git a/tests/mv/vfat b/tests/mv/vfat deleted file mode 100644 index cd2f8b4..0000000 --- a/tests/mv/vfat +++ /dev/null @@ -1,64 +0,0 @@ -#!/bin/sh -# This is just for the record. -# This test is not run. - -# Copyright (C) 2003-2014 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 . - -exit 0 - -cat <<\EOF - -Prior to 5.0.91, ... - -The problem: - On a VFAT file system with coreutils-5.0.90, 'mv FOO foo' removes the - sole copy of the file named by both the source and destination argumen= ts. - -Demonstrate the problem, as root: - -cd /tmp \ - && dd if=3D/dev/zero of=3D1 bs=3D8192 count=3D50 \ - && mkdir mnt && mkfs -t vfat 1 \ - && mount -oloop 1 mnt && cd mnt \ - && printf something important > foo \ - && mv foo FOO -test -f FOO && echo PASS-1 || echo FAIL-1 -ln foo bar -mv foo FOO -test -f FOO && echo PASS-2 || echo FAIL-2 - -And in case you actually do the above, you can do this to clean up: - - cd /tmp && umount /tmp/mnt && rm -r 1 mnt - -Hey! Can't create hard links on vfat. -The above 'ln' evokes an 'operation not permitted' failure. - -This demonstrates the same thing with file system type 'umsdos' -No hard links: - -cd /tmp \ - && dd if=3D/dev/zero of=3D1 bs=3D8192 count=3D50 \ - && mkdir mnt && mkfs -t msdos 1 \ - && mount -t umsdos -oloop 1 mnt && cd mnt \ - && printf something important > foo \ - && mv foo FOO -test -f FOO && echo PASS-1 || echo FAIL-1 -ln foo bar -mv foo FOO -test -f FOO && echo PASS-2 || echo FAIL-2 - -EOF --=20 2.1.0 --------------010807010302050609080008-- From debbugs-submit-bounces@debbugs.gnu.org Sat Nov 29 17:59:58 2014 Received: (at 18499) by debbugs.gnu.org; 29 Nov 2014 22:59:58 +0000 Received: from localhost ([127.0.0.1]:49448 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1Xuqzd-0008P7-M8 for submit@debbugs.gnu.org; Sat, 29 Nov 2014 17:59:58 -0500 Received: from mail2.vodafone.ie ([213.233.128.44]:57936) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1Xuqzb-0008Ox-0k for 18499@debbugs.gnu.org; Sat, 29 Nov 2014 17:59:56 -0500 X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: AtIIAAhPelRtTnMX/2dsb2JhbABbgwZRWIMFxBSGSwECAoEKFgEBAQEBfYQCAQEBBCMERwsQCw0BAwMBAgEJFgsCAgkDAgECAT0IBg0BBQIBAYhAAQi8EoV+jzMBAQEBAQEEAQEBAQEBAQEBGZBqEQcJgmyBUwWGR4xWgU5niCM8hg6EKYUghAKDez8wAQGCRQEBAQ Received: from unknown (HELO localhost.localdomain) ([109.78.115.23]) by mail2.vodafone.ie with ESMTP; 29 Nov 2014 22:59:53 +0000 Message-ID: <547A4FE8.1020804@draigBrady.com> Date: Sat, 29 Nov 2014 22:59:52 +0000 From: =?UTF-8?B?UMOhZHJhaWcgQnJhZHk=?= User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0 MIME-Version: 1.0 To: Boris Ranto Subject: Re: bug#18499: Possible mv race for hardlinks (rhbz #1141368 ) References: <1411037552.10808.19.camel@dhcp-24-196.brq.redhat.com> <5422EEDC.80706@draigBrady.com> <1415887133.5037.6.camel@unused-4-145.brq.redhat.com> <5464D115.3060008@draigBrady.com> <1415894773.5037.19.camel@unused-4-145.brq.redhat.com> <5466920F.6010606@draigBrady.com> <1416137884.5049.25.camel@dhcp-24-196.brq.redhat.com> <54689356.8070806@draigBrady.com> <5468D246.2010808@cs.ucla.edu> <54694136.7090002@draigBrady.com> <1416328166.7154.3.camel@redhat.com> <546B77F7.3010900@draigBrady.com> <1416338883.7154.5.camel@redhat.com> <546EB1EA.7030307@draigBrady.com> In-Reply-To: <546EB1EA.7030307@draigBrady.com> Content-Type: multipart/mixed; boundary="------------030808070505050901040200" X-Spam-Score: 0.0 (/) X-Debbugs-Envelope-To: 18499 Cc: 18499@debbugs.gnu.org, ovasik@redhat.com X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.15 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. --------------030808070505050901040200 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit On 21/11/14 03:30, Pádraig Brady wrote: > We want to leave the logic in place for cp and install though, > and I've adjusted your patch accordingly. I've also adjusted > the tests to pass and augmented the tests to cover one of > the cases missed in the previous patch. I'll push this tomorrow. There was a small window where attachments were being stripped from gnu mailing list emails, that the above hit unfortunately. So for the record, the patch referenced above is: http://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commit;h=222d7ac0 There was a problem with that though, identified by the darwin job added to the hydra continuous integration system today: http://hydra.nixos.org/job/gnu/coreutils-master/build.x86_64-darwin I pushed the attached patch to address that. thanks, Pádraig. --------------030808070505050901040200 Content-Type: text/x-patch; name="darwin-same-file-test.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="darwin-same-file-test.patch" =46rom 6f16c63963b0624cbcbf285fb936b79276c047de Mon Sep 17 00:00:00 2001 From: =3D?UTF-8?q?P=3DC3=3DA1draig=3D20Brady?=3D Date: Sat, 29 Nov 2014 22:42:04 +0000 Subject: [PATCH] tests: avoid hardlink to symlink tests where not support= ed These checks weren't correctly avoided in commit v8.23-66-g222d7ac * tests/cp/same-file.sh: Avoid all hardlink to symlink tests on platforms where that's not supported. Identified by http://hydra.nixos.org/build/17636446 --- tests/cp/same-file.sh | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tests/cp/same-file.sh b/tests/cp/same-file.sh index 54d23a5..242c54b 100755 --- a/tests/cp/same-file.sh +++ b/tests/cp/same-file.sh @@ -36,7 +36,7 @@ ln dangling-slink hard-link > /dev/null 2>&1 \ rm -f no-such dangling-slink hard-link =20 test $hard_link_to_symlink_does_the_deref =3D yes \ - && remove_these_sed=3D'/^0 -[bf]*l .*sl1 ->/d' \ + && remove_these_sed=3D'/^0 -[bf]*l .*sl1 ->/d; /hlsl/d' \ || remove_these_sed=3D'/^ELIDE NO TEST OUTPUT/d' =20 exec 3>&1 1> actual @@ -71,11 +71,13 @@ for args in 'foo symlink' 'symlink foo' 'foo foo' 'sl= 1 sl2' \ # cont'd Instead, skip them only on systems for which link does # dereference a symlink. Detect and skip such tests here. case $hard_link_to_symlink_does_the_deref:$args:$options in - yes:*sl2:-fl) + 'yes:sl1 sl2:-fl') continue ;; - yes:*sl2:-bl) + 'yes:sl1 sl2:-bl') continue ;; - yes:*sl2:-bfl) + 'yes:sl1 sl2:-bfl') + continue ;; + yes:hlsl*) continue ;; esac =20 --=20 2.1.0 --------------030808070505050901040200-- From unknown Mon Jun 23 07:51:25 2025 Received: (at fakecontrol) by fakecontrolmessage; To: internal_control@debbugs.gnu.org From: Debbugs Internal Request Subject: Internal Control Message-Id: bug archived. Date: Sun, 28 Dec 2014 12:24:04 +0000 User-Agent: Fakemail v42.6.9 # This is a fake control message. # # The action: # bug archived. thanks # This fakemail brought to you by your local debbugs # administrator