GNU bug report logs - #29961
[PATCH] mv: document the missing atomicity of 'mv -n'

Previous Next

Package: coreutils;

Reported by: Kamil Dudka <kdudka <at> redhat.com>

Date: Wed, 3 Jan 2018 15:03:02 UTC

Severity: normal

Tags: patch

Done: Paul Eggert <eggert <at> cs.ucla.edu>

Bug is archived. No further changes may be made.

Full log


View this message in rfc822 format

From: Kamil Dudka <kdudka <at> redhat.com>
To: Pádraig Brady <P <at> draigbrady.com>
Cc: 29961 <at> debbugs.gnu.org, Paul Eggert <eggert <at> cs.ucla.edu>
Subject: bug#29961: [PATCH] mv -n: do not overwrite the destination
Date: Fri, 05 Jan 2018 17:19:47 +0100
On Friday, January 5, 2018 4:29:55 PM CET Pádraig Brady wrote:
> Thanks to both of you.
> The approaches can be summarized as:
> 
> Orig:
> ---------------------------------
> stat() => ENOENT
> <file created externally>
> rename()  may clobber file
> 
> Kamil's:
> ---------------------------------
> stat() => ENOENT
> <file created externally>
> renameat()  doesn't clobber file if -n
> exit early if -n && errno==EEXIST
> 
> Paul's:
> ---------------------------------
> renameat2() => EEXIST
>  -n => exit early
> if (renameat2_failed)
>  unless EEXIST && -n
>   stat()
> if (renameat2_failed)
>  rename()
> 
> I think both patches ensure rename() doesn't clobber when -n is specified.

Thanks for the summary!

> Paul's is more encompassing for the non -n case.
> For example if a directory was _created_ externally
> after the stat() in Kamil's logic above,
> it could be erroneously clobbered.

Do you mean without -n?

I am getting the following error message in that case:

    mv: cannot move 'a' to 'b': Is a directory

... which is consistent with the original behavior.

> Paul's also avoids a stat() in the common case
> where the initial renameat2() succeeds.

At the cost of _not_ avoiding the renameat2() call in the most common case.
I think both the solutions are equivalent performance-wise.

> Both versions are still susceptible to erroneous clobbering
> if the destination file was externally _replaced_
> by a directory for example, after the stat().
> Though that is less likely.
> 
> Paul's fix looks good to apply here,
> since it's more encompassing.

No problem with that.  Anyway, I will go with my conservative (or even the 
documentation-only) patch for RHEL-7, which was the trigger of this effort, 
because Paul's patch comes with changes of behavior observable beyond the 
reported scenario.

Kamil

> cheers,
> Pádraig




This bug report was last modified 7 years and 139 days ago.

Previous Next


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