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

Previous Next

Full log

View this message in rfc822 format

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Kamil Dudka <kdudka <at> redhat.com>
Cc: 29961 <at> debbugs.gnu.org, Pádraig Brady <P <at> draigbrady.com>
Subject: bug#29961: [PATCH] mv -n: do not overwrite the destination
Date: Thu, 4 Jan 2018 17:00:52 -0800

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

On 01/04/2018 03:01 AM, Kamil Dudka wrote:
> On Thursday, January 4, 2018 10:48:56 AM CET Paul Eggert wrote:
>> Kamil Dudka wrote:
>>> -      if (rename (src_name, dst_name) == 0)
>>> +      int flags = 0;
>>> +      if (x->interactive == I_ALWAYS_NO)
>>> +        /* do not replace DST_NAME if it was created since our last check
>>> */ +        flags = RENAME_NOREPLACE;
>> By then it's too late, as multiple decisions have been made on the basis of
>> stat/lstat calls that are still subject to races.
> Do you mean in the case of mv -n?  Which decisions exactly?

Mostly mv -n, but I suspect problems also for mv without -n.  It's all 
the decisions that depend on the result of lstat of dst_name, before 
abandon_move decides whether to skip the rename. With the patch you 
proposed, mv -n could call lstat and get a failure (with errno == 
ENOENT), then (after another process creates the file) call renameat2 
with RENAME_NOREPLACE and after this fails (with errno == EEXIST) report 
an error. mv -n should silently succeed in that case.

> Sounds like a corner case. Please consider writing a separate patch 
> for that.

OK, that's pretty straightforward so I installed it. Please see the 
first attached patch.

> I had difficulties trying to evaluate the patch. It does not compile
That's what I get for sending an untested patch. Sorry. I fixed the bugs 
you mentioned and tested the result. Please see the second attached 
patch, which I have not installed.

There is an interesting behavior change with this second patch. 
Currently, 'mv -n a a' fails with a diagnostic "mv: 'a' and 'a' are the 
same file". With the patch, 'mv -n a a' silently succeeds. The coreutils 
documentation allows both behaviors. I doubt whether anyone cares, and 
doing it the new way avoids some syscalls so I left it that way.

[0001-mv-n-overrides-u.patch (text/x-patch, attachment)]

[0002-mv-improve-n-atomicity.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.