bug#18499: Possible mv race for hardlinks (rhbz #1141368 )

[Boris Ranto]

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