GNU bug report logs - #18499
Possible mv race for hardlinks (rhbz #1141368 )

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 18499 in the body.
You can then email your comments to 18499 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Ondrej Vasik <ovasik <at> redhat.com>
To: CoreutilsBugs <bug-coreutils <at> gnu.org>
Subject: Possible mv race for hardlinks (rhbz #1141368 )
Date: Thu, 18 Sep 2014 12:52:32 +0200

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

Message #8 received at 18499 <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: ovasik <at> redhat.com, Miklos Szeredi <miklos <at> szeredi.hu>
Cc: 18499 <at> debbugs.gnu.org
Subject: Re: bug#18499: Possible mv race for hardlinks (rhbz #1141368 )
Date: Wed, 24 Sep 2014 17:18:36 +0100

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.

Message #11 received at 18499 <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Boris Ranto <branto <at> redhat.com>
Cc: 18499 <at> debbugs.gnu.org, ovasik <at> redhat.com,
 Miklos Szeredi <miklos <at> szeredi.hu>
Subject: Re: bug#18499: Possible mv race for hardlinks (rhbz #1141368 )
Date: Thu, 13 Nov 2014 15:41:09 +0000

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.

Message #14 received at 18499 <at> debbugs.gnu.org (full text, mbox):

From: Boris Ranto <branto <at> redhat.com>
To: Pádraig Brady <P <at> draigBrady.com>
Cc: 18499 <at> debbugs.gnu.org, ovasik <at> redhat.com,
 Miklos Szeredi <miklos <at> szeredi.hu>
Subject: Re: bug#18499: Possible mv race for hardlinks (rhbz #1141368 )
Date: Thu, 13 Nov 2014 14:58:53 +0100

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

Message #17 received at 18499 <at> debbugs.gnu.org (full text, mbox):

From: Boris Ranto <branto <at> redhat.com>
To: Pádraig Brady <P <at> draigbrady.com>
Cc: 18499 <at> debbugs.gnu.org, ovasik <at> redhat.com,
 Miklos Szeredi <miklos <at> szeredi.hu>
Subject: Re: bug#18499: Possible mv race for hardlinks (rhbz #1141368 )
Date: Thu, 13 Nov 2014 17:06:13 +0100

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.

Message #20 received at 18499 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Boris Ranto <branto <at> redhat.com>, Pádraig Brady
 <P <at> draigbrady.com>
Cc: 18499 <at> debbugs.gnu.org, ovasik <at> redhat.com,
 Miklos Szeredi <miklos <at> szeredi.hu>
Subject: Re: bug#18499: Possible mv race for hardlinks (rhbz #1141368 )
Date: Thu, 13 Nov 2014 12:51:55 -0800

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.

Message #23 received at 18499 <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Boris Ranto <branto <at> redhat.com>
Cc: 18499 <at> debbugs.gnu.org, ovasik <at> redhat.com,
 Miklos Szeredi <miklos <at> szeredi.hu>
Subject: Re: bug#18499: Possible mv race for hardlinks (rhbz #1141368 )
Date: Fri, 14 Nov 2014 23:36:47 +0000

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.

Message #26 received at 18499 <at> debbugs.gnu.org (full text, mbox):

From: Ondrej Vasik <ovasik <at> redhat.com>
To: Pádraig Brady <P <at> draigBrady.com>
Cc: 18499 <at> debbugs.gnu.org, Boris Ranto <branto <at> redhat.com>,
 Miklos Szeredi <miklos <at> szeredi.hu>
Subject: Re: bug#18499: Possible mv race for hardlinks (rhbz #1141368 )
Date: Sun, 16 Nov 2014 12:38:04 +0100

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

Message #29 received at 18499 <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: ovasik <at> redhat.com
Cc: 18499 <at> debbugs.gnu.org, Boris Ranto <branto <at> redhat.com>,
 Miklos Szeredi <miklos <at> szeredi.hu>
Subject: Re: bug#18499: Possible mv race for hardlinks (rhbz #1141368 )
Date: Sun, 16 Nov 2014 12:06:46 +0000

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.

Message #32 received at 18499 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Pádraig Brady <P <at> draigBrady.com>, 
 ovasik <at> redhat.com
Cc: 18499 <at> debbugs.gnu.org, Boris Ranto <branto <at> redhat.com>,
 Miklos Szeredi <miklos <at> szeredi.hu>
Subject: Re: bug#18499: Possible mv race for hardlinks (rhbz #1141368 )
Date: Sun, 16 Nov 2014 08:35:18 -0800

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.

Message #35 received at 18499 <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Paul Eggert <eggert <at> cs.ucla.edu>, ovasik <at> redhat.com
Cc: 18499 <at> debbugs.gnu.org, Boris Ranto <branto <at> redhat.com>,
 Miklos Szeredi <miklos <at> szeredi.hu>
Subject: Re: bug#18499: Possible mv race for hardlinks (rhbz #1141368 )
Date: Mon, 17 Nov 2014 00:28:38 +0000

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.

Message #38 received at 18499 <at> debbugs.gnu.org (full text, mbox):

From: Boris Ranto <branto <at> redhat.com>
To: Pádraig Brady <P <at> draigbrady.com>
Cc: Paul Eggert <eggert <at> cs.ucla.edu>, 18499 <at> debbugs.gnu.org, ovasik <at> redhat.com,
 Miklos Szeredi <miklos <at> szeredi.hu>
Subject: Re: bug#18499: Possible mv race for hardlinks (rhbz #1141368 )
Date: Tue, 18 Nov 2014 17:29:26 +0100

[Message part 1 (text/plain, inline)]

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

[0001-copy-treat-hard-links-to-the-same-file-as-same-file.patch (text/x-patch, attachment)]

Message #41 received at 18499 <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Boris Ranto <branto <at> redhat.com>
Cc: Paul Eggert <eggert <at> cs.ucla.edu>, 18499 <at> debbugs.gnu.org, ovasik <at> redhat.com,
 Miklos Szeredi <miklos <at> szeredi.hu>
Subject: Re: bug#18499: Possible mv race for hardlinks (rhbz #1141368 )
Date: Tue, 18 Nov 2014 16:46:47 +0000

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.

Message #44 received at 18499 <at> debbugs.gnu.org (full text, mbox):

From: Boris Ranto <branto <at> redhat.com>
To: Pádraig Brady <P <at> draigbrady.com>
Cc: Paul Eggert <eggert <at> cs.ucla.edu>, 18499 <at> debbugs.gnu.org, ovasik <at> redhat.com,
 Miklos Szeredi <miklos <at> szeredi.hu>
Subject: Re: bug#18499: Possible mv race for hardlinks (rhbz #1141368 )
Date: Tue, 18 Nov 2014 20:28:03 +0100

[Message part 1 (text/plain, inline)]

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

[0001-copy-treat-hard-links-to-the-same-file-as-same-file.patch (text/x-patch, attachment)]

Message #47 received at 18499 <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Boris Ranto <branto <at> redhat.com>
Cc: Paul Eggert <eggert <at> cs.ucla.edu>, 18499 <at> debbugs.gnu.org, ovasik <at> redhat.com,
 Miklos Szeredi <miklos <at> szeredi.hu>
Subject: Re: bug#18499: Possible mv race for hardlinks (rhbz #1141368 )
Date: Fri, 21 Nov 2014 03:30:50 +0000

[Message part 1 (text/plain, inline)]

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.

[mv-hardlink-race.patch (text/x-patch, attachment)]

Message #50 received at 18499 <at> debbugs.gnu.org (full text, mbox):

From: Boris Ranto <branto <at> redhat.com>
To: Pádraig Brady <P <at> draigbrady.com>
Cc: Paul Eggert <eggert <at> cs.ucla.edu>, 18499 <at> debbugs.gnu.org, ovasik <at> redhat.com,
 Miklos Szeredi <miklos <at> szeredi.hu>
Subject: Re: bug#18499: Possible mv race for hardlinks (rhbz #1141368 )
Date: Fri, 21 Nov 2014 09:29:38 +0100

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

Message #55 received at 18499-done <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Boris Ranto <branto <at> redhat.com>
Cc: Paul Eggert <eggert <at> cs.ucla.edu>, 18499-done <at> debbugs.gnu.org,
 ovasik <at> redhat.com, Miklos Szeredi <miklos <at> szeredi.hu>
Subject: Re: bug#18499: Possible mv race for hardlinks (rhbz #1141368 )
Date: Fri, 21 Nov 2014 11:53:28 +0000

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.

Message #58 received at 18499 <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Boris Ranto <branto <at> redhat.com>
Cc: Paul Eggert <eggert <at> cs.ucla.edu>, 18499 <at> debbugs.gnu.org, ovasik <at> redhat.com,
 Miklos Szeredi <miklos <at> szeredi.hu>
Subject: Re: bug#18499: Possible mv race for hardlinks (rhbz #1141368 )
Date: Fri, 21 Nov 2014 14:39:35 +0000

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.

Message #61 received at 18499 <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Boris Ranto <branto <at> redhat.com>
Cc: Paul Eggert <eggert <at> cs.ucla.edu>, 18499 <at> debbugs.gnu.org, ovasik <at> redhat.com,
 Miklos Szeredi <miklos <at> szeredi.hu>
Subject: Re: bug#18499: Possible mv race for hardlinks (rhbz #1141368 )
Date: Sat, 22 Nov 2014 03:49:33 +0000

[Message part 1 (text/plain, inline)]

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.

[mv-hardlink-hfs.patch (text/x-patch, attachment)]

Message #64 received at 18499 <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Boris Ranto <branto <at> redhat.com>
Cc: 18499 <at> debbugs.gnu.org, ovasik <at> redhat.com
Subject: Re: bug#18499: Possible mv race for hardlinks (rhbz #1141368 )
Date: Sat, 29 Nov 2014 22:59:52 +0000

[Message part 1 (text/plain, inline)]

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.

[darwin-same-file-test.patch (text/x-patch, attachment)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.