Subject: Re: bug#25342: GNU coreutils: race condition in "ln -f source dest"

From: Mirsad Goran Todorovac <[email-masked]>
Date: Wed, 4 Jan 2017 21:02:37 +0000 (UTC)

I did not understand this no-op behavior. It is unlike what my Ubuntu kernel experienced. I may later try to see what 4.x kernels do ...

Please consider the trace below. Thank you.
M.T.
marvin@marvin-desktop:~$ uname -a
Linux marvin-desktop 3.5.0-34-generic #55-Ubuntu SMP Thu Jun 6 20:20:19 UTC 2013 i686 athlon i686 GNU/Linux
marvin@marvin-desktop:~$ 
marvin@marvin-desktop:~$ rm -f copy copy2 copy2~ ; cp /etc/resolv.conf.orig copy ; cp /etc/resolv.conf copy2
marvin@marvin-desktop:~$ cat copy copy2
# Dynamic resolv.conf(5) file for glibc resolver(3) generated by resolvconf(8)
#     DO NOT EDIT THIS FILE BY HAND -- YOUR CHANGES WILL BE OVERWRITTEN
nameserver 127.0.1.1
search lan
# Dynamic resolv.conf(5) file for glibc resolver(3) generated by resolvconf(8)
#     DO NOT EDIT THIS FILE BY HAND -- YOUR CHANGES WILL BE OVERWRITTEN
nameserver 194.146.109.223
nameserver 194.146.109.224
search lan
marvin@marvin-desktop:~$ strace coreutils-devel/bin/ln -b -fo copy copy2
execve("coreutils-devel/bin/ln", ["coreutils-devel/bin/ln", "-b", "-fo", "copy", "copy2"], [/* 65 vars */]) = 0
brk(NULL)                               = 0xa036000
access("/etc/ld.so.nohwcap", F_OK)      = -1 ENOENT (No such file or directory)
mmap2(NULL, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xb7745000
access("/etc/ld.so.preload", R_OK)      = -1 ENOENT (No such file or directory)
open("/etc/ld.so.cache", O_RDONLY|O_CLOEXEC) = 3
fstat64(3, {st_mode=S_IFREG|0644, st_size=127411, ...}) = 0
mmap2(NULL, 127411, PROT_READ, MAP_PRIVATE, 3, 0) = 0xb7725000
close(3)                                = 0
access("/etc/ld.so.nohwcap", F_OK)      = -1 ENOENT (No such file or directory)
open("/lib/i386-linux-gnu/libc.so.6", O_RDONLY|O_CLOEXEC) = 3
read(3, "\177ELF\1\1\1\3\0\0\0\0\0\0\0\0\3\0\3\0\1\0\0\0\320\207\1\0004\0\0\0"..., 512) = 512
fstat64(3, {st_mode=S_IFREG|0755, st_size=1786484, ...}) = 0
mmap2(NULL, 1792540, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 0xb756f000
mprotect(0xb771e000, 4096, PROT_NONE)   = 0
mmap2(0xb771f000, 12288, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x1af000) = 0xb771f000
mmap2(0xb7722000, 10780, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0xb7722000
close(3)                                = 0
mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xb756e000
set_thread_area({entry_number:-1, base_addr:0xb756e940, limit:1048575, seg_32bit:1, contents:0, read_exec_only:0, limit_in_pages:1, seg_not_present:0, useable:1}) = 0 (entry_number:6)
mprotect(0xb771f000, 8192, PROT_READ)   = 0
mprotect(0x8058000, 4096, PROT_READ)    = 0
mprotect(0xb776a000, 4096, PROT_READ)   = 0
munmap(0xb7725000, 127411)              = 0
brk(NULL)                               = 0xa036000
brk(0xa057000)                          = 0xa057000
open("/usr/lib/locale/locale-archive", O_RDONLY|O_LARGEFILE|O_CLOEXEC) = 3
fstat64(3, {st_mode=S_IFREG|0644, st_size=4229984, ...}) = 0
mmap2(NULL, 2097152, PROT_READ, MAP_PRIVATE, 3, 0) = 0xb736e000
mmap2(NULL, 4096, PROT_READ, MAP_PRIVATE, 3, 0x2d4000) = 0xb7744000
close(3)                                = 0
stat64("copy2", {st_mode=S_IFREG|0644, st_size=216, ...}) = 0
open("/usr/share/locale/locale.alias", O_RDONLY|O_CLOEXEC) = 3
fstat64(3, {st_mode=S_IFREG|0644, st_size=2995, ...}) = 0
read(3, "# Locale name alias data base.\n#"..., 4096) = 2995
read(3, "", 4096)                       = 0
close(3)                                = 0
open("/home/marvin/coreutils-devel/share/locale/en_US/LC_MESSAGES/coreutils.mo", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/home/marvin/coreutils-devel/share/locale/en/LC_MESSAGES/coreutils.mo", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/usr/share/locale-langpack/en_US/LC_MESSAGES/coreutils.mo", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/usr/share/locale-langpack/en/LC_MESSAGES/coreutils.mo", O_RDONLY) = 3
fstat64(3, {st_mode=S_IFREG|0644, st_size=619, ...}) = 0
mmap2(NULL, 619, PROT_READ, MAP_PRIVATE, 3, 0) = 0xb7743000
close(3)                                = 0
lstat64("copy", {st_mode=S_IFREG|0644, st_size=183, ...}) = 0
lstat64("copy2", {st_mode=S_IFREG|0644, st_size=216, ...}) = 0
open(".", O_RDONLY|O_NONBLOCK|O_LARGEFILE|O_DIRECTORY|O_CLOEXEC) = 3
fstat64(3, {st_mode=S_IFDIR|0755, st_size=12288, ...}) = 0
getdents64(3, /* 152 entries */, 32768) = 5808
getdents64(3, /* 0 entries */, 32768)   = 0
close(3)                                = 0
link("copy2", "copy2~")                 = 0
getpid()                                = 3085
linkat(AT_FDCWD, "copy", AT_FDCWD, "copy.003085.bak", 0) = 0
renameat(AT_FDCWD, "copy.003085.bak", AT_FDCWD, "copy2") = 0
_llseek(0, 0, 0xbf9087a8, SEEK_CUR)     = -1 ESPIPE (Illegal seek)
close(0)                                = 0
close(1)                                = 0
close(2)                                = 0
exit_group(0)                           = ?
+++ exited with 0 +++
marvin@marvin-desktop:~$ 
marvin@marvin-desktop:~$ cat copy copy2 copy2~
# Dynamic resolv.conf(5) file for glibc resolver(3) generated by resolvconf(8)
#     DO NOT EDIT THIS FILE BY HAND -- YOUR CHANGES WILL BE OVERWRITTEN
nameserver 127.0.1.1
search lan
# Dynamic resolv.conf(5) file for glibc resolver(3) generated by resolvconf(8)
#     DO NOT EDIT THIS FILE BY HAND -- YOUR CHANGES WILL BE OVERWRITTEN
nameserver 127.0.1.1
search lan
# Dynamic resolv.conf(5) file for glibc resolver(3) generated by resolvconf(8)
#     DO NOT EDIT THIS FILE BY HAND -- YOUR CHANGES WILL BE OVERWRITTEN
nameserver 194.146.109.223
nameserver 194.146.109.224
search lan
marvin@marvin-desktop:~$ 

    On Tuesday, January 3, 2017 6:38 AM, Paul Eggert <[email-masked]> wrote:

 Mirsad Goran Todorovac wrote:
> It would suffice that:
> linkat (AT_CWDFD, source, AT_CWDFD, dest, 0) == 0 || unlink (dest) == 0 && linkat (AT_CWDFD, source, AT_CWDFD, dest, 0) == 0;
> be replaced with:
> linkat (AT_CWDFD, source, AT_CWDFD, dest_tmp, 0) == 0 && renameat (AT_CWDFD, dest_tmp, AT_CWDFD, dest) == 0;

It isn't that simple, since renameat is a no-op if the source and destination 
are already hard links. So the patch you sent in would not work.

Anyway, I'm not disagreeing with the basic idea, I'm merely saying I don't see 
why we should have an option to keep the old behavior. Anybody who really wants 
the old behavior can do 'rm -f new && ln old new' instead.