bug#11100: Racy code in copy.c

[Pádraig Brady]

On 03/27/2012 01:58 PM, Philipp Thomas wrote:
> I'd like to pass on observations from my collegue Neil Brown:
> 
> in src/copy.c, copy_reg() is passed "bool *new_dst".
> 
> This is 'false' if the file already exists, in which case it attempts to
> open the file with O_WRONLY | O_TRUNC | O_BINARY.
> If it is 'true', only then does it use O_CREAT (and others).
> 
> Somewhere up the call chain - I'm not sure where - new_dst is set if 'stat'
> on the file succeeds.  The above mentioned code assumes that the file still
> exists.  This is racy - particularly for NFS where deletions from other
> clients can take a while to appear.

True. That would result in:
  "cannot create regular file ...": ENOENT
or if -f was specified a more confusing:
  "cannot remove ...": ENOENT
You could patch to avoid that edge case,
but handling dangling symlinks would complicate things.
It's borderline whether this is warranted.

Note the opposite case where a file is
created between the stat() and the creat()
would result in:
  "cannot create regular file ...": EEXIST
even if -f was specified.
Note on NFS < 3 O_EXCL is not supported
so this condition won't be flagged at all.
Note also handling of this case would
need to honor the --no-clobber option(s).
Again awkward for an unusual edge case.

I suppose some comments would be appropriate at least.

cheers,
Pádraig.