|
From: | Paul Eggert |
Subject: | bug#29961: [PATCH] mv -n: do not overwrite the destination |
Date: | Thu, 4 Jan 2018 17:00:52 -0800 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 |
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.
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.I had difficulties trying to evaluate the patch. It does not compile
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
Description: Text Data
0002-mv-improve-n-atomicity.patch
Description: Text Data
[Prev in Thread] | Current Thread | [Next in Thread] |